Closed Bug 786314 Opened 9 years ago Closed 9 years ago

Remove obsolete platforms/build/test names from Config.js and Data.js

Categories

(Tree Management Graveyard :: TBPL, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: emorley, Assigned: emorley)

References

Details

Attachments

(5 files)

eg:
* QT
* maemo
* android-xul
* RPM builds
* Likely quite a few of the old talos suites
* (If 18 weeks pass before this is done, OS X 10.5)

We should also make the inclusion of builds more consistent. ie: at the moment for B2G we:
>        /B2G .*_gecko/i.test(name) ? "B2G Gecko Build" :
...however for everything else it gets listed at "Build". We should either list all individually, or else all "Build" (I'm leaning towards the latter to simplify Data.js). If we go with the latter we can also simplify:
https://bugzilla.mozilla.org/attachment.cgi?id=655930&action=diff#a/js/UserInterface.js_sec2

Whilst we're at it, we should also tweak the order / at least make Data.js and Config.js match + fix the comments.
s/listed at "Build"/listed as "Build"/
Bg isn't B (despite the way we got screwed into calling it that by releng changes today) - the only thing it's close to is the SpiderMonkey builds that only build the jsshell (or, more accurately but less actual and visible, the XULRunner builds, if we had actually gone ahead with doing Firefox-on-XULRunner). Possibly if we someday do have the Bf that bug 767473 comment 0 talks about, that would amount to a B. Or possibly we'll decide that such a thing is proprietary and won't ever do it in public, and Bg will be the only public build and can become B, but right now it isn't.
Removes obsolete platforms & suites.

Joel, can you just have a quick skim of the talos changes to confirm there isn't anything unexpected there please? :-)
Assignee: nobody → bmo
Status: NEW → ASSIGNED
Attachment #673024 - Flags: review?(philringnalda)
Attachment #673024 - Flags: feedback?(jmaher)
Attached patch Part B: CleanupSplinter Review
* Removes many unnecessary uses of: '.*'
* Consolidates most cases of several regex for the same platform/type.
* Annotates entries that will be made redundant soon.

(To test, see next attachment)

jmaher, similar to the previous, just wondered if you could take a very quick glance and let me know if anything drastically wrong with the talos entries - or if you wanted any changes to the grouped order or display letters/captions.
Attachment #673031 - Flags: review?(philringnalda)
Attachment #673031 - Flags: feedback?(jmaher)
Comment on attachment 673024 [details] [diff] [review]
Part A: Remove obsolete

>-        // Debug builds on 10.7 that are only run on the 10.5 "osx" platform
>-        /OS X.*32-bit/.test(name) ? "osx-10-5" :

It hasn't actually been 18 weeks since the end of August, just yet.
Applies after the other two patches.

Open the webconsole, filter on '###' and browse around the different trees with &noignore=1 set, and pressing the downarrow to load more pushes and thus check the less frequent buildernames.

The only entry you should see is for dxr, which is an existing issue, being sorted in bug 803357.
(In reply to Phil Ringnalda (:philor) from comment #6)
> It hasn't actually been 18 weeks since the end of August, just yet.

Those entries are already redundant - they don't appear in the attached buildername list. I've left the ones that are still needed.
(In reply to Ed Morley [:edmorley UTC+1] from comment #7)
> Created attachment 673034 [details] [diff] [review]
> For testing locally
> 
> Applies after the other two patches.

I should add that unknown platforms/types will (as of bug 802202) show up as "Other" and "?" respectively - the attached testing patch just saves having to skim over every row of builds carefully.
Comment on attachment 673024 [details] [diff] [review]
Part A: Remove obsolete

Review of attachment 673024 [details] [diff] [review]:
-----------------------------------------------------------------

I don't see anything that concerns me.

::: js/Config.js
@@ +237,3 @@
>                        "SpiderMonkey --enable-sm-fail-on-warnings"],
> +    "Talos Performance" : ["Talos dhtml", "Talos ts", "Talos dromaeo", "Talos svg",
> +      "Talos tp nochrome", "Talos tp", "Talos nochrome",

we don't run tpnochrome, unless it is on some old fashioned branch.

@@ -295,5 @@
>      "Reftest Unaccelerated" : "Ru",
>      "Reftest-IPC" : "Ripc",
>      "Reftest" : "R",
>      "JSReftest" : "J",
> -    "Peptest" : "P",

is this really removed?
Attachment #673024 - Flags: feedback?(jmaher) → feedback+
Comment on attachment 673031 [details] [diff] [review]
Part B: Cleanup

Review of attachment 673031 [details] [diff] [review]:
-----------------------------------------------------------------

this stuff looks hairy, but you got to work with what is already there:)

::: js/Data.js
@@ +479,5 @@
>        // transitioned to the new bug 586664 form
>        var os =
> +        /(?:linux|fedora).*64/i.test(name) ? "linux64" :
> +        /linux|fedora/i.test(name) ? "linux" :
> +        /mountain[ ]?lion/i.test(name) ? "osx-10-8" :

no need for '/OS\s?X.*10\.8/.test(name) ? "osx-10-8" :' ?
Attachment #673031 - Flags: feedback?(jmaher) → feedback+
(In reply to Joel Maher (:jmaher) from comment #11)
> this stuff looks hairy, but you got to work with what is already there:)

Yeah the whole implementation is suboptimal - hope to overhaul at a later date.

> > +        /mountain[ ]?lion/i.test(name) ? "osx-10-8" :
> 
> no need for '/OS\s?X.*10\.8/.test(name) ? "osx-10-8" :' ?

All of the 10.8 entries in the attached buildernames list are either of form:
.* MacOSX Mountain Lion 10.8 .*
  or:
jetpack-.*-mountainlion-.*

...so I took the opportunity to remove the regex overlap (I haven't done any profiling of TBPL [yet], but I don't imagine the nested regexes help in any way, so keen to remove as many as possible).

(In reply to Joel Maher (:jmaher) from comment #10)
> > +      "Talos tp nochrome", "Talos tp", "Talos nochrome",
> 
> we don't run tpnochrome, unless it is on some old fashioned branch.

We run "Android Tegra 250 mozilla-central talos remote-tp4m_nochrome" (and for all the other trunk trees). Should this be turned off, or is it currently being called the wrong thing in TBPL?

> > -    "Peptest" : "P",
> 
> is this really removed?

Peptest is currently switched off on all trees (bug 785373).

I contemplated leaving the entry for it in there (since I'm presuming it will be turned on again at some point), but bug 785373 removed the hacky buildbot-config implementation for it (bhearsum said: "can you rip it out completely instead? we should do it in a better way when we want it back") - so it won't just be a quick case of toggling a False to True. We should therefore have enough lead time to add it back to TBPL when patches start appearing to enable it again.

That said, happy to leave it in TBPL if you think the timeframe for re-enabling makes that more appropriate?
(In reply to Ed Morley [:edmorley UTC+1] from comment #12)

> (In reply to Joel Maher (:jmaher) from comment #10)
> > > +      "Talos tp nochrome", "Talos tp", "Talos nochrome",
> > 
> > we don't run tpnochrome, unless it is on some old fashioned branch.
> 
> We run "Android Tegra 250 mozilla-central talos remote-tp4m_nochrome" (and
> for all the other trunk trees). Should this be turned off, or is it
> currently being called the wrong thing in TBPL?
> 

we don't run tp5 with no chrome, and for remote tp4m, we only run it with nochrome (tpn).  Yes, it is all confusing and I get confused by it as well. 


> > > -    "Peptest" : "P",
> > 
> > is this really removed?
> 
> Peptest is currently switched off on all trees (bug 785373).
> 
> That said, happy to leave it in TBPL if you think the timeframe for
> re-enabling makes that more appropriate?

I am not aware of a timeline to re-enable Peptest, we can always add it back in.
Not going to bother anyone with an r? for this.
philor, don't suppose you've had a chance to glance at this? I realise it's not really the easiest patch to read for accuracy - sorry! I've retested using the attached testing patch and no other unknown items have cropped up, so maybe just easier to land this on tbpl-dev and see how we go? (A have a half dozen things that are blocked on this bug due to the rebasing they would cause if I landed prior).
Comment on attachment 673024 [details] [diff] [review]
Part A: Remove obsolete

I was figuring a couple of hours, which wasn't an option last weekend when I only had one day off work, but there are plenty of other things I'd rather do with a couple of hours. It'll work, or it won't.
Attachment #673024 - Flags: review?(philringnalda) → review+
Attachment #673031 - Flags: review?(philringnalda) → review+
Depends on: 805201
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Blocks: 789652
Product: Webtools → Tree Management
Product: Tree Management → Tree Management Graveyard
You need to log in before you can comment on or make changes to this bug.