Flip remaining chromium_gn bots to the chromium recipe.

Flip remaining chromium_gn bots to the chromium recipe.

There were only a few remaining V8 bots that were using
the chromium_gn recipe, and it turns out they can be
easily flipped over to the chromium recipe.

R=phajdan.jr@chromium.org, machenbach@chromium.org
BUG=481693
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299438

[Dirk Pranke, 2016-03-19 00:34:19]

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium/api.py (left):

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium/api.py:141: if
bot_config.get('set_component_rev'):
The coverage report complained that this was uncovered once I removed the
recipe.

I can see where this was getting set in the chromium_gn recipe, but it looks
like it was only used
by the v8 bots there. 

The same feature exists in the client_v8_fyi.py bot configs in chromium_tests,
but I can't tell if it is simply being ignored, or if maybe it's being handled
elsewhere.

I'm not actually sure what this was for.

@machenbach, any thoughts?

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium/test_api.py (right):

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium/test_api.py:39: if override_step_data:  #
pragma: nocover
The coverage bot also complained that this was now going uncovered, which is a
bit surprising. 

Please speak up if anyone has thoughts about this.

[Paweł Hajdan Jr., 2016-03-19 00:56:28]

Nice cleanup. Minor comments.

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium/api.py (left):

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium/api.py:141: if
bot_config.get('set_component_rev'):
On 2016/03/19 at 00:34:19, Dirk Pranke wrote:
> The coverage report complained that this was uncovered once I removed the
recipe.
> 
> I can see where this was getting set in the chromium_gn recipe, but it looks
like it was only used
> by the v8 bots there. 
> 
> The same feature exists in the client_v8_fyi.py bot configs in chromium_tests,
but I can't tell if it is simply being ignored, or if maybe it's being handled
elsewhere.
> 
> I'm not actually sure what this was for.
> 
> @machenbach, any thoughts?

Ah, configure_bot is not used by the chromium recipe. However, I think it'd be
more correct to move this logic somewhere in chromium_tests rather than delete
it completely.

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium/test_api.py (right):

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium/test_api.py:39: if override_step_data:  #
pragma: nocover
On 2016/03/19 at 00:34:19, Dirk Pranke wrote:
> The coverage bot also complained that this was now going uncovered, which is a
bit surprising. 
> 
> Please speak up if anyone has thoughts about this.

I'd suggest we remove the "overrides" parameter and related code entirely.

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py (right):

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py:32: 'V8 Linux GN':
{
Don't we need to re-generate recipe expectations for these ones?

[Dirk Pranke, 2016-03-19 01:00:54]

I've posted the updated expectations but will wait for feedback from machenbach@
before revising further.

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium/api.py (left):

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium/api.py:141: if
bot_config.get('set_component_rev'):
On 2016/03/19 00:56:28, Paweł Hajdan Jr. wrote:
> On 2016/03/19 at 00:34:19, Dirk Pranke wrote:
> > The coverage report complained that this was uncovered once I removed the
> recipe.
> > 
> > I can see where this was getting set in the chromium_gn recipe, but it looks
> like it was only used
> > by the v8 bots there. 
> > 
> > The same feature exists in the client_v8_fyi.py bot configs in
chromium_tests,
> but I can't tell if it is simply being ignored, or if maybe it's being handled
> elsewhere.
> > 
> > I'm not actually sure what this was for.
> > 
> > @machenbach, any thoughts?
> 
> Ah, configure_bot is not used by the chromium recipe. However, I think it'd be
> more correct to move this logic somewhere in chromium_tests rather than delete
> it completely.

I don't think I follow you here. Are you suggesting that we should move this
into chromium_tests
because without it the v8 bots won't work right, or are you suggesting we should
move it just
in case we might need it in the future?

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium/test_api.py (right):

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium/test_api.py:39: if override_step_data:  #
pragma: nocover
On 2016/03/19 00:56:28, Paweł Hajdan Jr. wrote:
> On 2016/03/19 at 00:34:19, Dirk Pranke wrote:
> > The coverage bot also complained that this was now going uncovered, which is
a
> bit surprising. 
> > 
> > Please speak up if anyone has thoughts about this.
> 
> I'd suggest we remove the "overrides" parameter and related code entirely.

oh, I see, that was a separate passed in parameter. Yeah, we should remove that,
then.

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py (right):

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py:32: 'V8 Linux GN':
{
On 2016/03/19 00:56:28, Paweł Hajdan Jr. wrote:
> Don't we need to re-generate recipe expectations for these ones?

Whoops, good catch. I did generate them but forgot to check them in.

[Paweł Hajdan Jr., 2016-03-19 01:08:32]

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium/api.py (left):

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium/api.py:141: if
bot_config.get('set_component_rev'):
On 2016/03/19 at 01:00:54, Dirk Pranke wrote:
> On 2016/03/19 00:56:28, Paweł Hajdan Jr. wrote:
> > Ah, configure_bot is not used by the chromium recipe. However, I think it'd
be
> > more correct to move this logic somewhere in chromium_tests rather than
delete
> > it completely.
> 
> I don't think I follow you here. Are you suggesting that we should move this
into chromium_tests
> because without it the v8 bots won't work right, or are you suggesting we
should move it just
> in case we might need it in the future?

My rationale is because of v8 bots.

[Dirk Pranke, 2016-03-19 01:29:43]

On 2016/03/19 01:08:32, Paweł Hajdan Jr. wrote:
>
https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
> File scripts/slave/recipe_modules/chromium/api.py (left):
> 
>
https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
> scripts/slave/recipe_modules/chromium/api.py:141: if
> bot_config.get('set_component_rev'):
> On 2016/03/19 at 01:00:54, Dirk Pranke wrote:
> > On 2016/03/19 00:56:28, Paweł Hajdan Jr. wrote:
> > > Ah, configure_bot is not used by the chromium recipe. However, I think
it'd
> be
> > > more correct to move this logic somewhere in chromium_tests rather than
> delete
> > > it completely.
> > 
> > I don't think I follow you here. Are you suggesting that we should move this
> into chromium_tests
> > because without it the v8 bots won't work right, or are you suggesting we
> should move it just
> > in case we might need it in the future?
> 
> My rationale is because of v8 bots.

Okay. Since there were already v8 bots using the chromium recipe, that suggests
to me that either we don't need this code because they're accomplishing the same
thing a different way, or we don't need this code because we don't actually need
this
for the v8 bots, or the other existing v8 bots are actually doing the wrong
thing,
or perhaps we actually need two different kinds of v8 bots and only the ones
that
were on the GN recipe need this.

Since I don't actually understand what this is doing, I don't know which answer
is
right. Let's let @machenbach weigh in ...

[Michael Achenbach, 2016-03-21 09:19:36]

I think this is how the end result should look like, but I'd like to get there
in 2-3 steps to not disturb developers.

As I understand, we need to land configs for the new bots in chromium/src,
right? Until those are in, the bots will fail. I suggest this plan:

1. I add new bots and make them work.
2. I switch over dependent configs, cq, lkgr_finder, gatekeeper, etc.
3. You rebase this CL and land just the deletions.

Might take 2-3 days. I'll also have a look if we really need our own trybots.
I'd be happier about having a chromium trybot that does what we need and
specifying it on our CQ. I'll investigate.

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium/api.py (left):

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium/api.py:141: if
bot_config.get('set_component_rev'):
On 2016/03/19 01:08:32, Paweł Hajdan Jr. wrote:
> On 2016/03/19 at 01:00:54, Dirk Pranke wrote:
> > On 2016/03/19 00:56:28, Paweł Hajdan Jr. wrote:
> > > Ah, configure_bot is not used by the chromium recipe. However, I think
it'd
> be
> > > more correct to move this logic somewhere in chromium_tests rather than
> delete
> > > it completely.
> > 
> > I don't think I follow you here. Are you suggesting that we should move this
> into chromium_tests
> > because without it the v8 bots won't work right, or are you suggesting we
> should move it just
> > in case we might need it in the future?
> 
> My rationale is because of v8 bots.

I'm ok with deleting this code. A similar piece of code exists in 1-2 more
places and looking at the expectations I can see that it is exercised.

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/chromium/test_api.py (right):

https://codereview.chromium.org/1819613002/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/chromium/test_api.py:39: if override_step_data:  #
pragma: nocover
On 2016/03/19 01:00:54, Dirk Pranke wrote:
> On 2016/03/19 00:56:28, Paweł Hajdan Jr. wrote:
> > On 2016/03/19 at 00:34:19, Dirk Pranke wrote:
> > > The coverage bot also complained that this was now going uncovered, which
is
> a
> > bit surprising. 
> > > 
> > > Please speak up if anyone has thoughts about this.
> > 
> > I'd suggest we remove the "overrides" parameter and related code entirely.
> 
> oh, I see, that was a separate passed in parameter. Yeah, we should remove
that,
> then.

Remove is fine.

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py (right):

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py:42: 'BUILD_CONFIG':
'Debug',
This was a release bot before and should stay one.

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipes/c...
File scripts/slave/recipes/chromium.expected/full_client_v8_fyi_V8_Linux_GN.json
(right):

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium.expected/full_client_v8_fyi_V8_Linux_GN.json:26:
"src/v8@22135",
This is correct and indicates that the v8 component revision is set.

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipes/c...
File
scripts/slave/recipes/chromium_trybot.expected/full_tryserver_v8_v8_linux_chromium_gn_rel.json
(right):

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium_trybot.expected/full_tryserver_v8_v8_linux_chromium_gn_rel.json:32:
"src/v8@HEAD",
Also this looks correct. I might add a test case that simulates a failure +
deapply patch to see if got_revision is used correctly on resync.

[Michael Achenbach, 2016-03-21 09:41:59]

> As I understand, we need to land configs for the new bots in chromium/src,
> right? Until those are in, the bots will fail.

Didn't think this until the end. Of course if all the names stay the same,
there's no need for more stuff to land in chromium and cq config and the like
will start using the new bot immediately. So, maybe landing this at once is ok.

I might high-jack this CL and land it in emea daytime though, to have an eye on
the consequences.

[Michael Achenbach, 2016-03-21 09:44:38]

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/trybots.py (right):

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/trybots.py:648: 'buildername': 'V8
Linux GN',
Won't this need analyze_mode='compile'? It should be a compile-only bot, or is
that already given by having only a builder associated with it?

[Dirk Pranke, 2016-03-21 19:21:07]

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py (right):

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py:42: 'BUILD_CONFIG':
'Debug',
On 2016/03/21 09:19:36, Michael Achenbach wrote:
> This was a release bot before and should stay one.

Whoops, that must've been a copy/paste error. Will fix.

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/trybots.py (right):

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/trybots.py:648: 'buildername': 'V8
Linux GN',
On 2016/03/21 09:44:38, Michael Achenbach wrote:
> Won't this need analyze_mode='compile'? It should be a compile-only bot, or is
> that already given by having only a builder associated with it?

You don't have to specify a 'tester' in order to run tests; if the 'buildername'
builder
is going to run tests, then the tryserver will also run the tests.

We could specify analyze_mode='compile' to be ultra-paranoid here, but since
'V8 Linux GN' doesn't run any tests anyway, you don't have to worry about it.

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipes/c...
File scripts/slave/recipes/chromium.expected/full_client_v8_fyi_V8_Linux_GN.json
(right):

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium.expected/full_client_v8_fyi_V8_Linux_GN.json:26:
"src/v8@22135",
On 2016/03/21 09:19:36, Michael Achenbach wrote:
> This is correct and indicates that the v8 component revision is set.

Cool.

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipes/c...
File
scripts/slave/recipes/chromium_trybot.expected/full_tryserver_v8_v8_linux_chromium_gn_rel.json
(right):

https://codereview.chromium.org/1819613002/diff/20001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium_trybot.expected/full_tryserver_v8_v8_linux_chromium_gn_rel.json:32:
"src/v8@HEAD",
On 2016/03/21 09:19:36, Michael Achenbach wrote:
> Also this looks correct. I might add a test case that simulates a failure +
> deapply patch to see if got_revision is used correctly on resync.

Okay, I can take a stab at that.

[Dirk Pranke, 2016-03-22 23:19:04]

Updated. Please take another look?

[Michael Achenbach, 2016-03-23 10:37:16]

lgtm, thanks! I'll try to land this soon in EMEA daytime.

https://codereview.chromium.org/1819613002/diff/40001/scripts/slave/recipes/c...
File
scripts/slave/recipes/chromium_trybot.expected/compile_failure_with_component_rev.json
(right):

https://codereview.chromium.org/1819613002/diff/40001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium_trybot.expected/compile_failure_with_component_rev.json:346:
"src/v8@801ada225ddc271c132c3a35f03975671d43e399",
Yes. That looks good. Both chromium and v8 specify the revision that has been
synced to on the first bot_update.

[Michael Achenbach, 2016-03-23 10:37:35]

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

[commit-bot: I haz the power, 2016-03-23 10:37:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1819613002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1819613002/40001

[commit-bot: I haz the power, 2016-03-23 10:41:36]

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

[commit-bot: I haz the power, 2016-03-23 10:41:36]

Dry run: This issue passed the CQ dry run.

[Michael Achenbach, 2016-03-23 12:36:34]

The CQ bit was checked by machenbach@chromium.org

[commit-bot: I haz the power, 2016-03-23 12:36:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1819613002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1819613002/40001

[commit-bot: I haz the power, 2016-03-23 12:40:45]

Description was changed from

==========
Flip remaining chromium_gn bots to the chromium recipe.

There were only a few remaining V8 bots that were using
the chromium_gn recipe, and it turns out they can be
easily flipped over to the chromium recipe.

R=phajdan.jr@chromium.org, machenbach@chromium.org
BUG=481693
==========

to

==========
Flip remaining chromium_gn bots to the chromium recipe.

There were only a few remaining V8 bots that were using
the chromium_gn recipe, and it turns out they can be
easily flipped over to the chromium recipe.

R=phajdan.jr@chromium.org, machenbach@chromium.org
BUG=481693

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299438
==========

[commit-bot: I haz the power, 2016-03-23 12:40:46]

Committed patchset #3 (id:40001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=299438

[Michael Achenbach, 2016-03-23 14:24:50]

https://codereview.chromium.org/1819613002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py (right):

https://codereview.chromium.org/1819613002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py:52: 'V8 Android GN
(dbg)': {
I thing this config should more like this:
https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re...

I'll upload a fix CL as the new bot is failing.

[Dirk Pranke, 2016-03-23 17:54:57]

https://codereview.chromium.org/1819613002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py (right):

https://codereview.chromium.org/1819613002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/client_v8_fyi.py:52: 'V8 Android GN
(dbg)': {
On 2016/03/23 14:24:49, Michael Achenbach wrote:
> I thing this config should more like this:
>
https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re...
> 
> I'll upload a fix CL as the new bot is failing.

You're right, though probably should've been 'android' configs. Thanks!