Closed Bug 1451501 Opened 2 years ago Closed 2 years ago

Fix error when running python-tests with |mach test|


(Testing :: Python Test, enhancement)

Version 3
Not set


(firefox61 fixed)

Tracking Status
firefox61 --- fixed


(Reporter: ahal, Assigned: ahal)



(1 file)

Running |mach test testing/mozbase| results in something like:
> 0:03.15 TEST-UNEXPECTED-FAIL | No tests collected for subsuite 'mozbase, os == "linux"' (Not in PYTHON_UNITTEST_MANIFESTS?)

This is happening because |mach test| is helpfully trying to forward the 'subsuite' that all python unittests have defined in their manifests. But python-tests use the subsuite key differently than other harnesses (mostly to split tests out into different tasks).

We just need to ignore the subsuite when being invoked from |mach test|.
Comment on attachment 8965112 [details]
Bug 1451501 - Fix error trying to run python-tests via |mach test|,

::: python/
(Diff revision 1)
>          finally:
>              import mozfile
>              mozfile.remove(tempdir)
>      def run_python_tests(self,
> -                         tests=[],
> +                         tests=None,

Wow, nasty bug by using a mutable type as default.

::: python/
(Diff revision 1)
> -                                                      flavor='python')
> -            else:
> +        else:
> -                # Otherwise just run everything in PYTHON_UNITTEST_MANIFESTS
> -                test_objects = resolver.resolve_tests(flavor='python')
> +            # We've received test_objects from |mach test|. We need to ignore
> +            # the subsuite because python-tests don't use this key like other
> +            # harnesses do and |mach test| doesn't realize this.
> +            subsuite = None

All the above works but I'm unsure about the changes given that I don't know anything about test discovery. It's better to ask gps for review here.
Attachment #8965112 - Flags: review?(hskupin)
Fwiw, I don't think gps will have much more context than you do. It's more related to how |mach python-test| uses subsuites than test discovery.
Comment on attachment 8965112 [details]
Bug 1451501 - Fix error trying to run python-tests via |mach test|,

Ok, so I spent a bit of time to figure out how stuff works in that file, and it now makes sense. But please note that I'm not a peer of python/
Attachment #8965112 - Flags: review+
Comment on attachment 8965112 [details]
Bug 1451501 - Fix error trying to run python-tests via |mach test|,

This seems reasonable.

Mutable default argument values strike again!

I really wish we had a global lint for this...
Attachment #8965112 - Flags: review?(gps) → review+
Pushed by
Fix error trying to run python-tests via |mach test|, r=gps,whimboo
Being too pedantic here, but the mutable default was just a drive-by fix I happened to notice. The root of the issue here was that |mach python-test| treats --subsuite differently than our other harnesses.

Good idea about a linter though. I wonder if someone's already written a flake8 plugin to catch it.
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.