848008 - add support into tbpl for robocop 1 and 2

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

today we landed and reconfigured the buildbot masters to split robocop into two chunks: 
robocop-1
robocop-2

these should be:
rc1
rc2

on tbpl.

Comment 1

[Ed Morley [:emorley]]

Attachment: Patch v1

Not sure from whom to request review.

Bit of an intro to this patch:
* TBPL has the concept of a job type and what it calls a "machineNumber".
* The helpfully named machineNumber is the job part (ie "1" for mochitest 1 out of 5; or "bc" in the case of mochitest-browser-chrome.
* TBPL currently uses two ways of dealing with tests vs subtests:
  (a) The first (used by mochitests) is to call all of them mochitests, but abuse the machine number (which in my mind should only for for numeric part numbers) to differentiate between {mochitest-plain,mochitest-browser-chrome,...}.
  (b) The second (used by the reftests group) is to call each different strain of reftests their correct name ({reftest,crashtest,jsreftest}), but then join them back together again using the grouping feature in Config.js (eg: https://hg.mozilla.org/webtools/tbpl/file/62faa43eb3c7/js/Config.js#l293)

This patch switches the robocop implementation from (a) to (b), since I think it's cleaner, and it's what we already do for things like reftest/crashtest, where we have multiple symbol+numbers displayed (eg R1 R2 C1 C2).

In order to do this, we:
* Make sure robocop jobs are designated "Robocop" by Data.js, not "Mochitest".
* Add this new job type to the Mochitest group in Config.js, as well as a symbol for them ("rc").
* In Data.js, add robocop to the list of job types that have a part number extracted.
* Remove the now obsolete (and yucky hardcoded) |return "rc";| in Data.js, since the new output will be a combination of the symbol added to Config.js + part number from the previous bullet.

I'll write a followup patch to do some more cleanup with suites other than robocop.

Comment 2

[Ed Morley [:emorley]]

tl;dr:
We can't (nor do we wish to) get away with hardcoding robocop as "rc" at the end of Data.js; so are just switching to the same method used by the other multi-part tests (reftest, crashtest etc).

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 721718 [details] [diff] [review]
Patch v1

The approach is sound, the code changes make sense, and applied locally, the patch works. Sounds like an r+ to me!

Comment 4

[Ed Morley [:emorley]]

Thank you :-)

https://hg.mozilla.org/webtools/tbpl/rev/05d457d4e7f8

Comment 5

[Geoff Brown [:gbrown]]

Does this affect try server syntax? Do we support -u robocop-1,robocop-2 now? Does -u robocop still work?

Comment 6

[Ed Morley [:emorley]]

(In reply to Geoff Brown [:gbrown] from comment #5)
> Does this affect try server syntax? Do we support -u robocop-1,robocop-2
> now? Does -u robocop still work?

Bug 843107 will, yeah (this bug is just for TBPL). Not sure if -u robocop will trigger both. I've filed bug 848708 for updating trychooser.

Comment 7

[Ed Morley [:emorley]]

In production :-)