1304354 - Display TaskCluster Windows Server 2012 builds in rows showing a less ambiguous build platform

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect)

Description

[Rob Thijssen [:grenade (EET/UTC+0300)]]

We have traditionally displayed Windows builds in rows corresponding to the test-platform, rather than the actual build-platform eg: BuildBot windows builds are performed on Windows Server 2008 but displayed in the Windows XP rows for 32 bit builds and the Windows 8 rows for 64 bit builds. See: https://github.com/mozilla/treeherder/blob/69b81c772fd9a9686ac11bcd87c9b212ad2abd58/treeherder/etl/buildbot.py#L208-L209

This causes a fair amount of confusion and is becoming misleading as we roll out support for testing on multiple platforms for a single platform build (eg: 32 bit builds are tested on both WinXP and Win7, 64 bit builds are tested on Win8 and Win10).

It no longer makes sense to display the 64 bit TaskCluster builds in the Windows 8 rows, when they are in fact tested on Windows 10. Rather than compounding the confusion, this proposal is to use rows that more accurately reflect the *build* platform rather than choosing one or the other of the available *test* platforms and leaving it to the user to guess at what operating system the build actually took place on.

TreeHerder already supports a row for Windows Server 2012 x64 (see: https://github.com/mozilla/treeherder/blob/69b81c772fd9a9686ac11bcd87c9b212ad2abd58/ui/js/values.js#L20) and if we were to add a mapping for the same, sans the "x64", we would have a nice place to list both 32 bit and 64 bit Windows TaskCluster builds.

Comment 1

[GitHub Autolander Bot]

Attachment: [treeherder] grenade:patch-1 > mozilla:master

Comment 2

[Rob Thijssen [:grenade (EET/UTC+0300)]]

Attachment: Bug 1304354 - disambiguate Windows build platform in treeherder;

Review commit: https://reviewboard.mozilla.org/r/80134/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/80134/

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8793384 [details]
Bug 1304354 - disambiguate Windows build platform in treeherder;

https://reviewboard.mozilla.org/r/80134/#review78852

I'm happy with the form of the patch, but I will leave the broader question about changing TH platforms to those more expert in such things..

Comment 4

[Rob Thijssen [:grenade (EET/UTC+0300)]]

arr: do you have any ideas about people i should check with before pulling the trigger on this? i sent an email to release, tc and sheriffs and got no response. i assume that this is due to nobody objecting. the patch works (tested on try). as far as i can tell there are no adverse effects, its only a question of whether anyone would object to the idea (which so far nobody has).

Comment 5

[Phil Ringnalda (:philor)]

Nobody's objected, and it's already been 11 hours? Done deal! Everybody completely clears their email backlog in less time than that, including the ones who've slept 8 of those 11, don't they? :)

Comment 6

[Amy Rich [:arr] [:arich]]

I'll tag jmaher (who I know has talked about treeherder visibility stuff in the past), jgriffin (whose group owns treeherder), and dburns (who manages the sheriffs).

Comment 7

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

I have no problem with this- currently we build on osx 10.7 and test on 10.10, they are on different lines.  What will be problematic is that our view in treeherder will be harder to read only due to more real estate.

We already get a lot of developer confusion around the winxp build being used for win7 and likewise the osx 10.7 build used for 10.10.  This is because it is not clear what build is being used for what platform.

regarding win10, we do plan to transition over in the near future to win10 and remove win8- that would reduce confusion there.  With all my ramblings there is no right answer!  I am fine going with the proposed changes.

Comment 8

[Dustin J. Mitchell [:dustin] (he/him)]

Ah, I'm glad to hear you contacted those folks already.  Looking at a try push with the new layout might help to understand the change.

Down the road, I think we could find a way to change the display so that the "parent" builds for tests are indicated visually -- but I think we'll want to sort out bug 1286086 first.  This is a nice start in that direction.

Comment 9

[Jonathan Griffin (:jgriffin)]

I'm ok with this if sheriffs are. Agree with Dustin that ultimately we need a better solution for connecting builds and tests in the UI, but this seems like a fine stop-gap for this particular issue.

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8793281 [details] [review]
[treeherder] grenade:patch-1 > mozilla:master

I'd like to see a before/after comparison of what a single full push (all tiers, showing excluded jobs, expand all job groups) for mozilla-central looks like, but I don't see anything horribly wrong with the change in principle.

Comment 11

[Rob Thijssen [:grenade (EET/UTC+0300)]]

This try push gives some indication of what the change looks like on try/staging: https://treeherder.allizom.org/#/jobs?repo=try&revision=c941b7054d25b8e222992976f423febd4fa9ccdc

Note that the ordering and word spacing is wrong for win 10 and win2012 (because we need those platforms added to the platform order js still).

BuildBot [tier 1] builds still show in their test platform rows.
TaskCluster [tier 2] builds show up in the windows 2012 rows and the TaskCluster [tier 2] tests show up on the test platform rows for win7 and win10.

Comment 12

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/3263bcd624cf09e5f29fe4336cd5a6e99701c40b
Bug 1304354 - Add support for 32 bit Win 2012 builds (#1867)

Allows for 32 bit Windows builds to be shown by build platform rather
than test platform.

Comment 13

[David Burns :automatedtester]

This should be fine as long as Wes is happy with the result of his question in comment 10

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Looks fine to me. I need to make a different patch to teach Treeherder about those Win10VM platforms, but the changes in here look fine.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2a3082dd360
disambiguate Windows build platform in treeherder; r=dustin

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a2a3082dd360