1157852 - Mochitest DevTools test directories run multiple times

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

When I run "mach mochitest-devtools browser/devtools/webide", I see:

dir: browser/devtools/webide/test

The tests in this directory are run, plus those in browser/devtools/webide/test/sidebars

dir: browser/devtools/webide/test/sidebars

The tests in only this directory are run again, even though they already ran in the context of the parent directory above.

Are these test directories misconfigured in some way, or is the test runner duplicating work?

Comment 1

[(not currently active) Ted Mielczarek]

This is either run-by-dir or chunking fallout, but I don't know which.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

odd, looking in my inbound clone, sidebars has no tests within it- for runbydir if there is a directory that has no tests we do not run it.

quite possibly :jryans is adding tests to browser.ini in the sidebars/ directory.

more to investigate.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

here is the problem:
https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#2106

tests_in_dir = [t for t in testsToRun if t.startswith(d)]


no idea if this is runbydir or new chunking- I would like to think the latter, but who knows.  How to fix this?

The original idea behind runbydir is to get all tests from a given manifest and run them.  more of a runbymanifest.  

One thing is we could do
tests_in_dir = [t for t in testsToRun if dirname(t) == d]

^ that would remove tests in a contained subdir.  how would this work if the root manifest was the only thing to include the submanifest?  For this specific webide/test/ directory, it would work as there is a browser.ini in the sidebars subdir.

thoughts anyone?

Comment 4

[Andrew Halberstadt [:ahal]]

(In reply to Joel Maher (:jmaher) from comment #3) 
> One thing is we could do
> tests_in_dir = [t for t in testsToRun if dirname(t) == d]

Yeah, I think that's the proper fix. Though I'm not really sure how (if?) this was working before, because the old options.testPath method also used startswith:
https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#1962
 
> ^ that would remove tests in a contained subdir.  how would this work if the
> root manifest was the only thing to include the submanifest?  For this
> specific webide/test/ directory, it would work as there is a browser.ini in
> the sidebars subdir.

Running ./mach mochitest <test_path> uses options.testPath to filter down tests (see dxr link above). So as long as the test is in a subdirectory of <test_path> and is included by a manifest (and not skipped), it should get run. This is the behaviour of options.testPath with or without runByDir.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: only run tests from a given dir, not the subdirs (1.0)

Comment 6

[Andrew Halberstadt [:ahal]]

Comment on attachment 8597317 [details] [diff] [review]
only run tests from a given dir, not the subdirs (1.0)

Review of attachment 8597317 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with comment fixed.

::: testing/mochitest/runtests.py
@@ +2102,5 @@
>              if inputTestPath and not inputTestPath.startswith(d):
>                  continue
>  
>              print "dir: %s" % d
> +            tests_in_dir = [t for t in testsToRun if '/'.join(t.split('/')[:-1]) == d]

I think you're looking for os.path.dirname.

Also it might be a good idea to run them both through "os.path.normpath":
https://docs.python.org/2/library/os.path.html#os.path.normpath

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: only run tests from a given dir, not the subdir (1.1)

carry forward the r+, thanks for the os.path.dirname, it works in local testing. let me do a nice try push before landing.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=26c10f401933

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bcfc39710995

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bcfc39710995