917363 - Run Python unit tests with nose

Status: NEW

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

Description

[Gregory Szorc [:gps]]

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.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Part 1: Add nose Python package version 1.3.0

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).

Comment 2

[Gregory Szorc [:gps]]

I'm not actively working on this.

Comment 3

[Gregory Szorc [:gps]]

Attachment: Part 2: Add nose to virtualenv

Add nose to the virtualenv. Should be a rubber stamp.

Comment 4

[Gregory Szorc [:gps]]

Attachment: Part 3: Support defining Python tests in moz.build

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.

Comment 5

[Gregory Szorc [:gps]]

Attachment: Part 4: Custom test runner for automation

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.

Comment 6

[Gregory Szorc [:gps]]

Attachment: Part 5: Adjust mozbuild tests to work with nose

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.

Comment 7

[Gregory Szorc [:gps]]

Attachment: Part 6: Add python/mozbuild to PYTHON_TESTS

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.

Comment 8

[Mike Hommey [:glandium]]

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.

Comment 9

[Mike Hommey [:glandium]]

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.

Comment 10

[Gregory Szorc [:gps]]

(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`.

Comment 11

[Mike Hommey [:glandium]]

(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)