make `mach python-test` better

RESOLVED FIXED in Firefox 48

Status

RESOLVED FIXED
3 years ago
8 months ago

People

(Reporter: ted, Assigned: ted)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla48
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(2 attachments)

Every so often I want to run the mozbuild unit tests and I try to do `mach python-test python/mozbuild` and it doesn't quite work because the test discovery isn't right.

Nowadays we have all the parts to make this work better, so I have patches to do so! They add the tests from `PYTHON_UNIT_TESTS` to all-tests.json and use `TestResolver` for test resolution.

I had to make a few tweaks to things to make everything work properly, notably:
1) There were bogus entries in PYTHON_UNIT_TESTS, things like __init__.py files. These were probably added accidentally and they were harmless before because `make check` just runs python on each file, and that won't error with an empty file. `mach python-test` will error if it doesn't see any test output, so those got removed.
2) A few of the things in PYTHON_UNIT_TESTS weren't using `mozunit.main` (like bug 1103226), which means they don't output the format that the mach command is looking for, so I fixed those as well.
Created attachment 8729072 [details]
MozReview Request: bug 1255479 - add PYTHON_UNIT_TESTS to all-tests.json. r?nalexander

Review commit: https://reviewboard.mozilla.org/r/39231/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39231/
Attachment #8729072 - Flags: review?(nalexander)
Created attachment 8729073 [details]
MozReview Request: bug 1255479 - make `mach python-tests` use TestResolver for discovery, make `mach test` work for python tests. r?nalexander

Review commit: https://reviewboard.mozilla.org/r/39233/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39233/
Attachment #8729073 - Flags: review?(nalexander)
Blocks: 1210759
Comment on attachment 8729073 [details]
MozReview Request: bug 1255479 - make `mach python-tests` use TestResolver for discovery, make `mach test` work for python tests. r?nalexander

https://reviewboard.mozilla.org/r/39233/#review35929

Stamp on all the test changes; resolver work looks fine.
Attachment #8729073 - Flags: review?(nalexander) → review+
Comment on attachment 8729072 [details]
MozReview Request: bug 1255479 - add PYTHON_UNIT_TESTS to all-tests.json. r?nalexander

https://reviewboard.mozilla.org/r/39231/#review35927

My only significant concern is that we're not declaring Python tests in a test manifest, consonant with all of our other tests.  Is this desired, i.e., should you file follow-up?  Is this explicitly not desired, since we can't really ship a Python tests package (without a source tree) anyway?

Otherwise, looks good!

::: python/mozbuild/mozbuild/frontend/emitter.py:1290
(Diff revision 1)
> +    def _process_python_tests(self, context, python_tests):

Consider if this can be unified with w-p-t's example.  (Reftest looks a little funkier.)

::: python/mozbuild/mozbuild/testing.py:282
(Diff revision 1)
>  WEB_PATFORM_TESTS_FLAVORS = ('web-platform-tests',)

Consider fixing PATFORM -> PLATFORM while you're here.
Attachment #8729072 - Flags: review?(nalexander) → review+
https://reviewboard.mozilla.org/r/39231/#review35927

I don't know that it buys us much, given that the Python unittest module already has pretty decent support for annotating individual tests with skip/fail/etc, and we're not going to package the tests up. If anything, we might wind up running them in a new automation job that just clones the repo and runs mach.
https://reviewboard.mozilla.org/r/39231/#review35927

> Consider if this can be unified with w-p-t's example.  (Reftest looks a little funkier.)

I looked at this and I think it's just different enough to make this a PITA. I'll file a followup to see if we can make this less bad.

> Consider fixing PATFORM -> PLATFORM while you're here.

I didn't even notice that. Funny!
https://hg.mozilla.org/integration/mozilla-inbound/rev/879ea25d7775a6b99154d9fe2babbe50f1ae5758
bug 1255479 - add PYTHON_UNIT_TESTS to all-tests.json. r=nalexander

https://hg.mozilla.org/integration/mozilla-inbound/rev/a03f77804981532999e4d3d41795d8bc65797059
bug 1255479 - make `mach python-tests` use TestResolver for discovery, make `mach test` work for python tests. r=nalexander

Comment 10

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/879ea25d7775
https://hg.mozilla.org/mozilla-central/rev/a03f77804981
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Blocks: 1257820

Updated

8 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.