The test resolver matches tests under a directory if the input matches a directory containing tests, but if it doesn't, it matches if an input is a substring of a test path. I'd like to add an option to select on path prefixes instead. I'm writing a tool that selects across suites, so if I provide "image/" as an input path, I end up selecting devtools tests and scheduling those due to substring matches, which isn't really desirable. I might go as far as to say this should be the default behavior for the test resolver.

/r/7573 - Bug 1158019 - Only resolve to tests under a subdirectory if an input to mozbuild's test resolver is the prefix of a directory containing tests.;r=gps Pull down this commit: hg pull -r 557543335753f49a1ed51e5082a004c0a8017828 https://reviewboard-hg.mozilla.org/gecko/

Comment on attachment 8597024 [details] MozReview Request: bz://1158019/chmanchester https://reviewboard.mozilla.org/r/7571/#review6423 I don't fully grok what's going on. This change needs inline comments (the docstring for this method is quite detailed). Tests would also be nice. We've had enough bugs around test resolution over the years and people always complain when subtle changes occur. Explicit tests so changes in behavior are detected is well worth the effort. See python/mozbuild/mozbuild/test/test_testing.py. ::: python/mozbuild/mozbuild/testing.py:128 (Diff revision 1) > + any([p.startswith(path) for p in self._tests_by_path])): I'm concerned about the performance impact here. Can you measure this? Nit: I don't believe you need the [].

https://reviewboard.mozilla.org/r/7571/#review6427 Yes I will write tests and comments in a moment. I also wrote a detailed commit message for this, I'm not sure why it's not appearing in reviewboard.

https://reviewboard.mozilla.org/r/7571/#review6429 The commit message isn't appearing because of a bad UX decision. There are patches under development to make it better.

https://reviewboard.mozilla.org/r/7571/#review6433 > I'm concerned about the performance impact here. Can you measure this? > > Nit: I don't believe you need the []. I measured the following 5 times for each of the four cases below, just taking the execution time of the loop that starts with "for path in sorted(paths):". To test for a path that is the prefix of a directory with tests, I ran |./mach test image/|, to test for a term that is not a prefix of a directory with tests but resolves tests, I ran |./mach test animation| Without my patch applied, the loop consistently takes 17ms to run, whether or not the input is a prefix of a directory matching tests. With my patch applied, an input that is a prefix of directory with tests takes about 15ms to run, but with it it takes 33ms to run. I have a pretty fast computer, but I'm not sure this additional time penalty will be significant to a user: when running the |./mach test animation| case, I can calmly count to ten between the time I hit enter and the first mochitest-bc tests start running.

Comment on attachment 8597024 [details] MozReview Request: bz://1158019/chmanchester /r/7573 - Bug 1158019 - Tests exercising the proposed behavior.;r=gps /r/7671 - Bug 1158019 - Only resolve to tests under a subdirectory if an input to mozbuild's test resolver is the prefix of a directory containing tests.;r=gps Pull down these commits: hg pull -r 686cd0664b534296a43d2c96c8efdc337f484abf https://reviewboard-hg.mozilla.org/gecko/

I'm only going this route because I think the existing behavior is not correct, if that's contentious I can make it toggleable.

https://reviewboard.mozilla.org/r/7671/#review6437 Wait - where's the test for the added condition?

https://reviewboard.mozilla.org/r/7573/#review6439 Ship It!

https://reviewboard.mozilla.org/r/7671/#review6441 "test_resolve_path_prefix" executes the condition (the test fails without the patch).

https://reviewboard.mozilla.org/r/7671/#review6443 But the test is only failing because the "image" directory doesn't exist on disk. Had the test created a directory (as what would match a number of use cases), we'd be getting the first condition, not the one you added. But I suppose the test as written is good enough :)

Comment on attachment 8597024 [details] MozReview Request: bz://1158019/chmanchester https://reviewboard.mozilla.org/r/7571/#review6445

https://reviewboard.mozilla.org/r/7671/#review6447 I don't see your point, but if this test is broken in some way I don't understand I'd like to fix it. The "image" directory exists, it's just not a directory that itself contains tests (so doesn't appear in self._test_dirs). If it were, then we'd get the first condition. When I run the test without the patch it fails because it resolves to the two tests I added, because the devtools one contains "image".

At any rate, thanks for the review.

The tests for this are run during make check and pass on this try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=46f7d6c8395d

https://hg.mozilla.org/integration/mozilla-inbound/rev/e654d5d726e9 https://hg.mozilla.org/integration/mozilla-inbound/rev/4103078c32e5

https://hg.mozilla.org/mozilla-central/rev/e654d5d726e9 https://hg.mozilla.org/mozilla-central/rev/4103078c32e5