982560 - Mach command fails to find tests if executed from a sub directory

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P4)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Today I tried to execute the xpcshell tests for fxaccounts from within the testing/tps folder:

../../mach xpcshell-test ../../services/fxaccounts/tests/xpcshell/

As result I get:

From _tests: Kept 16267 existing; Added/updated 0; Removed 0 files and 0 directories.
We could not find an xpcshell test for the passed test path. Please select a path that is a test file or is a directory containing xpcshell tests.

When I execute that command from the root folder, all is fine. So mach fails to handle the relative path, or doesn't operate that well in general from a lower directory.

Comment 1

[Gregory Szorc [:gps]]

I can reproduce this.

This is almost certainly a bug in python/mozbuild/mozbuild/testing.py:TestResolver.

Anyone who knows Python should be able to set some breakpoints in TestResolver.resolve_tests() to identify and fix the problem. We definitely want a test case for this. python/mozbuild/mozbuild/test/test_testing.py.

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I would like working on this bug. Would you tell me how to run test case in python/mozbuild/mozbuild/test/test_testing.py? Thanks.

Comment 3

[Gregory Szorc [:gps]]

mach python-test python/mozbuild/mozbuild/test

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Fix mach fails to find tests in subdirectory

Make the test paths relative to topsrcdir before passing them to
TestResolver. Also do not passing cwd to TestResolver since it will
filter out tests that do not live under the directory where the mach
command is executed.

Verification steps:
Execute a mach test command from any subdirectory. For example:
$ cd testing/tps/
$ ../../mach xpcshell-test ../../services/fxaccounts/tests/xpcshell/
$ ../../mach test ../../services/fxaccounts/tests/xpcshell/

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8449195 [details] [diff] [review]
Fix mach fails to find tests in subdirectory

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

This works and I'm giving it r+. But it isn't my preferred solution.

I would much prefer that the automagical TestResolver.resolve_tests() would perform this normalization automatically. e.g. if you pass in a relative path, it expands it given the cwd context. The reason I prefer this is because it can be tested easier. We currently don't have good test coverage of mach commands themselves. But we do have decent test coverage of the underlying libraries. Putting this translation inside a testable Python module would make me feel better about things.

That being said, I'm granting r+ because this improves developer workflow and I shouldn't let an ivory tower get in the way.

Comment 6

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Hi Gregory,

I agree it will be better to let TestResolver.resolve_tests() normalize the paths. However its document says "If cwd is defined, we will limit our results to tests under the directory specified. [1]" Therefore, if the user specifies a relative path not under cwd, no test could be found. That's why I removed cwd argument passing to TestResolver.resolve_tests(). Do you recall why it was designed to limit the result under cwd? Could this limitation be removed? I feel tests searching is already limited by the argument `paths`.

[1] http://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/testing.py#164

Comment 7

[Gregory Szorc [:gps]]

IIRC the `cwd` argument was intended to filter results to under the specified `cwd` subtree. If we even considered supporting ../ relative paths into TestResolver (I don't see any evidence of it in bug 920849), then it was a non-requirement for the initial launch.

All the code consuming TestResolver is in tree. If you want to change the API, go for it.

But I still encourage you to land this patch and do the improvement as a follow-up.

Comment 8

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Thanks for your information. Let's land this patch.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/545d28bfe4fb

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/545d28bfe4fb