Closed
Bug 1397829
Opened 8 years ago
Closed 8 years ago
add buildernames for stylo-disabled tests which run on hardware, win7 and win10
Categories
(Infrastructure & Operations Graveyard :: CIDuty, task, P1)
Infrastructure & Operations Graveyard
CIDuty
Tracking
(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox58 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | unaffected |
firefox58 | --- | fixed |
People
(Reporter: jmaher, Assigned: aselagea)
References
Details
Attachments
(12 files, 7 obsolete files)
7.28 KB,
patch
|
Details | Diff | Splinter Review | |
134.17 KB,
text/plain
|
jmaher
:
feedback+
|
Details |
11.30 KB,
patch
|
kmoir
:
review+
kmoir
:
feedback+
aselagea
:
checked-in+
|
Details | Diff | Splinter Review |
61.86 KB,
text/plain
|
jmaher
:
feedback+
|
Details |
1.38 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
1.30 KB,
patch
|
aobreja
:
review+
aselagea
:
checked-in+
|
Details | Diff | Splinter Review |
6.82 KB,
patch
|
aobreja
:
review+
aselagea
:
checked-in+
|
Details | Diff | Splinter Review |
2.58 KB,
patch
|
kmoir
:
review+
aselagea
:
checked-in+
|
Details | Diff | Splinter Review |
2.39 KB,
patch
|
kmoir
:
review+
aselagea
:
checked-in+
|
Details | Diff | Splinter Review |
538 bytes,
patch
|
aobreja
:
review+
aselagea
:
checked-in+
|
Details | Diff | Splinter Review |
1.58 KB,
patch
|
aobreja
:
review+
aselagea
:
checked-in+
|
Details | Diff | Splinter Review |
4.10 KB,
patch
|
aobreja
:
review+
aselagea
:
checked-in+
|
Details | Diff | Splinter Review |
we noticed that the jobs we run on hardware are not running in parallel with -stylo-disabled, this is due to the buildernames not existing (and some small tweaks needed in taskcluster configs)
Reporter | ||
Comment 1•8 years ago
|
||
I am not sure if this is even close, :kmoir- I have bugged you quite frequently the last 2 weeks, can I bug you some more to help me get a builderdiff and polish up this patch?
![]() |
||
Comment 2•8 years ago
|
||
I'll look at this this afternoon, have to address a few other bugs first.
Reporter | ||
Comment 3•8 years ago
|
||
:ting, I also see win10-asan having troubles with buildbot-bridge. Are win10-asan tests to be run on hardware or vm? Do all the tests currently run on it (or should they be)? I ask so we need to determine if win10-asan should be added to the buildbot changes and run via buildbot-bridge, or hacked around in taskcluster.
Flags: needinfo?(janus926)
![]() |
||
Comment 4•8 years ago
|
||
Attachment #8905589 -
Attachment is obsolete: true
Attachment #8905589 -
Flags: review?(kmoir)
![]() |
||
Comment 5•8 years ago
|
||
builder diff
![]() |
||
Comment 6•8 years ago
|
||
Reporter | ||
Comment 7•8 years ago
|
||
in this case we want to solve this for unittests, not talos tests; and stylo -> stylo-disabled or some variant. Is there a need to do the full copy like we did in the win10 patch last week?
![]() |
||
Comment 8•8 years ago
|
||
![]() |
||
Updated•8 years ago
|
Attachment #8906059 -
Flags: feedback?(jmaher)
Reporter | ||
Comment 9•8 years ago
|
||
Comment on attachment 8906059 [details]
bug1397829builder.diff
this is specifying a config of 'stylo' and it should be 'stylo-disabled', likewise this is just talos jobs and we need all the jobs (like mochitest, reftest)
Attachment #8906059 -
Flags: feedback?(jmaher) → feedback-
![]() |
||
Comment 10•8 years ago
|
||
This because the name you gave it here was "Windows 10 64-bit stylo"
PLATFORMS['win64-stylo-disabled']['win64_hw_stylo'] = {'name': 'Windows 10 64-bit stylo',
I changed it to 'Windows 10 64-bit stylo disabled' and the diff looks fine. I'll attach a new builder diff and patch
![]() |
||
Comment 11•8 years ago
|
||
Attachment #8906056 -
Attachment is obsolete: true
Reporter | ||
Comment 12•8 years ago
|
||
does this also include unittests, not just talos? from looking over the patch it looks like just talos.
![]() |
||
Comment 13•8 years ago
|
||
Sorry, I read hardware tests as talos tests. If this patch looks okay for talos I can modify it to include unit tests.
Attachment #8906059 -
Attachment is obsolete: true
Reporter | ||
Comment 14•8 years ago
|
||
thanks for the new builderdiff, the 'platform' name looks right now- the branches are good. to be safe we should do beta (or ensure that we migrate to beta when 57 goes there next week.
Otherwise this is good and we should move forward with the unittest names!
Thanks for your help :)
Comment 15•8 years ago
|
||
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #3)
> :ting, I also see win10-asan having troubles with buildbot-bridge. Are
> win10-asan tests to be run on hardware or vm? Do all the tests currently
> run on it (or should they be)? I ask so we need to determine if win10-asan
> should be added to the buildbot changes and run via buildbot-bridge, or
> hacked around in taskcluster.
IIRC, all the tests run on hardware. I am not sure why win64-asan is affected by buildbot changes, aren't the tests run by taskcluster? But anyway, please make win64-asan sync with linux64-asan if you can. Hopefully win64-asan will promote to tier-2 in next quarter.
Flags: needinfo?(janus926)
Comment 16•8 years ago
|
||
(In reply to Ting-Yu Chou [:ting] from comment #15)
> IIRC, all the tests run on hardware.
I was wrong. Just checked, win64-asan runs common-tests [1] and only mochitest-chrome [2], mochitest-clipboard [3] have virtualization "hardware". I am not sure, but I expect the other tests without defining virtualization run on VM.
[1] http://searchfox.org/mozilla-central/rev/00fa5dacedb925022f53d025121f1a919508e7ce/taskcluster/ci/test/test-platforms.yml#250
[2] http://searchfox.org/mozilla-central/rev/00fa5dacedb925022f53d025121f1a919508e7ce/taskcluster/ci/test/tests.yml#665
[3] http://searchfox.org/mozilla-central/rev/00fa5dacedb925022f53d025121f1a919508e7ce/taskcluster/ci/test/tests.yml#725
Reporter | ||
Comment 17•8 years ago
|
||
we have only had win10 hardware support for the last few months and this was stood up only for Talos performance tests- so I will assume this needs the same vm || hardware as desktop unittests- right now desktop unittests don't run on windows 10 reliably, so it is sort of unrealistically to make win10-asan run our unittests and expect them to pass- although we do hope to have that all fixed in the next month or so.
:kmoir, in the patch for unittests, can we have a win10-asan platform as well?
![]() |
||
Updated•8 years ago
|
Flags: needinfo?(kmoir)
Reporter | ||
Comment 18•8 years ago
|
||
:kmoir, I understand you have been busy with the uplift, is there anyone else who can help here?
![]() |
||
Comment 19•8 years ago
|
||
:jmaher I apologize for the delay. I was on course earlier this week and am on releaseduty so trying to catch up with other work. I'll look at it now while my merge scripts are running and if I can't get complete patches today I'll try to get someone from buildduty to look at it.
Flags: needinfo?(kmoir)
![]() |
||
Comment 20•8 years ago
|
||
Mihai do you know if the buildduty folks might have time to look at this and help Joel? If not, I can try to look at it next week when releaseduty calms down.
Flags: needinfo?(mtabara)
Assignee | ||
Comment 21•8 years ago
|
||
I'll look at this today and see if I can help somehow.
Flags: needinfo?(mtabara)
![]() |
||
Comment 22•8 years ago
|
||
Thanks Alin, let me know if you have questions!
Assignee | ||
Comment 23•8 years ago
|
||
Looked at the existing patches and tried a couple of solutions to enable the same unittests on "win64_hw_stylo" as the regular ones we have on Windows 10, but had little luck today :| If it's okay with you, I'll resume working on this on Monday when I return. Meanwhile, I'd like to clarify two things:
a) is my approach above correct? do we need 'stylo disabled' variant for each Windows 10 test we run on hardware? Do we want this for Win 7 as well?
b) what tests do we want for "win10-asan"?
![]() |
||
Updated•8 years ago
|
Flags: needinfo?(jmaher)
Reporter | ||
Comment 24•8 years ago
|
||
For all tests that run on windows10-hw (including unittest and talos), we want the ability to schedule a |windows10-stylo-disabled| and a |windows10-asan| copy. For windows7, we also need a |windows7-stylo-disabled| copy, but not an asan copy.
All changes should apply to version 57+, so this is mozilla-beta, try, mozilla-central, autoland, mozilla-inbound, and other trunk based branches.
Flags: needinfo?(jmaher)
![]() |
||
Comment 25•8 years ago
|
||
Alin, how is it going on this bug?
catlee asked me about it today, let me know if I can help in any way
kmoir: do you / aselagea think you can get https://bugzilla.mozilla.org/show_bug.cgi?id=1397829 done in the next day or so?
Bug 1397829 — ASSIGNED, jmaher@mozilla.com — add buildernames for stylo-disabled tests which run on hardware, win7 and win10
12:18 PM
<kmoir> Kim Moir let me ask him the status of this work
Flags: needinfo?(aselagea)
![]() |
||
Comment 26•8 years ago
|
||
I looked at Alin's dev-master but I wasn't sure of the state of his patches so I'll talk to him tomorrow about it and see how I can help if required.
![]() |
||
Comment 27•8 years ago
|
||
Also, as a side note, catlee mentioned this bug has a script that might be useful for debugging buildbot bridge errors bug 1381597
Assignee | ||
Comment 28•8 years ago
|
||
Note: this patch adds many builders, so I had to manually increase the builder limit in master/master.cfg on my master in order to test the patches.
Also, we already have a "stylo-disabled" variant for the talos tests, so I wonder if we should only run that variant for the corresponding Win7 and Win10 platform where we want stylo disabled.
We'll probably need to do some adjustments in TaskCluster too, didn't have the chance to take a look at that yet.
Flags: needinfo?(aselagea)
Attachment #8909759 -
Flags: feedback?(kmoir)
Assignee | ||
Comment 29•8 years ago
|
||
Builder diff for the patch above.
![]() |
||
Comment 30•8 years ago
|
||
Comment on attachment 8909759 [details] [diff] [review]
bug1397829-4.patch
Note: this patch adds many builders, so I had to manually increase the builder limit in master/master.cfg on my master in order to test the patches.
>> If we are hitting builder limits we may want reconfigure some windows masters to just run certain types of tests in production_masters.json otherwise the master will hit performance issues due to the number of builders. Before we do that though, I'd like to ask jmaher to review the builder list and see if it is correct. Perhaps we could just enable these tests on try first and see how the patches work and then we could deal with the number of builders issue after that.
Also, we already have a "stylo-disabled" variant for the talos tests, so I wonder if we should only run that variant for the corresponding Win7 and Win10 platform where we want stylo disabled.
>>This is a good approach
We'll probably need to do some adjustments in TaskCluster too, didn't have the chance to take a look at that yet.
Other feedback, for this bit starting here
trunk_branches = []
+for name, branch in items_at_least(BRANCHES, 'gecko_version', 57):
+ trunk_branches.append(name)
+
+platforms = ['win32-stylo-disabled', 'win64-stylo-disabled', 'win64-asan']
+for branch in BRANCHES.keys():
+ for platform in platforms:
+ if platform not in BRANCHES[branch]['platforms'].keys():
+ continue
+
+ # Windows 7
+ if platform == 'win32-stylo-disabled':
+ BRANCHES[branch]['platforms'][platform]['talos_slave_platforms'] = ['win7_ix_stylo_disabled']
+ # copy the regular tests we run on Windows 7 HW
+ BRANCHES[branch]['platforms'][platform]['win7_ix_stylo_disabled'] = deepcopy(BRANCHES[branch]['platforms']['win32']['win7_ix'])
+ # Windows 10
+ if platform == 'win64-stylo-disabled':
+ BRANCHES[branch]['platforms'][platform]['talos_slave_platforms'] = ['win10_64_stylo_disabled']
+ # copy the regular tests we run on Windows 10 HW
+ BRANCHES[branch]['platforms'][platform]['win10_64_stylo_disabled'] = deepcopy(BRANCHES[branch]['platforms']['win64']['win10_64'])
+ if platform == 'win64-asan':
+ BRANCHES[branch]['platforms']['win64-asan']['win10_64_asan'] = deepcopy(BRANCHES[branch]['platforms']['win64']['win10_64'])
+
+ if branch not in trunk_branches:
+ del BRANCHES[branch]['platforms'][platform]
Instead of defining the truck branches you could probably do something like
platforms = ['win32-stylo-disabled', 'win64-stylo-disabled', 'win64-asan']
for name, branch in items_at_least(BRANCHES, 'gecko_version', 57):
for platform in platforms:
then the rest of your if statements, then you don't have the delete them from non-trunk branches
Attachment #8909759 -
Flags: feedback?(kmoir) → feedback+
![]() |
||
Comment 31•8 years ago
|
||
Comment on attachment 8909760 [details]
builder_diff.txt
Jmaher is this the builder list you are expecting? Alin is running into some builder limits so we wanted to ensure that this is correct. Would it make sense to enable these builders on try only first to see if they tests run as expected and then enable on other branches?
Attachment #8909760 -
Flags: feedback?(jmaher)
Reporter | ||
Comment 32•8 years ago
|
||
Comment on attachment 8909760 [details]
builder_diff.txt
a few optimizations:
1) I am not sure about the oak branch - do we need this there?
2) win asan, could be try and mozilla-central only if we need to save builders
3) .*web-platform-tests.* can be ignored, this is VM only
4) .*devtools-chrome.* can be ignored, this is VM only
we don't need to optimize, but those are the quick wins- I could nitpick, but that gets complicated.
Thanks for getting this so far!
Attachment #8909760 -
Flags: feedback?(jmaher) → feedback+
Assignee | ||
Comment 34•8 years ago
|
||
Sorry for the late response, I was hitting some issues while working on this patch since it turns out it's a bit more complicated than it seems.
What I tried to do in this patch is to adjust the builder list to match Joel's optimizations mentioned above.
Flags: needinfo?(aselagea)
Attachment #8910859 -
Flags: review?(kmoir)
Assignee | ||
Comment 35•8 years ago
|
||
Assignee | ||
Comment 36•8 years ago
|
||
(In reply to Kim Moir [:kmoir] ET from comment #30)
> Comment on attachment 8909759 [details] [diff] [review]
> bug1397829-4.patch
> Also, we already have a "stylo-disabled" variant for the talos tests, so I
> wonder if we should only run that variant for the corresponding Win7 and
> Win10 platform where we want stylo disabled.
>
> >>This is a good approach
Tried to do this, but it seems it's a bit more difficult to accomplish that due to the way these talos tests are defined. There were 54 such tests added with my latest patch, so I preferred not to touch them for now.
> Instead of defining the truck branches you could probably do something like
> platforms = ['win32-stylo-disabled', 'win64-stylo-disabled', 'win64-asan']
> for name, branch in items_at_least(BRANCHES, 'gecko_version', 57):
> for platform in platforms:
>
> then the rest of your if statements, then you don't have the delete them
> from non-trunk branches
That was my intention too, but we define the new platforms at the beginning of the script and they are then updated in various places in the code (so it's not like I'm adding then deleting something in the same snippet). Rather than touching various parts of the code to prevent adding builders on these platforms, I preferred to simply remove what's not needed.
![]() |
||
Comment 37•8 years ago
|
||
Comment on attachment 8910860 [details]
builder_diff2.txt
Could you verify these builder changes, you probably have more context than me on the changes required
Attachment #8910860 -
Flags: feedback?(jmaher)
![]() |
||
Comment 38•8 years ago
|
||
Comment on attachment 8910859 [details] [diff] [review]
bug1397829-5.patch
I think this look good to me, but this is dependent on Joel's verification that the builder list looks good. Did you have the adjust the number of builders or did the optimizations reduce it sufficiently?
Attachment #8910859 -
Flags: feedback+
Reporter | ||
Comment 39•8 years ago
|
||
Comment on attachment 8910860 [details]
builder_diff2.txt
this is looking great; possibly we are not at builderlimits with this list?
Looking forward to this landing.
Attachment #8910860 -
Flags: feedback?(jmaher) → feedback+
![]() |
||
Comment 40•8 years ago
|
||
Comment on attachment 8910859 [details] [diff] [review]
bug1397829-5.patch
since Jmaher approved the builder list, I will r+. I think the tools and puppet patches need to be updated as well. Thank you so much Alin! Much appreciated!
Attachment #8910859 -
Flags: review?(kmoir) → review+
Assignee | ||
Comment 41•8 years ago
|
||
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #39)
> Comment on attachment 8910860 [details]
> builder_diff2.txt
>
> this is looking great; possibly we are not at builderlimits with this list?
>
> Looking forward to this landing.
We are not hitting any builder limit with this patch. I think we should also check the TC configs before landing this though.
![]() |
||
Comment 42•8 years ago
|
||
I think you might have to do add the new platforms for talos here so they go through bb bridge
https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/job/mozharness_test.py?q=mozharness_test.py&redirect_type=direct#476
If you get the buildbot changes landed and the builders enabled jmaher can probably run a try run for the tc changes to ensure they work. The buildbot changes won't schedule the tests, just enable the builders.
Assignee | ||
Comment 43•8 years ago
|
||
Comment on attachment 8910859 [details] [diff] [review]
bug1397829-5.patch
https://hg.mozilla.org/build/buildbot-configs/rev/dc9d968744ec
https://hg.mozilla.org/build/buildbot-configs/rev/0f8e75f81fa9
Attachment #8910859 -
Flags: checked-in+
Assignee | ||
Comment 44•8 years ago
|
||
This should adjust the TC buildernames for talos tests so they match the ones existing in BB.
Attachment #8911722 -
Flags: review?(jmaher)
Assignee | ||
Comment 45•8 years ago
|
||
I still believe that we should stick to either "stylo disabled" platforms or "stylo disabled" tests variants. What we have now is a mix of them, which may be confusing.
Assignee | ||
Comment 46•8 years ago
|
||
Updated patch for puppet.
Attachment #8906060 -
Attachment is obsolete: true
Attachment #8906086 -
Attachment is obsolete: true
Attachment #8909759 -
Attachment is obsolete: true
Attachment #8911749 -
Flags: review?(aobreja)
Assignee | ||
Comment 47•8 years ago
|
||
Updated patch for tools.
Attachment #8906069 -
Attachment is obsolete: true
Attachment #8911750 -
Flags: review?(aobreja)
![]() |
||
Comment 48•8 years ago
|
||
Attachment #8911749 -
Flags: review?(aobreja) → review+
![]() |
||
Comment 49•8 years ago
|
||
Attachment #8911750 -
Flags: review?(aobreja) → review+
Assignee | ||
Comment 50•8 years ago
|
||
Comment on attachment 8911749 [details] [diff] [review]
bug1397829_puppet.patch
https://hg.mozilla.org/build/puppet/rev/56074044313e
https://hg.mozilla.org/build/puppet/rev/30a528ab2225
Attachment #8911749 -
Flags: checked-in+
Assignee | ||
Comment 51•8 years ago
|
||
Comment on attachment 8911750 [details] [diff] [review]
bug1397829_tools.patch
https://hg.mozilla.org/build/tools/rev/2f61f0652567
Attachment #8911750 -
Flags: checked-in+
Reporter | ||
Comment 52•8 years ago
|
||
Comment on attachment 8911722 [details] [diff] [review]
adjust_tc_buildernames.patch
Review of attachment 8911722 [details] [diff] [review]:
-----------------------------------------------------------------
this looks nice and simple
Attachment #8911722 -
Flags: review?(jmaher) → review+
Reporter | ||
Updated•8 years ago
|
Assignee: jmaher → aselagea
Comment 53•8 years ago
|
||
Pushed by aselagea@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/23494177c6b2
add buildernames for stylo-disabled tests which run on hardware, win7 and win10, r=jmaher
Comment 54•8 years ago
|
||
bugherder |
![]() |
||
Comment 55•8 years ago
|
||
This work is not yet done, reopening.
Assignee | ||
Comment 56•8 years ago
|
||
Few updates here:
Status:
- created and landed patches to add these buildernames in Buildbot configs and various other places
- created a patch to adjust the buildernames in TaskCluster
=> no new jobs in TH yet
Next steps:
1. Talos:
- we have "stylo-disabled" talos tests that run on the regular platforms
- we also have dedicated "stylo-disabled" platforms, but the above mentioned "stylo-disabled" talos tests don't run there [1]
Should we try to move everything that's "stylo-disabled" to dedicated platforms?
2. do we actually want the new unittests to run via BBB and appear under those dedicated platforms?
[1] https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-searchStr=stylo-disabled
![]() |
||
Updated•8 years ago
|
Component: General → Buildduty
Priority: -- → P1
QA Contact: catlee
Assignee | ||
Comment 57•8 years ago
|
||
Had a meeting with :jmaher where we discussed about the next steps required on this bug.
a) at this point, we have two sets of Windows unittests running on BB - one of them should be "stylo-disabled", but it's being displayed under the wrong platform.
e.g. https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=35fbf14b96a633c3f66ea13c1a163a3f3a4219b9&filter-searchStr=windows%20x64%20reftest
Even though we added the required buildernames above, some pieces are still missing and I will focus on solving that.
b) there's also some confusion regarding the way "stylo-disabled" talos tests are being run:
- they should be displayed under the appropriate platform
- we shouldn't run regular talos tests for "stylo-disabled" platforms
Will file a new bug to address that.
![]() |
||
Comment 58•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=35fbf14b96a633c3f66ea13c1a163a3f3a4219b9&filter-searchStr=windows%20x64%20reftest&group_state=expanded is a good example of duplicate jobs that need fixing.
Comment 59•8 years ago
|
||
(In reply to Alin Selagea [:aselagea][:buildduty] from comment #57)
> a) at this point, we have two sets of Windows unittests running on BB - one
> of them should be "stylo-disabled", but it's being displayed under the wrong
> platform.
> e.g.
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-
> central&revision=35fbf14b96a633c3f66ea13c1a163a3f3a4219b9&filter-
> searchStr=windows%20x64%20reftest
> Even though we added the required buildernames above, some pieces are still
> missing and I will focus on solving that.
>
> b) there's also some confusion regarding the way "stylo-disabled" talos
> tests are being run:
> - they should be displayed under the appropriate platform
> - we shouldn't run regular talos tests for "stylo-disabled" platforms
> Will file a new bug to address that.
Now that we have bug 1403600 for the stylo-disabled talos tests and platform names, can this bug be closed?
status-firefox55:
--- → unaffected
status-firefox56:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Assignee | ||
Comment 60•8 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #59)
> Now that we have bug 1403600 for the stylo-disabled talos tests and platform
> names, can this bug be closed?
No, this is still needed since we also want the regular unittests to run on the corresponding "stylo-disabled" platform.
Assignee | ||
Comment 61•8 years ago
|
||
Spent some time investigating this and it seems there are a couple of problems we need to fix before getting this work done.
First of them is a builder limit on BB: "AssertionError: t-w1064-ix-156 has 4099 builders; limit is 4084". Not sure why we hit it again (maybe due to further pushes in bb-configs since my last push), but since we'll also need to deal with bug 1403600 soon, we could take care of it first so that we don't run regular talos tests on "stylo-disabled" platforms. That will save us some builders and get us past the builder limit.
Assignee | ||
Comment 62•8 years ago
|
||
The test platforms defined in TC [1], have "stylo-disabled" in their name. When we retrieve the test variant, we use this method [2]. Since we initially specified "stylo disabled" as variant, we won't get any change in the builder name as that variant can't be found on the platform name.
I also updated the configs to account for the changes required when computing the builder names for the unit tests as well.
[1] https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/test/test-platforms.yml#85-132
[2] https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/job/mozharness_test.py#70-74
@Kim: I'll address this review request to you. Please feel free to redirect it if you consider it necessary/you're too busy with release duty.
Attachment #8913629 -
Flags: review?(kmoir)
Assignee | ||
Comment 63•8 years ago
|
||
Buildbot will need these small changes as well.
NOTE: these last two patches will need to land *before* the patches that will come up in bug 1403600.
Attachment #8913630 -
Flags: review?(aobreja)
![]() |
||
Updated•8 years ago
|
Attachment #8913630 -
Flags: review?(aobreja) → review+
![]() |
||
Comment 64•8 years ago
|
||
Comment on attachment 8913629 [details] [diff] [review]
bug1397829_fix_tc_buildernames.patch
Thanks you both for working on these patches. We really appreciate all your work to test and implement this functionality. I also appreciate you picking up this work while I was on releaseduty.
Attachment #8913629 -
Flags: review?(kmoir) → review+
Comment 65•8 years ago
|
||
Pushed by aselagea@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/349550665585
Fix bb buildernames for 'stylo-disabled' tests, r=kmoir
Comment 66•8 years ago
|
||
bugherder |
Assignee | ||
Updated•8 years ago
|
Attachment #8913629 -
Flags: checked-in+
Assignee | ||
Comment 67•8 years ago
|
||
Comment on attachment 8913630 [details] [diff] [review]
bug1397829_fix_bb_buildernames.patch
https://hg.mozilla.org/build/buildbot-configs/rev/25fb86bf5638
https://hg.mozilla.org/build/buildbot-configs/rev/5af72c62a6a9
Attachment #8913630 -
Flags: checked-in+
Assignee | ||
Comment 68•8 years ago
|
||
It seems I omitted to update the variant name in one place, here's a small fix for that.
Attachment #8914666 -
Flags: review?(aobreja)
![]() |
||
Comment 69•8 years ago
|
||
Comment on attachment 8914666 [details] [diff] [review]
fix_test_variant.patch
Looks good
Attachment #8914666 -
Flags: review?(aobreja) → review+
Comment 70•8 years ago
|
||
Pushed by aselagea@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a938d79a7ef8
fix variant name for 'stylo-disabled', r=aobreja
Assignee | ||
Updated•8 years ago
|
Attachment #8914666 -
Flags: checked-in+
Assignee | ||
Comment 71•8 years ago
|
||
Comment on attachment 8914666 [details] [diff] [review]
fix_test_variant.patch
I had to back out this change since the "stylo-disabled" tests scheduled in TC were not starting in BB. Will investigate what else is needed and eventually test on try before landing this again.
Attachment #8914666 -
Flags: checked-in+ → checked-in-
Assignee | ||
Comment 72•8 years ago
|
||
On a closer look, it seems those "stylo-disabled" unittests won't start since they're not on the bb masters - that is because the reconfigs fail with the following error:
cd master && /builds/buildbot/tests1-windows/bin/buildbot checkconfig
Traceback (most recent call last):
File "/builds/buildbot/tests1-windows/lib/python2.7/site-packages/buildbot-0.8.2_hg_d769fbdcf8ee_production_0.8-py2.7.egg/buildbot/scripts/runner.py", line 1042, in doCheckConfig
ConfigLoader(configFileName=configFileName)
File "/builds/buildbot/tests1-windows/lib/python2.7/site-packages/buildbot-0.8.2_hg_d769fbdcf8ee_production_0.8-py2.7.egg/buildbot/scripts/checkconfig.py", line 31, in __init__
self.loadConfig(configFile, check_synchronously_only=True)
File "/builds/buildbot/tests1-windows/lib/python2.7/site-packages/buildbot-0.8.2_hg_d769fbdcf8ee_production_0.8-py2.7.egg/buildbot/master.py", line 652, in loadConfig
exec f in localDict
File "/builds/buildbot/tests1-windows/master/master.cfg", line 94, in <module>
c['slaves'].append(BuildSlave(name, SlavePasswords[slave_platform], max_builds=1,
KeyError: 'win7_ix_stylo_disabled'
make: *** [checkconfig] Error 1
This patch adds an entry for 'win7_ix_stylo_disabled' in Puppet.
Attachment #8915040 -
Flags: review?(aobreja)
![]() |
||
Updated•8 years ago
|
Attachment #8915040 -
Flags: review?(aobreja) → review+
Assignee | ||
Comment 73•8 years ago
|
||
Comment on attachment 8915040 [details] [diff] [review]
bug1397829_2_puppet.patch
Landed this patch, checked that the Windows bb masters reconfigured successfully and can confirm these tests are now present on the said masters.
Did a try push [1] and I can see "stylo-disabled" unittests being run via BBB and having the correct builder names. They will be moved to the correct platform in TH once bug is fixed.
[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=efb95a9cacb2e0198a67066e57ad1b88e08adfce&filter-searchStr=stylo-disabled
Attachment #8915040 -
Flags: checked-in+
Assignee | ||
Comment 74•8 years ago
|
||
According to [1], we can leave the "stylo-disabled" talos tests to run as they do now and not move them to separate platforms. Regular talos tests on "stylo-disabled" platforms are also not needed, so let's just remove them from BB to reduce the builder count.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1403600#c17
Attachment #8915610 -
Flags: review?(aobreja)
Assignee | ||
Comment 75•8 years ago
|
||
Once we have the TH changes (bug 1404303) and the patch above landed, we can go on and re-land https://bugzilla.mozilla.org/attachment.cgi?id=8914666 above to have these unittests run under on the corresponding platform.
![]() |
||
Comment 76•8 years ago
|
||
Comment on attachment 8915610 [details] [diff] [review]
bug1397829_remove_talos.patch
cat old | grep stylo-disabled | wc -l
1315
cat new | grep stylo-disabled | wc -l
959
Attachment #8915610 -
Flags: review?(aobreja) → review+
Assignee | ||
Comment 77•8 years ago
|
||
Comment on attachment 8915610 [details] [diff] [review]
bug1397829_remove_talos.patch
https://hg.mozilla.org/build/buildbot-configs/rev/1002bebad59b
https://hg.mozilla.org/build/buildbot-configs/rev/38201b6b7f12
Attachment #8915610 -
Flags: checked-in+
Comment 78•8 years ago
|
||
Pushed by aselagea@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6050fffa0c5
add buildernames for stylo-disabled tests which run on hardware, r=aobreja
Assignee | ||
Updated•8 years ago
|
Attachment #8914666 -
Flags: checked-in- → checked-in+
Assignee | ||
Comment 79•8 years ago
|
||
Okay, so all Treeherder, BB and TC changes mentioned above are now landed. Here's a push to m-i showing the "stylo-disabled" tests running at this point:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=f232c635ecd51d1557683b0457c926e7e2f23128&filter-searchStr=stylo%20disabled
Is there anything else needed here?
As for the Windows 10 asan jobs, I checked the list of tasks and noticed we don't run any asan job for Windows. If those are still required to be enabled, we could probably treat that in a separate bug.
Flags: needinfo?(jmaher)
Reporter | ||
Comment 80•8 years ago
|
||
this is great, win-asan is try only and can be handled with a different bug.
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Flags: needinfo?(jmaher)
Resolution: --- → FIXED
Comment 81•8 years ago
|
||
bugherder |
Comment 82•8 years ago
|
||
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Updated•7 years ago
|
Product: Release Engineering → Infrastructure & Operations
Updated•6 years ago
|
Product: Infrastructure & Operations → Infrastructure & Operations Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•