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

RESOLVED FIXED

Status

Release Engineering
Mozharness
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ahal, Assigned: ahal)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed, b2g-master affected)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Assignee)

Description

2 years ago
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)

Updated

2 years ago
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
Created attachment 8648877 [details]
MozReview Request: Bug 1181261 - Merge configs from testing/config/mozharness into mozharness proper, r=chmanchester

Bug 1181261 - Remove in-tree configs from mozharness, r=chmanchester
Attachment #8648877 - Flags: review?(cmanchester)
(Assignee)

Comment 2

2 years ago
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+
(Assignee)

Comment 4

2 years ago
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.
(Assignee)

Comment 5

2 years ago
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
(Assignee)

Comment 6

2 years ago
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)
(Assignee)

Comment 7

2 years ago
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.
(Assignee)

Comment 8

2 years ago
Oops, try run uncovered bustage:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2080990ce070

Pushed fix, here's a new try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e78c25206fdc
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+
(Assignee)

Comment 10

2 years ago
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
(Assignee)

Comment 11

2 years ago
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.
(Assignee)

Comment 12

2 years ago
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).

Comment 13

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/28fa968ea46c
This broke at least mulet tests, so backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/639a28373b05

https://treeherder.mozilla.org/logviewer.html#?job_id=13142665&repo=mozilla-inbound
Flags: needinfo?(ahalberstadt)
(Assignee)

Comment 15

2 years ago
Looks like mulet tests don't get run with try: -a:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c39d80aa410

Hooray..
Flags: needinfo?(ahalberstadt)
(Assignee)

Comment 16

2 years ago
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.
(Assignee)

Updated

2 years ago
Depends on: 1188330
No longer depends on: 1131856
(Assignee)

Updated

2 years ago
Depends on: 1131856
(Assignee)

Comment 17

2 years ago
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.
(Assignee)

Comment 18

2 years ago
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]

Comment 19

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/473c927692ca
https://hg.mozilla.org/mozilla-central/rev/473c927692ca
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
(Assignee)

Comment 22

2 years ago
Created attachment 8652319 [details] [diff] [review]
Bustage fix for hidden Mnw tests

Sorry about that, didn't catch it in the try run as it was hidden. This patch should fix it.
Attachment #8652319 - Flags: review+

Comment 23

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/836b81148458
https://hg.mozilla.org/mozilla-central/rev/836b81148458
(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)
(Assignee)

Comment 26

2 years ago
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)

Comment 27

2 years ago
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
(Assignee)

Comment 28

2 years ago
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)

Comment 30

2 years ago
(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.
(Assignee)

Comment 31

2 years ago
(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)

Comment 32

2 years ago
https://hg.mozilla.org/integration/b2g-inbound/rev/ca3986781f87
(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

Comment 34

2 years ago
https://hg.mozilla.org/integration/b2g-inbound/rev/fa68da43bac9
(Assignee)

Comment 35

2 years ago
Re-added marionette.py as well.
https://hg.mozilla.org/mozilla-central/rev/fa68da43bac9
https://hg.mozilla.org/mozilla-central/rev/ca3986781f87
Depends on: 1205934
Created attachment 8663478 [details]
MozReview Request: Bug 1181261 - Remove remaining in-tree mozhanress configs. r?ahal

Bug 1181261 - Remove remaining in-tree mozhanress configs. r?ahal
Attachment #8663478 - Flags: review?(ahalberstadt)
Try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=099a93de7c40&exclusion_profile=false&group_state=expanded
(Assignee)

Comment 40

2 years ago
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+
(Assignee)

Comment 41

2 years ago
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]

Comment 44

2 years ago
https://hg.mozilla.org/integration/b2g-inbound/rev/ad28fa105c30
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ad28fa105c30
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.