Rename existing Blink try bots to include OS version.

Rename existing Blink try bots to include OS version.

Purpose: To make builder names consistent, since builders
for different versions of each OS are being added
(http://crrev.com/2168483002), and those builder names
include OS version.

BUG=590036
Committed: https://chromium.googlesource.com/chromium/tools/build/+/d8ce580ec8c7edd90c9d59a87898d77827237ea6

[qyearsley, 2016-07-21 22:33:35]

qyearsley@chromium.org changed reviewers:
+ dpranke@chromium.org, phajdan.jr@chromium.org

[qyearsley, 2016-07-21 22:33:43]

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

[commit-bot: I haz the power, 2016-07-21 22:33:48]

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

[commit-bot: I haz the power, 2016-07-21 22:37:57]

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

[commit-bot: I haz the power, 2016-07-21 22:37:58]

Dry run: This issue passed the CQ dry run.

[qyearsley, 2016-07-21 22:40:35]

Description was changed from

==========
Rename existing Blink try bots to include OS version.

BUG=590036
==========

to

==========
Rename existing Blink try bots to include OS version.

Purpose: To make builder names consistent, since builders for different versions
of each OS are being added (http://crrev.com/2168483002), and those builder
names include OS version.

BUG=590036
==========

[qyearsley, 2016-07-21 22:40:48]

Description was changed from

==========
Rename existing Blink try bots to include OS version.

Purpose: To make builder names consistent, since builders for different versions
of each OS are being added (http://crrev.com/2168483002), and those builder
names include OS version.

BUG=590036
==========

to

==========
Rename existing Blink try bots to include OS version.

Purpose: To make builder names consistent, since builders
for different versions of each OS are being added
(http://crrev.com/2168483002), and those builder names
include OS version.

BUG=590036
==========

[Dirk Pranke, 2016-07-22 01:00:19]

that's a lot of redundant expectations; we might as well clean that up while
we're renaming things.

The "blink_rel_pass" expectations are identical to the "blink_rel" expectations.
Can you delete those?

Also, instead of generating a "_fail" expectation for every builder that'll be
covering the same code paths, can you just generate one?

[Dirk Pranke, 2016-07-22 01:00:57]

lgtm otherwise.

[qyearsley, 2016-07-22 17:47:40]

On 2016/07/22 at 01:00:19, dpranke wrote:
> that's a lot of redundant expectations; we might as well clean that up while
we're renaming things.
> 
> The "blink_rel_pass" expectations are identical to the "blink_rel"
expectations. Can you delete those?
> 
> Also, instead of generating a "_fail" expectation for every builder that'll be
covering the same code paths, can you just generate one?

Sounds good.

After experimentation, I saw that if all of the
full_blink_tryserver_<buildername>_{pass,fail} tests are removed, line coverage
is still 100%.

In the latest patch, I removed that block, but added one test that had both
"webkit_tests (with patch)" and "webkit_tests (without patch)" fail, since that
appeared to be the case that was covered by those "_fail" tests, and there
wasn't a test like that elsewhere -- although this could be removed and coverage
should still be full.

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

https://codereview.chromium.org/2174553002/diff/20001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium_trybot.expected/webkit_tests_with_and_without_patch_fail.json:705:
"@@@STEP_FAILURE@@@"
This is the expectation file for the new test.

Note that it is considered a "rename from
scripts/slave/recipes/chromium_trybot.expected/full_tryserver_blink_linux_blink_rel_fail.json"
with "linux" changed to "linux_precise".

https://codereview.chromium.org/2174553002/diff/20001/scripts/slave/recipes/c...
File scripts/slave/recipes/chromium_trybot.py (right):

https://codereview.chromium.org/2174553002/diff/20001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium_trybot.py:901: )
If we want to keep just one expectation test (for one platform) that's like the
test above, then it can be moved over here with other similar expectation tests.

[Dirk Pranke, 2016-07-22 22:06:08]

lgtm

[qyearsley, 2016-07-25 16:18:48]

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

[commit-bot: I haz the power, 2016-07-25 16:18:56]

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

[commit-bot: I haz the power, 2016-07-25 16:23:05]

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

[commit-bot: I haz the power, 2016-07-25 16:23:07]

Dry run: This issue passed the CQ dry run.

[qyearsley, 2016-07-25 17:27:51]

On 2016/07/22 at 22:06:08, dpranke wrote:
> lgtm

Note, before submitting this, I still need to prepare a CL to rename all other
places in chromium/src where these bots are referenced, e.g.
https://cs.chromium.org/chromium/src/cc/PRESUBMIT.py?l=341

[qyearsley, 2016-07-26 03:04:34]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2016-07-26 03:04:42]

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

[commit-bot: I haz the power, 2016-07-26 03:08:45]

Description was changed from

==========
Rename existing Blink try bots to include OS version.

Purpose: To make builder names consistent, since builders
for different versions of each OS are being added
(http://crrev.com/2168483002), and those builder names
include OS version.

BUG=590036
==========

to

==========
Rename existing Blink try bots to include OS version.

Purpose: To make builder names consistent, since builders
for different versions of each OS are being added
(http://crrev.com/2168483002), and those builder names
include OS version.

BUG=590036

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/d8ce580ec8c7edd90c9d...
==========

[commit-bot: I haz the power, 2016-07-26 03:08:47]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/tools/build/+/d8ce580ec8c7edd90c9d...

[Michael Achenbach, 2016-07-26 07:26:20]

machenbach@chromium.org changed reviewers:
+ machenbach@chromium.org

[Michael Achenbach, 2016-07-26 07:26:21]

Please don't land changes like this without issuing a master restart. The blink
trybot is now in a bad state and blocks the v8 roll:
https://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel?numbuil...

CC'ing trooper for restarting the relevant masters.

[tandrii(chromium), 2016-07-26 10:10:52]

On 2016/07/26 07:26:21, Michael Achenbach (slow) wrote:
> Please don't land changes like this without issuing a master restart. The
blink
> trybot is now in a bad state and blocks the v8 roll:
>
https://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel?numbuil...
> 
> CC'ing trooper for restarting the relevant masters.

Thanks Michael! Master restart issued:
https://chromereviews.googleplex.com/478757013

[Michael Achenbach, 2016-07-26 12:46:32]

There was no MB change on the chromium side for this:
https://build.chromium.org/p/tryserver.blink/builders/linux_precise_blink_rel...

This bothers the skia and v8 auto-roller.

[tandrii(chromium), 2016-07-26 15:24:55]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2185443003/ by tandrii@chromium.org.

The reason for reverting is: This is breaking autorollers of v8 and skia. See
http://crbug.com/631448.

[qyearsley, 2016-07-26 16:05:51]

On 2016/07/26 at 07:26:21, machenbach wrote:
> Please don't land changes like this without issuing a master restart. The
blink trybot is now in a bad state and blocks the v8 roll:
>
https://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel?numbuil...
> 
> CC'ing trooper for restarting the relevant masters.

Aye, sorry about this:.

So, in general for builder renames, adding builders (and other changes in
master.cfg/slaves.cfg?), the correct order is:

1. Add any changes to mb/mb_config.pyl.
2. File an issue for a master restart, linking to the CL that changes the bot.
3. Commit, making sure that the trooper is aware 
4. Also commit any other relevant changes in src, v8, skia etc.

Is that right?

[Dirk Pranke, 2016-07-26 18:42:04]

On 2016/07/26 16:05:51, qyearsley wrote:
> On 2016/07/26 at 07:26:21, machenbach wrote:
> > Please don't land changes like this without issuing a master restart. The
> blink trybot is now in a bad state and blocks the v8 roll:
> >
>
https://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel?numbuil...
> > 
> > CC'ing trooper for restarting the relevant masters.
> 
> Aye, sorry about this:.
> 
> So, in general for builder renames, adding builders (and other changes in
> master.cfg/slaves.cfg?), the correct order is:
> 
> 1. Add any changes to mb/mb_config.pyl.
> 2. File an issue for a master restart, linking to the CL that changes the bot.
> 3. Commit, making sure that the trooper is aware 
> 4. Also commit any other relevant changes in src, v8, skia etc.
> 
> Is that right?

Yes, all of those changes should be landed as closely together as possible.

You can actually land 1) ahead of time, if instead of renaming the builders, you
just add new entries (and then delete the old entries in a follow-up CL).