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: