Add tests for Android GL driver version extraction.

Add tests for Android GL driver version extraction.

BUG=481955
Committed: https://crrev.com/f9268969d2516c5748ba4b76d716ad0844390ed8
Cr-Commit-Position: refs/heads/master@{#333038}

[Tobias Sargeant, 2015-06-03 14:16:08]

tobiasjs@chromium.org changed reviewers:
+ sievers@chromium.org

[Tobias Sargeant, 2015-06-03 14:16:09]

PTAL. Adds the unittest for driver version parsing (on android) requested in the
discussion on https://codereview.chromium.org/1114503003

[no sievers, 2015-06-03 20:28:14]

lgtm, thanks!

https://codereview.chromium.org/1160293004/diff/60001/gpu/config/gpu_info_col...
File gpu/config/gpu_info_collector_unittest.cc (right):

https://codereview.chromium.org/1160293004/diff/60001/gpu/config/gpu_info_col...
gpu/config/gpu_info_collector_unittest.cc:171: } tests[] = {
nit: s/tests/kTestStrings or so

https://codereview.chromium.org/1160293004/diff/60001/gpu/config/gpu_info_col...
gpu/config/gpu_info_collector_unittest.cc:172: #if defined(OS_ANDROID)
nit: I'd ifdef out the whole test thingie and put a comment that only on Android
we attempt to get the driver version from the GL_VERSION string.

https://codereview.chromium.org/1160293004/diff/60001/gpu/config/gpu_info_col...
gpu/config/gpu_info_collector_unittest.cc:180: "343.00"},
Can you also update the comment in gpu_info_collector_android.cc since it
doesn't apply anymore?

  // Extract driver version from the second number in a string like:
  // "OpenGL ES 2.0 V@6.0 AU@ (CL@2946718)"

Maybe just say that it tries to find a version number behind the 'OpenGL ES x.x'
string?

[Tobias Sargeant, 2015-06-04 12:22:26]

On 2015/06/03 20:28:14, sievers wrote:
> lgtm, thanks!
> 
>
https://codereview.chromium.org/1160293004/diff/60001/gpu/config/gpu_info_col...
> File gpu/config/gpu_info_collector_unittest.cc (right):
> 
>
https://codereview.chromium.org/1160293004/diff/60001/gpu/config/gpu_info_col...
> gpu/config/gpu_info_collector_unittest.cc:171: } tests[] = {
> nit: s/tests/kTestStrings or so

Will do.

>
https://codereview.chromium.org/1160293004/diff/60001/gpu/config/gpu_info_col...
> gpu/config/gpu_info_collector_unittest.cc:172: #if defined(OS_ANDROID)
> nit: I'd ifdef out the whole test thingie and put a comment that only on
Android
> we attempt to get the driver version from the GL_VERSION string.

It seems like the equivalent osx and linux functions also collect the driver
version from the GL_VERSION string?

I left it like this in case someone wanted to add tests for those.

>
https://codereview.chromium.org/1160293004/diff/60001/gpu/config/gpu_info_col...
> gpu/config/gpu_info_collector_unittest.cc:180: "343.00"},
> Can you also update the comment in gpu_info_collector_android.cc since it
> doesn't apply anymore?

Will do.

[no sievers, 2015-06-04 18:51:57]

On 2015/06/04 12:22:26, Tobias Sargeant wrote:

>
https://codereview.chromium.org/1160293004/diff/60001/gpu/config/gpu_info_col...
> > gpu/config/gpu_info_collector_unittest.cc:172: #if defined(OS_ANDROID)
> > nit: I'd ifdef out the whole test thingie and put a comment that only on
> Android
> > we attempt to get the driver version from the GL_VERSION string.
> 
> It seems like the equivalent osx and linux functions also collect the driver
> version from the GL_VERSION string?
> 
> I left it like this in case someone wanted to add tests for those.
> 


You're right! Maybe just add something for Linux and Mac then?

Example for Linux:
GL_VERSION: '4.4.0 NVIDIA 331.79'

Example for Mac:
GL_VERSION: '2.1 INTEL-8.10.44'

[Tobias Sargeant, 2015-06-05 12:34:54]

The CQ bit was checked by tobiasjs@chromium.org

[Tobias Sargeant, 2015-06-05 12:34:54]

The patchset sent to the CQ was uploaded after l-g-t-m from sievers@chromium.org
Link to the patchset: https://codereview.chromium.org/1160293004/#ps100001
(title: "->,<-")

[commit-bot: I haz the power, 2015-06-05 12:35:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1160293004/100001

[commit-bot: I haz the power, 2015-06-05 12:38:20]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-06-05 12:39:21]

Patchset 6 (id:??) landed as
https://crrev.com/f9268969d2516c5748ba4b76d716ad0844390ed8
Cr-Commit-Position: refs/heads/master@{#333038}