Open Bug 1188730 Opened 4 years ago Updated 11 months ago

Fix mach try's interaction with mochitest-oth

Categories

(Firefox Build System :: Try, defect)

defect
Not set

Tracking

(firefox42 fixed)

REOPENED
Tracking Status
firefox42 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

Details

Attachments

(2 files)

M(oth) is three suites run in sequence by mozharness: plugins, a11y, and chrome. Chrome mochitests are integrated into the build system and have their own master manifest, so mach try knows about them and adds 'mochitest-oth' to try syntax to make them run. In the case of a11y the entire suite just runs next to chrome (these test have their own master manifest), but in the case of plugins the entire job will turn red unless the selected tests overlap (the tests are selected by path, here: https://hg.mozilla.org/mozilla-central/annotate/2ddec2dedced/testing/mochitest/mochitest_options.py#l555 ).

There are a few potential fixes depending on where we think the problem is (multiple ways to select tests for a suite that get out of sync is a problem, mozharness' inability to detect when a user's selection results in no tests, obscuring the difference between suite results in a single chunk, etc.), and future changes will probably make this problem obsolete. But in the short term this is very confusing so we should probably fix it either by running all or none of the plugins tests alongside selected chrome tests.
Bug 1188730 - Select tests in the plugins subsuite by manifest rather than path to fix unintended interaction with mach try. r=ahal

This prevents mach try's filtering of the master mochitest from interacting
with the plugins subsuite by selecting by manifest rather than test path.
The way we run mochitest other could use a lot of cleanup -- this is a
quick fix to prevent misleading output on treeherder when using mach try.
Attachment #8640598 - Flags: review?(ahalberstadt)
Assignee: nobody → cmanchester
Comment on attachment 8640598 [details]
MozReview Request: Bug 1188730 - Select tests in the plugins subsuite by manifest rather than path to fix unintended interaction with mach try. r=ahal

https://reviewboard.mozilla.org/r/14341/#review13063

It's really nice to review this in one patch :). Thanks for removing --ipcplugins.

I'm not sure how many people run ipcplugins tests locally, but this might break some workflows. I don't think it's a big deal, as they can just run |mach mochitest dom/plugins| instead.
Attachment #8640598 - Flags: review?(ahalberstadt) → review+
(In reply to Andrew Halberstadt [:ahal] from comment #3)
> I'm not sure how many people run ipcplugins tests locally, but this might
> break some workflows. I don't think it's a big deal, as they can just run
> |mach mochitest dom/plugins| instead.

That doesn't actually do what you'd want, since the point of ipcplugins is to run those tests with the plugins running in-process, but --ipcplugins didn't actually do that for you either, so that's immaterial.

We do apparently have "mach test mochitest-ipcplugins", but that just punts to running the make target: https://dxr.mozilla.org/mozilla-central/source/testing/mach_commands.py#83
Ah right, it's unlikely anyone was using --ipcplugins locally in that case. Is there a reason 'ipcplugins' isn't a formal flavor? (I'm probably getting off topic here)
It's just a hack we whipped up back when out of process plugins were being developed, I don't think it's ever had much more thought given to it. I asked bsmedberg if we could remove it, and it sounded like something we might be able to do.
https://hg.mozilla.org/mozilla-central/rev/41dadd801de3
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Bug 1188730 - Exit with a warning and status code 0 if we don't find any mochitests to run.

This can happen, in particular with the M-oth job on buildbot in combination
with test selection through |mach try| without being an error. That job is a
particular problem because it runs several mochitest-based suites of which only
a subset may have tests selected
Attachment #8646452 - Flags: review?(cmanchester)
Comment on attachment 8646452 [details]
MozReview Request: Bug 1188730 - Exit with a warning and status code 0 if we don't find any mochitests to run.

https://reviewboard.mozilla.org/r/15769/#review14033

::: testing/mochitest/runtests.py:1948
(Diff revision 1)
> -                self.log.error("no tests to run using specified "
> +                self.log.warning("no tests to run using specified "
> -                               "combination of filters: {}".format(
> +                                 "combination of filters: {}".format(
> -                                    manifest.fmt_filters()))
> +                                     manifest.fmt_filters()))
> +                sys.exit(0)

Did you test this? Mozharness sets job status based on how many tests are run, and 0 is always too few to get a passing run, so I doubt this will fix the issue.

But this bug should be re-opened, we still have this problem pretty much every time tests are selected by tag with mach try.
Attachment #8646452 - Flags: review?(cmanchester)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8646452 [details]
MozReview Request: Bug 1188730 - Exit with a warning and status code 0 if we don't find any mochitests to run.

Bug 1188730 - Exit with a warning and status code 0 if we don't find any mochitests to run.

This can happen, in particular with the M-oth job on buildbot in combination
with test selection through |mach try| without being an error. That job is a
particular problem because it runs several mochitest-based suites of which only
a subset may have tests selected
Attachment #8646452 - Flags: review?(cmanchester)
Attachment #8646452 - Flags: review?(cmanchester)
Comment on attachment 8646452 [details]
MozReview Request: Bug 1188730 - Exit with a warning and status code 0 if we don't find any mochitests to run.

https://reviewboard.mozilla.org/r/15769/#review15775

Nothing to add here.
Component: General → Try
Product: Testing → Firefox Build System
Target Milestone: mozilla42 → ---
You need to log in before you can comment on or make changes to this bug.