Open Bug 1337844 Opened 7 years ago Updated 1 year ago

make reproducing test failures in context of automation much easier

Categories

(Testing :: Mochitest, defect)

Product:
Testing
Component:
Mochitest
Type:
defect

Tracking

(Not tracked)

People

(Reporter: jmaher, Unassigned)

References

(Blocks 1 open bug)

Details

I think this is a large pain point with a few areas to invest in:
1) one-click loaners (linux/android) and the simpler mach interface there
2) local dev environment and the normal mach interface there

right now the mochitest harness has dozens of special purpose command line flags, these are confusing if not misleading to say the least.  In automation we use many of these, but they serve little to no purpose locally, except a few which are critical.  mach is supposed to take care of all this- but we should make it clear what options do what and how to use them.

To get to the point of this bug, we can reproduce failures this way:
./mach mochitest browser/base/content/test/general/browser_bug559991.js

or for the full directory:
./mach mochitest browser/base/content/test/general

the problem with the full directory is that all subdirectories and other test types will run- in fact this is by design so that you can test all the tests for the code you are working on...but this doesn't help for the case when investigating a failure.

So we would like to find a way to make it easier to run just the single directory that we care about, i.e. to run what we do in automation.

to do this we need:
* proper command line arguments
* edits to the harness (more cli arguments) so we can run the specific directory

In this case we care about a manifest (not directory) which is a .ini file.  We also need to not run other .ini files which are included as sub manifests.

To solve this bug I would like to:
* ensure/add support for running a directory via direct manifest |mach mochitest <path>/mochitest.ini|
* all other command line arguments to mach to reproduce a run in automation should live in the log from automation- to do this the mochitest harness would print out a "mach command line".
* consider removing the directory/manifest.ini from the command line and let the developer fill in the blank
* consider a cli flag to shortcut some of these options so we can give a specific test case and run it in context, i.e. |./mach mochitest browser/base/content/test/general/browser_bug559991.js --automation| <- a horrible name, but conveys the idea.
> the problem with the full directory is that all subdirectories and other test types will run- in fact this is by design so that you can test all the tests for the code you are working on...but this doesn't help for the case when investigating a failure.

Do we think this is a thing people actually want? I guess we only provide the one mochitest command so people wanting to run chrome or browser-chrome tests would have to specify `--flavor`, but maybe that's OK?

Maybe at the very least we could have the harness detect if there are multiple flavors of mochitest in that directory and do something better? I don't know that there's a great default behavior here, but it could just refuse to run multiple flavors and print "This path contains mochitests of flavor (plain, browser-chrome, ...). You should specify which flavor to run with `--flavor=foo`." We could add a `--flavor=all` if someone really wanted the existing behavior.

Anyway, I think just printing a "run this mach command to run this directory of tests locally" in the harness output in automation would make life a lot better. mozharness prints this sort of output and I use it fairly often. Before running each directory we could just print like:
```
Copy and paste the following command to run the next set of tests locally:
./mach mochitest --flavor=foo <whatever other options are required> path/to/dir_or_manifest
```
A couple points to add:
* Mochitest can already run specific manifests with the --manifest argument. We could simplify this a bit by removing --manifest, and instead automatically interpreting paths that end in .ini as a manifest (low hanging fruit).

* I don't think we should default to only running one flavor/subsuite if multiple of them exist.. there are a lot of edge cases which would make this even more confusing than the current situation. E.g what happens if only mochitest-plain-media and mochitest-browser exist? Which one is more default-y than the other?

* We could make use of mach aliases (either through advertisement or bootstrap script) to make flavor/subsuite specific commands again like we used to have. E.g, in ~/.mozbuild/machrc:
[alias]
mochitest-devtools = mochitest --flavor browser --subsuite devtools

