Closed Bug 1035551 Opened 6 years ago Closed 6 years ago

Add in_tree_config support for Marionette


(Testing :: Marionette, defect)

Not set


(firefox30 wontfix, firefox31 fixed, firefox32 fixed, firefox33 fixed, firefox-esr24 fixed, firefox-esr31 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

Tracking Status
firefox30 --- wontfix
firefox31 --- fixed
firefox32 --- fixed
firefox33 --- fixed
firefox-esr24 --- fixed
firefox-esr31 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed


(Reporter: gps, Assigned: chmanchester)



(Whiteboard: [leave-open])


(3 files, 3 obsolete files)

The mozharness Marionette configs don't currently define in_tree_config. We need this to help roll out bug 940954.
We'll be adding a new argument to the Marionette CLI driver in bug
940954. We'll need to have mozharness pass this argument to get tests to
work properly. To facilitate this, we're adding an in-tree Marionette
config that will add this argument.

Reviewer Note: I'm not sure if I changed too many or too few of the
Marionette config files to now include an in-tree config. Also, I'm
pretty sure mozharness fails if the in-tree config file isn't present.
So I guess I need to add an empty file to all the trees?

As much as I wanted to use the existing,,
etc config files, marionette seems to share config files between
platforms. I'm not sure if I should be doing os.platform() foo inside
the config file or what. That's what review is for!
Attachment #8452776 - Flags: review?(jgriffin)
Assignee: nobody → gps
Comment on attachment 8452776 [details] [diff] [review]
Support in-tree configs for Marionette

Review of attachment 8452776 [details] [diff] [review]:

Attachment #8452776 - Flags: review?(jgriffin) → review+
Blocks: 1037087
Duplicate of this bug: 1037087
Chris: If you want to steal this from me, go right ahead. I'll be side-tracked for the next few days dealing with Firefox foo.
Blocks: 1036954
Assignee: gps → cmanchester
Chris, you'll still need to pull the command line arguments out of and into the config. Also, all the other harness' in-tree configs live under testing/config/mozharness, can we put this file alongside those?
This is looking good on ash.
Attachment #8454559 - Flags: review?(ahalberstadt)
This is the config file to be used.
Attachment #8455411 - Flags: review?(ahalberstadt)
Attachment #8454559 - Attachment is obsolete: true
Attachment #8454559 - Flags: review?(ahalberstadt)
These are the modifications to the mozharness script to read the in-tree config.
Attachment #8455413 - Flags: review?(ahalberstadt)
Comment on attachment 8455411 [details] [diff] [review]
Add an in-tree config for marionette tests.

Review of attachment 8455411 [details] [diff] [review]:

Looks good!
Attachment #8455411 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8455413 [details] [diff] [review]
Add support for in tree configs in mozharness in script.

Review of attachment 8455413 [details] [diff] [review]:

Nice, thanks! So I see you are going the 'land on every tree' route. I agree this is a good idea since this time all you are doing is adding a file that should get automatically copied over since the ground work was already laid out by bug 981030. If you ask one of the sheriffs nicely, they might be able to help you uplift to all of the older branches since they do uplifts there once in awhile anyway. Failing that, I can help you do it.

One thing to note, people who haven't re-based their Mn try runs will hit a failure. Luckily from last time an appropriate error message should already get printed if this happens:

We'll just want to update the referenced bug to note that Mn was recently changed.
Attachment #8455413 - Flags: review?(ahalberstadt) → review+
Here's a version of the config suitable for all branches (doesn't add the "--log-raw" option).

As we did in bug 981030, we need to land the new config on every tree so that the mozharness change doesn't cause bustage. ahal recommended I alert a sheriff to see about how to do this.

The attachment in comment 11 is intended to land on all the trees.

Ryan, what do you think?
Flags: needinfo?(ryanvm)
Flags: needinfo?(ryanvm)
Attachment #8452776 - Attachment is obsolete: true
Attachment #8455507 - Flags: review+
Updated error message to point to this bug.

Attachment #8455413 - Attachment is obsolete: true
Attachment #8455604 - Flags: review+
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Resolution: FIXED → ---
Whiteboard: [leave-open]
No longer blocks: 1037087
This has been on central about a day, let's get the ball rolling. 

Ryan, could you land the patch from comment 11 on all trees when you have an opportunity?
Flags: needinfo?(ryanvm)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #17)

This is the config with the new option for logging, so will cause bustage once we land the mozharness changes. We need the config from comment 11 without this option, not the one checked into central:

I know this is a hassle, sorry for the confusion here!
Flags: needinfo?(ryanvm)
Here is a green cypress run with the above patch included:
Depends on: 1042842
Re-triggers from this morning on the older trees look ok, except for release where there isn't anything recent enough on release I can re-trigger with all the changes picked up.
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.