gpu/config Add unittests for driver vendor collection

gpu/config Add unittests for driver vendor collection

BUG=633286
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

[Corentin Wallez, 2016-08-01 18:22:26]

Description was changed from

==========
gpu/config Add unittests for driver vendor collection

BUG=633286
==========

to

==========
gpu/config Add unittests for driver vendor collection

BUG=633286
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Corentin Wallez, 2016-08-01 18:22:42]

The CQ bit was checked by cwallez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-01 18:23:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Corentin Wallez, 2016-08-01 18:23:36]

cwallez@chromium.org changed reviewers:
+ kbr@chromium.org, zmo@chromium.org

[Corentin Wallez, 2016-08-01 18:23:37]

PTAL, this will highlight the problem causing the Linux AMD bot to be completely
red.

[commit-bot: I haz the power, 2016-08-01 18:35:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-01 18:35:35]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Corentin Wallez, 2016-08-01 18:44:03]

The CQ bit was checked by cwallez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-01 18:44:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Julien Isorce Samsung, 2016-08-01 19:36:38]

On 2016/08/01 18:23:37, Corentin Wallez wrote:
> PTAL, this will highlight the problem causing the Linux AMD bot to be
completely
> red.

Hi, sorry I was only looking at the Linux Intel bot (cause of the revert
https://codereview.chromium.org/2153373002/).
I am having a look at the Linux AMD bot at the moment. At first glance it is a
bit mysterious
since prior my patch driver_vendor and driver_version for ""4.5.13399
Compatibility Profile Context 15.201.1151"
would not have been relevant either: "Compatibility" / "Profile" . At least the
version is ok now.
Will let you know if I found something quickly otherwise I will revert my CL.

[commit-bot: I haz the power, 2016-08-01 19:41:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-01 19:41:44]

Dry run: This issue passed the CQ dry run.

[Ken Russell (switch to Gerrit), 2016-08-01 19:47:58]

LGTM

I haven't checked the status of the bot but if a patch slipped through which has
turned it red it should be reverted right away.

[Corentin Wallez, 2016-08-01 19:49:20]

The CQ bit was checked by cwallez@chromium.org

[commit-bot: I haz the power, 2016-08-01 19:49:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Corentin Wallez, 2016-08-01 19:49:38]

The CQ bit was unchecked by cwallez@chromium.org

[Corentin Wallez, 2016-08-01 19:51:24]

On 2016/08/01 at 19:36:38, j.isorce wrote:
> On 2016/08/01 18:23:37, Corentin Wallez wrote:
> > PTAL, this will highlight the problem causing the Linux AMD bot to be
completely
> > red.
> 
> Hi, sorry I was only looking at the Linux Intel bot (cause of the revert
https://codereview.chromium.org/2153373002/).
> I am having a look at the Linux AMD bot at the moment. At first glance it is a
bit mysterious
> since prior my patch driver_vendor and driver_version for ""4.5.13399
Compatibility Profile Context 15.201.1151"
> would not have been relevant either: "Compatibility" / "Profile" . At least
the version is ok now.
> Will let you know if I found something quickly otherwise I will revert my CL.

No problem, I wasn't sure if your patch could be reverted safely. Doing this
now. Prior to your CL driver vendor was "ATI" and the version was parsed
correctly.

[Julien Isorce Samsung, 2016-08-01 20:30:56]

On 2016/08/01 19:51:24, Corentin Wallez wrote:
> On 2016/08/01 at 19:36:38, j.isorce wrote:
> > On 2016/08/01 18:23:37, Corentin Wallez wrote:
> > > PTAL, this will highlight the problem causing the Linux AMD bot to be
> completely
> > > red.
> > 
> > Hi, sorry I was only looking at the Linux Intel bot (cause of the revert
> https://codereview.chromium.org/2153373002/).
> > I am having a look at the Linux AMD bot at the moment. At first glance it is
a
> bit mysterious
> > since prior my patch driver_vendor and driver_version for ""4.5.13399
> Compatibility Profile Context 15.201.1151"
> > would not have been relevant either: "Compatibility" / "Profile" . At least
> the version is ok now.
> > Will let you know if I found something quickly otherwise I will revert my
CL.
> 
> No problem, I wasn't sure if your patch could be reverted safely. Doing this
> now. Prior to your CL driver vendor was "ATI" and the version was parsed
> correctly.

Ah you are right, thx for pointing that I had not realized there was
CollectDriverVersionATI.

[Julien Isorce Samsung, 2016-08-01 20:54:12]

On 2016/08/01 20:30:56, Julien Isorce wrote:
> On 2016/08/01 19:51:24, Corentin Wallez wrote:
> > On 2016/08/01 at 19:36:38, j.isorce wrote:
> > > On 2016/08/01 18:23:37, Corentin Wallez wrote:
> > > > PTAL, this will highlight the problem causing the Linux AMD bot to be
> > completely
> > > > red.
> > > 
> > > Hi, sorry I was only looking at the Linux Intel bot (cause of the revert
> > https://codereview.chromium.org/2153373002/).
> > > I am having a look at the Linux AMD bot at the moment. At first glance it
is
> a
> > bit mysterious
> > > since prior my patch driver_vendor and driver_version for ""4.5.13399
> > Compatibility Profile Context 15.201.1151"
> > > would not have been relevant either: "Compatibility" / "Profile" . At
least
> > the version is ok now.
> > > Will let you know if I found something quickly otherwise I will revert my
> CL.
> > 
> > No problem, I wasn't sure if your patch could be reverted safely. Doing this
> > now. Prior to your CL driver vendor was "ATI" and the version was parsed
> > correctly.
> 
> Ah you are right, thx for pointing that I had not realized there was
> CollectDriverVersionATI.

But about the last part you added in the unit test I cannot do anything since
for
the proprietary ATI driver it is not meant to be extracted from the gl version.
And prior my CL it would have hit the early return "if (pos == 0) return
kCollectInfoNonFatalFailure;"

[Corentin Wallez, 2016-08-01 20:59:19]

On 2016/08/01 at 20:54:12, j.isorce wrote:
> On 2016/08/01 20:30:56, Julien Isorce wrote:
> > On 2016/08/01 19:51:24, Corentin Wallez wrote:
> > > On 2016/08/01 at 19:36:38, j.isorce wrote:
> > > > On 2016/08/01 18:23:37, Corentin Wallez wrote:
> > > > > PTAL, this will highlight the problem causing the Linux AMD bot to be
> > > completely
> > > > > red.
> > > > 
> > > > Hi, sorry I was only looking at the Linux Intel bot (cause of the revert
> > > https://codereview.chromium.org/2153373002/).
> > > > I am having a look at the Linux AMD bot at the moment. At first glance
it is
> > a
> > > bit mysterious
> > > > since prior my patch driver_vendor and driver_version for ""4.5.13399
> > > Compatibility Profile Context 15.201.1151"
> > > > would not have been relevant either: "Compatibility" / "Profile" . At
least
> > > the version is ok now.
> > > > Will let you know if I found something quickly otherwise I will revert
my
> > CL.
> > > 
> > > No problem, I wasn't sure if your patch could be reverted safely. Doing
this
> > > now. Prior to your CL driver vendor was "ATI" and the version was parsed
> > > correctly.
> > 
> > Ah you are right, thx for pointing that I had not realized there was
> > CollectDriverVersionATI.
> 
> But about the last part you added in the unit test I cannot do anything since
for
> the proprietary ATI driver it is not meant to be extracted from the gl
version.
> And prior my CL it would have hit the early return "if (pos == 0) return
kCollectInfoNonFatalFailure;"

Yeah just noticed that, proprietary NVIDIA / AMD is noticed using the device PCI
id, so the change to the unittest is useless. Closing this CL, apologies for the
noise.

[Ken Russell (switch to Gerrit), 2016-08-01 22:20:24]

On 2016/08/01 20:59:19, Corentin Wallez wrote:
> On 2016/08/01 at 20:54:12, j.isorce wrote:
> > On 2016/08/01 20:30:56, Julien Isorce wrote:
> > > On 2016/08/01 19:51:24, Corentin Wallez wrote:
> > > > On 2016/08/01 at 19:36:38, j.isorce wrote:
> > > > > On 2016/08/01 18:23:37, Corentin Wallez wrote:
> > > > > > PTAL, this will highlight the problem causing the Linux AMD bot to
be
> > > > completely
> > > > > > red.
> > > > > 
> > > > > Hi, sorry I was only looking at the Linux Intel bot (cause of the
revert
> > > > https://codereview.chromium.org/2153373002/).
> > > > > I am having a look at the Linux AMD bot at the moment. At first glance
> it is
> > > a
> > > > bit mysterious
> > > > > since prior my patch driver_vendor and driver_version for ""4.5.13399
> > > > Compatibility Profile Context 15.201.1151"
> > > > > would not have been relevant either: "Compatibility" / "Profile" . At
> least
> > > > the version is ok now.
> > > > > Will let you know if I found something quickly otherwise I will revert
> my
> > > CL.
> > > > 
> > > > No problem, I wasn't sure if your patch could be reverted safely. Doing
> this
> > > > now. Prior to your CL driver vendor was "ATI" and the version was parsed
> > > > correctly.
> > > 
> > > Ah you are right, thx for pointing that I had not realized there was
> > > CollectDriverVersionATI.
> > 
> > But about the last part you added in the unit test I cannot do anything
since
> for
> > the proprietary ATI driver it is not meant to be extracted from the gl
> version.
> > And prior my CL it would have hit the early return "if (pos == 0) return
> kCollectInfoNonFatalFailure;"
> 
> Yeah just noticed that, proprietary NVIDIA / AMD is noticed using the device
PCI
> id, so the change to the unittest is useless. Closing this CL, apologies for
the
> noise.

Darn, sorry to hear this wouldn't work. Julien, can you figure out a way to
better unit test your other change ( https://codereview.chromium.org/2153373002/
)?