* It'll be a bit tricky to print the command from the logs because the local srcdir environment will be different enough from the tests.zip environment that simply copying them wholesale won't work. We could cherry pick only relevant args like flavor and subsuite though..
(In reply to Andrew Halberstadt [:ahal] from comment #2)
> * It'll be a bit tricky to print the command from the logs because the local
> srcdir environment will be different enough from the tests.zip environment
> that simply copying them wholesale won't work. We could cherry pick only
> relevant args like flavor and subsuite though..

Right, I don't think this should try to be "here's how to reproduce exactly what automation is running", but "here's how to run just this set of tests locally".
Thanks for filing this Joel.

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> > the problem with the full directory is that all subdirectories and other test types will run- in fact this is by design so that you can test all the tests for the code you are working on...but this doesn't help for the case when investigating a failure.
> 
> Do we think this is a thing people actually want? I guess we only provide
> the one mochitest command so people wanting to run chrome or browser-chrome
> tests would have to specify `--flavor`, but maybe that's OK?

Any time mach ends up running multiple Firefox instances is a confusing and undesirable experience for me. I don't think it should be the default.

> Maybe at the very least we could have the harness detect if there are
> multiple flavors of mochitest in that directory and do something better? I
> don't know that there's a great default behavior here, but it could just
> refuse to run multiple flavors and print "This path contains mochitests of
> flavor (plain, browser-chrome, ...). You should specify which flavor to run
> with `--flavor=foo`." We could add a `--flavor=all` if someone really wanted
> the existing behavior.

Yes, I agree. The same goes for the behavior where it recurses into subdirectories. I would rather have it give you a list of the possible sub-directories to choose from. If you really want multiple invocations, you should have to specify that using a --recursive option or something.
(In reply to Andrew Halberstadt [:ahal] from comment #2)
> * I don't think we should default to only running one flavor/subsuite if
> multiple of them exist.. there are a lot of edge cases which would make this
> even more confusing than the current situation. E.g what happens if only
> mochitest-plain-media and mochitest-browser exist? Which one is more
> default-y than the other?

I don't think that's a good idea either. As Ted said, we should ask you to pick a choice if there isn't a single one.
We could definitely implement a "single invocation" mode. Whether or not that would be default could be a matter of debate (personally I'd prefer it to run subdirs).. but as long as we can toggle it on/off, whoever prefers the opposite of what we choose can create an alias to get their desired behaviour.

Fwiw, I agree that multiple invocations can get confusing with all the different result summaries. There is room for improvement there as well (i.e print a "final" summary or some such). That's probably something we should implement regardless of what happens here.
Also, running the manifest explicitly should avoid running subdirectories, e.g:
./mach mochitest --manifest path/to/mochitest.ini

Just suggesting it as a workaround for now.
(In reply to Andrew Halberstadt [:ahal] from comment #7)
> Also, running the manifest explicitly should avoid running subdirectories,
> e.g:
> ./mach mochitest --manifest path/to/mochitest.ini
> 
> Just suggesting it as a workaround for now.

That did not work for me. It prints a lot of output like this:

Included file '/home/billm/ws/gecko/objdir-ff-dbgopt/_tests/testing/mochitest/tests/toolkit/content/tests/mochitest/mochitest.ini' does not exist
Included file '/home/billm/ws/gecko/objdir-ff-dbgopt/_tests/testing/mochitest/tests/toolkit/content/tests/widgets/mochitest.ini' does not exist
Included file '/home/billm/ws/gecko/objdir-ff-dbgopt/_tests/testing/mochitest/tests/toolkit/modules/tests/mochitest/mochitest.ini' does not exist
Included file '/home/billm/ws/gecko/objdir-ff-dbgopt/_tests/testing/mochitest/tests/toolkit/mozapps/extensions/test/mochitest/mochitest.ini' does not exist
Included file '/home/billm/ws/gecko/objdir-ff-dbgopt/_tests/testing/mochitest/tests/toolkit/xre/test/mochitest.ini' does not exist
Included file '/home/billm/ws/gecko/objdir-ff-dbgopt/_tests/testing/mochitest/tests/uriloader/exthandler/tests/mochitest/mochitest.ini' does not exist
Included file '/home/billm/ws/gecko/objdir-ff-dbgopt/_tests/testing/mochitest/tests/widget/tests/mochitest.ini' does not exist
37 ERROR no tests to run using specified combination of filters: skip_if, run_if, fail_if, remove_imptest_failure_expectations, subsuite(name=webrequest)
Eventually I was able to do what I wanted by commenting out all the test subdirectories from moz.build. So I have a workaround at this point. It's just not a very good one.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.