Open Bug 917363 Opened 6 years ago Updated 2 years ago

Run Python unit tests with nose

Categories

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

defect

Tracking

(Not tracked)

People

(Reporter: gps, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(6 files)

nose (https://nose.readthedocs.org/en/latest/) is a pretty awesome tool for running Python tests. It's features over the built-in test runner are pretty nice:

* better test discovery
* plugins
* code coverage
* writing to machine readable output files
* log capture
* ...

Many of its features are arguably features we want in our Python test "suite." Rather than reinvent the wheel, I propose we add nose to the tree and have it run our Python unit tests.

As part of this, we may also create a moz.build-ism for declaring Python unit tests, since currently things are hooked up via explicit check:: targets in make files.
Package obtained from
https://pypi.python.org/packages/source/n/nose/nose-1.3.0.tar.gz#md5=95d6d32b9d6b029c3c65674bd9e7eabe
and added to the tree without any modifications.

Should be a rubber stamp review. I'll also consider your r+ an
indication that you think this is a good idea (tm).
Attachment #806046 - Flags: review?(ted)
Assignee: nobody → gps
Attachment #806046 - Flags: review?(ted) → review+
I'm not actively working on this.
Assignee: gps → nobody
Add nose to the virtualenv. Should be a rubber stamp.
Attachment #8350808 - Flags: review?(ted)
Assignee: nobody → gps
Status: NEW → ASSIGNED
This adds preliminary support for defining Python tests in moz.build.
I'm not asking for review yet because I'm trying to feel out what all
we'll need to support to get existing tests hooked up right.

My ultimate goal is to replace mozunit with nose, as nose can do things
so much better.
This is the start of nose integration into automation. Like mozunit, I
had to roll some classes for converting nose results into the output
expected by automation.

I ultimately want for humans to see nose's default output (or something
very similar to it) and for the automation results to be derived from a
JSON or XML file (nose does support writing xUnit XML files after all,
so why reinvent the wheel). But for a transition, we'll need nose to
output the TEST-* messages.

I still need to massage the output a bit. But it's a start.
nose is slightly more aggressive about test discovery compared to what
we were using before. Functions with "test" in them were being marked as
test functiosn. These functions have been renamed to avoid ambiguity.

In addition, the test for pythonutil.iter_modules_in_path() needed to be
updated because nose runs every test in the same process and this
leaking extra modules into sys.modules.
Attachment #8350848 - Flags: review?(mh+mozilla)
In addition to replacing Makefile.in declaration with the new moz.build
config, the old boilerplate for running tests with mozunit has been
removed, as it is no longer needed.

While I was here, I cleaned up some unused imports.

This patch won't be safe to land until we add nose-based test running to
another make target and provide a mechanism to easily run tests in a
given file. Those will come in earlier, not-yet-completed parts.
Attachment #8350849 - Flags: review?(mh+mozilla)
Comment on attachment 8350848 [details] [diff] [review]
Part 5: Adjust mozbuild tests to work with nose

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

::: python/mozbuild/mozbuild/test/test_pythonutil.py
@@ +12,5 @@
>      def test_iter_modules_in_path(self):
>          mozbuild_path = os.path.dirname(os.path.dirname(__file__))
>          paths = list(iter_modules_in_path(mozbuild_path))
> +        self.assertIn(os.path.join(os.path.abspath(mozbuild_path), '__init__.py'), paths)
> +        self.assertIn(os.path.join(os.path.abspath(mozbuild_path), 'pythonutil.py'), paths)

missing test_pythonutil.py.

Does nose add modules in the list? If yes, why not just add them if using nose?

@@ +15,5 @@
> +        self.assertIn(os.path.join(os.path.abspath(mozbuild_path), '__init__.py'), paths)
> +        self.assertIn(os.path.join(os.path.abspath(mozbuild_path), 'pythonutil.py'), paths)
> +
> +        # Ensure a system path isn't in the list.
> +        self.assertFalse(any(p for p in paths if p.endswith('os.py')))

Not sure what this is testing.
Attachment #8350848 - Flags: review?(mh+mozilla) → feedback+
Comment on attachment 8350849 [details] [diff] [review]
Part 6: Add python/mozbuild to PYTHON_TESTS

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

This removes support for running individual test files manually, i don't think i like that.
Attachment #8350849 - Flags: review?(mh+mozilla) → feedback-
(In reply to Mike Hommey [:glandium] from comment #9)
> Comment on attachment 8350849 [details] [diff] [review]
> Part 6: Add python/mozbuild to PYTHON_TESTS
> This removes support for running individual test files manually, i don't
> think i like that.

Only for this patch. We'll still support running individual test files via `nose` or `mach python-test`.
(In reply to Gregory Szorc [:gps] from comment #10)
> Only for this patch. We'll still support running individual test files via
> `nose` or `mach python-test`.

I see no reason to break `python foo.py` when it's only two extra lines. (and, specificially, when it's lines that are already there)
Attachment #8350808 - Flags: review?(ted) → review+
Priority: -- → P3
Blocks: 967696
Assignee: gps → nobody
Status: ASSIGNED → NEW
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.