Closed Bug 1181261 Opened 9 years ago Closed 9 years ago

Remove concept of in-tree mozharness configs; merge existing in-tree configs with normal ones

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

Product:
Release Engineering
Component:
Applications: MozharnessCore
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox44 fixed, b2g-master affected)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox44 --- fixed
b2g-master --- affected

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(3 files)

MozReview Request: Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester
40 bytes, text/x-review-board-request
chmanchester
: review+
Details
Bustage fix for hidden Mnw tests
1.76 KB, patch
ahal
: review+
Details | Diff | Splinter Review
MozReview Request: Bug 1181261 - Remove remaining in-tree mozhanress configs. r?ahal
40 bytes, text/x-review-board-request
ahal
: review+
Details
Mozharness itself and all its configs will soon be in-tree. Once that happens, there will be no point in maintaining separate "in-tree" mozharness configs (all configs will be in-tree!). We should merge the current in-tree configs with the normal configs, and remove any logic related to them from mozharness core and various scripts.
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Bug 1181261 - Remove in-tree configs from mozharness, r=chmanchester
Attachment #8648877 - Flags: review?(cmanchester)
Here's a working try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c39d80aa410 Doesn't seem like I missed anything.. but there's so many configs, it's hard to tell. Sorry for the large patch! (Most of it is just copy/paste from one file to another)
Comment on attachment 8648877 [details] MozReview Request: Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester https://reviewboard.mozilla.org/r/16315/#review14813 Lots of great cleanup in here! ::: testing/mozharness/mozharness/mozilla/testing/testbase.py:446 (Diff revision 1) > - def _read_tree_config(self): > + def _read_try_config(self): No big deal, but we could move this to mozharness too if we wanted.
Attachment #8648877 - Flags: review?(cmanchester) → review+
https://reviewboard.mozilla.org/r/16315/#review14813 > No big deal, but we could move this to mozharness too if we wanted. Good idea, might as well and then we can delete the config/mozharness directory entirely.
Comment on attachment 8648877 [details] MozReview Request: Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester The config files under testing/config/mozharness were created so that certain mozharness options such as test harness arguments could ride the trees, simplifying a lot of logic in mozharness. But now that mozharness itself is in-tree, these configs no longer serve any purpose. Instead they are merged into the main configs at testing/mozharness/configs.
Attachment #8648877 - Attachment description: MozReview Request: Bug 1181261 - Remove in-tree configs from mozharness, r=chmanchester → MozReview Request: Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester
Comment on attachment 8648877 [details] MozReview Request: Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester Hmm, I can't figure out how to get mozreview to re-flag you, so doing it here. You should probably take a look at the second diff on the slider. I moved try_arguments.py into TryToolsMixin.
Attachment #8648877 - Flags: review+ → review?(cmanchester)
Comment on attachment 8648877 [details] MozReview Request: Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester The config files under testing/config/mozharness were created so that certain mozharness options such as test harness arguments could ride the trees, simplifying a lot of logic in mozharness. But now that mozharness itself is in-tree, these configs no longer serve any purpose. Instead they are merged into the main configs at testing/mozharness/configs.
Comment on attachment 8648877 [details] MozReview Request: Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester https://reviewboard.mozilla.org/r/16315/#review14925 Much better, thank you for doing that.
Attachment #8648877 - Flags: review?(cmanchester) → review+
Not really sure why, but jobs didn't get scheduled on that last try run :/.. Here's a new one that worked: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0634eb3619d8
Comment on attachment 8648877 [details] MozReview Request: Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester The config files under testing/config/mozharness were created so that certain mozharness options such as test harness arguments could ride the trees, simplifying a lot of logic in mozharness. But now that mozharness itself is in-tree, these configs no longer serve any purpose. Instead they are merged into the main configs at testing/mozharness/configs.
Latest push also removes testing/config/mozharness/taskcluster_linux_config.py. I talked to :garndt, and it wasn't being used anymore. It also re-adds the if statement that makes sure this code only runs on try. Oops, that would have been bad. I don't think this requires re-review (and the try push above has these changes included).
Looks like mulet tests don't get run with try: -a: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c39d80aa410 Hooray..
Flags: needinfo?(ahalberstadt)
Oh! Taskcluster doesn't use in-tree mozharness yet. Ironically the push that makes builds use in-tree mozharness landed three pushes after mine. But it was still broken, so guess we have to do something similar for tests.
Depends on: 1188330
No longer depends on: 1131856
Depends on: 1131856
Comment on attachment 8648877 [details] MozReview Request: Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester The config files under testing/config/mozharness were created so that certain mozharness options such as test harness arguments could ride the trees, simplifying a lot of logic in mozharness. But now that mozharness itself is in-tree, these configs no longer serve any purpose. Instead they are merged into the main configs at testing/mozharness/configs.
Rather than wait for bug 1188330, which I'm not sure has an ETA yet, the above patch simply doesn't delete linux_config.py or linux_mulet_config.py yet. It adds a comment to each saying they aren't being used except for mulet. Here's a try run that proves it fixes the mulet tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0250b4ba8933 Once bug 1188330 is fixed, they can be moved. I'll re-use this bug for tackling that.
Whiteboard: [leave-open]
Unfortunately, now we get the following error when running the marionette test case in treeherder: Traceback (most recent call last): File "scripts/scripts/marionette.py", line 506, in <module> marionetteTest = MarionetteTest() File "scripts/scripts/marionette.py", line 174, in __init__ config={'require_test_zip': True}) File "/builds/slave/test/scripts/mozharness/base/vcs/vcsbase.py", line 116, in __init__ super(VCSScript, self).__init__(**kwargs) File "/builds/slave/test/scripts/mozharness/base/script.py", line 1473, in __init__ rw_config = ConfigClass(config_options=config_options, **kwargs) File "/builds/slave/test/scripts/mozharness/base/config.py", line 265, in __init__ self.parse_args(args=option_args) File "/builds/slave/test/scripts/mozharness/base/config.py", line 481, in parse_args options.config_files + options.opt_config_files, options=options File "/builds/slave/test/scripts/mozharness/base/config.py", line 446, in get_cfgs_from_files all_cfg_files_and_dicts.append((cf, parse_config_file(cf))) File "/builds/slave/test/scripts/mozharness/base/config.py", line 159, in parse_config_file execfile(file_path, global_dict, local_dict) File "/builds/slave/test/scripts/scripts/../configs/marionette/automation_emulator_config.py", line 44 "suite_definitions": { ^ IndentationError: unexpected indent program finished with exit code 1 http://ftp.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/mozilla-central-emulator/1440464312/mozilla-central_ubuntu64_vm-b2g-emulator_test-marionette-webapi-bm118-tests1-linux64-build37.txt.gz
Sorry about that, didn't catch it in the try run as it was hidden. This patch should fix it.
Attachment #8652319 - Flags: review+
(In reply to Andrew Halberstadt [:ahal] from comment #16) > Oh! Taskcluster doesn't use in-tree mozharness yet. Ironically the push that > makes builds use in-tree mozharness landed three pushes after mine. But it > was still broken, so guess we have to do something similar for tests. Hi :ahal, do you know is there any existing bug tracking on using in-tree mozharness on taskcluster? If no, I'd like to file one for it. Thank you.
Flags: needinfo?(ahalberstadt)
The patch for taskcluster builds using in-tree mozharness landed recently. The bug for taskcluster tests using in-tree mozharness is bug 1188330.
Flags: needinfo?(ahalberstadt)
Hi Andrew, Bug 1188330 is currently the major blocker for emulator-x86-KK. We do need the fix. Thank you!
Severity: normal → critical
blocking-b2g: --- → 2.5+
status-b2g-master: --- → affected
Priority: -- → P1
Hey Josh, I think you have it mixed up. This bug isn't blocking anything. The only work left to do here is remove the two files under testing/config/mozharness once bug 1188330 gets fixed. I'm not sure what you are blocked on, but it certainly isn't this. As for making taskcluster tests use the in-tree mozharness, I'd ask over in bug 1188330, that's not something that I know how to do.
Severity: critical → normal
blocking-b2g: 2.5+ → ---
Priority: P1 → --
hey andrew, sorry to make you double check, I noticed that this bug removed config/mozharness/b2g_emulator_config.py. it looks like that config is still being pointed at by something for x86-kk or at least that's what within https://bugzil.la/1188330 suggests. is it possible this config was deleted from testing/config/ and not ported to the in-tree /testing/mozharness ?
Flags: needinfo?(ahalberstadt)
(In reply to Andrew Halberstadt [:ahal] from comment #28) > Hey Josh, I think you have it mixed up. This bug isn't blocking anything. > The only work left to do here is remove the two files under > testing/config/mozharness once bug 1188330 gets fixed. I'm not sure what you > are blocked on, but it certainly isn't this. > > As for making taskcluster tests use the in-tree mozharness, I'd ask over in > bug 1188330, that's not something that I know how to do. Hi Andrew, Thanks for the info. I will check with Jonathan for who can help here.
(In reply to Jordan Lund (:jlund) from comment #29) > hey andrew, sorry to make you double check, I noticed that this bug removed > config/mozharness/b2g_emulator_config.py. it looks like that config is still > being pointed at by something for x86-kk or at least that's what within > https://bugzil.la/1188330 suggests. is it possible this config was deleted > from testing/config/ and not ported to the in-tree /testing/mozharness ? Thanks for double checking, it didn't sound like anything was broken from the comments above. It's not that it wasn't ported over, it's that taskcluster tests don't use the in-tree mozharness yet. I'll re-add that file for now.
Flags: needinfo?(ahalberstadt)
(In reply to Andrew Halberstadt [:ahal] from comment #31) > (In reply to Jordan Lund (:jlund) from comment #29) > > hey andrew, sorry to make you double check, I noticed that this bug removed > > config/mozharness/b2g_emulator_config.py. it looks like that config is still > > being pointed at by something for x86-kk or at least that's what within > > https://bugzil.la/1188330 suggests. is it possible this config was deleted > > from testing/config/ and not ported to the in-tree /testing/mozharness ? > > Thanks for double checking, it didn't sound like anything was broken from > the comments above. It's not that it wasn't ported over, it's that > taskcluster tests don't use the in-tree mozharness yet. I'll re-add that > file for now. Hi Andrew, I still observed the error below happening to marionette and marionette-webapi tests [1] and learnt that your push in comment 32 seemed not cover those two. Could you take a look again? == error == The in-tree configuration file '/home/worker/build/tests/config/mozharness/marionette.py' does not exist!It must be added to 'gecko/testing/config/mozharness/marionette.py'. See bug 1035551 for more details. [1] https://treeherder.allizom.org/#/jobs?repo=try&revision=d472039e6e52
Re-added marionette.py as well.
Depends on: 1205934
Bug 1181261 - Remove remaining in-tree mozhanress configs. r?ahal
Attachment #8663478 - Flags: review?(ahalberstadt)
Comment on attachment 8663478 [details] MozReview Request: Bug 1181261 - Remove remaining in-tree mozhanress configs. r?ahal https://reviewboard.mozilla.org/r/19759/#review17861 Thanks for doing this! So that means that taskcluster test jobs are using the in-tree mozharness now?
Attachment #8663478 - Flags: review?(ahalberstadt) → review+
Your try push missed mulet btw, might want to double check that just in case.
(In reply to Andrew Halberstadt [:ahal] from comment #40) > Comment on attachment 8663478 [details] > MozReview Request: Bug 1181261 - Remove remaining in-tree mozhanress > configs. r?ahal > > https://reviewboard.mozilla.org/r/19759/#review17861 > > Thanks for doing this! So that means that taskcluster test jobs are using > the in-tree mozharness now? Yes, now all taskcluster tests are using the in-tree one now (bug 1188330).
(In reply to Andrew Halberstadt [:ahal] from comment #41) > Your try push missed mulet btw, might want to double check that just in case. Try runs with Mulet: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c1087a8f664&exclusion_profile=false&group_state=expanded
Keywords: checkin-needed
Whiteboard: [leave-open]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: