1339178 - [mozunit] should use pytest with the pytest-mozlog plugin

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Andrew Halberstadt [:ahal]]

Right now most python-tests use mozunit [1] as their test runner. Mozunit is basically a thin wrapper around unittest that provides some Mozilla specific logging and result collection (plus some other unrelated utilities). Some python-tests, use pytest instead of mozunit [2].

Neither test runner is set up to work with structured logging. If we get mozunit to be a thin wrapper around pytest that sets up the pytest-mozlog plugin, we can kill 3 birds with one stone:

1. Consistent test runner for all python tests
2. Use mozlog for consistent logging with rest of test harnesses
3. Use pytest everywhere instead of unittest

We wouldn't even need to re-write any tests, because pytest can also run unittest based tests.


[1] https://dxr.mozilla.org/mozilla-central/source/config/mozunit.py
[2] https://dxr.mozilla.org/mozilla-central/source/python/mozlint/test/test_roller.py#82

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1339178 - Add python/mach_commands.py and config/mozunit.py to flake8 linter,

Review commit: https://reviewboard.mozilla.org/r/173950/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/173950/

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1339178 - Use pytest to run python-tests,

This switches most tests over to use pytest as the runner instead of unittest (taking
advantage of the fact that pytest can run unittest based tests).

There were a couple tests that had failures when swithing to pytest:
config/tests/unit-expandlibs.py
xpcom/idl-parser/xpidl/runtests.py

For these tests, I added a runwith='unittest' argument so that they still run the
same way as before. Once we fix them to use pytest, the unittest logic in mozunit.py
can be deleted.

Review commit: https://reviewboard.mozilla.org/r/173952/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/173952/

Comment 3

[Andrew Halberstadt [:ahal]]

You may notice that while pytest-mozlog is available, most tests aren't using it. I'm sort of on the fence about this. On one hand it's good to use structured logging and have everything going through there.

On the other hand, I find the failure output of pytest to be very helpful (especially when debugging failures that only happen on a particular platform). So I guess I'd prefer to keep the output format as is.. though if you strongly believe we should just use --log-tbpl everywhere, I could get on board with that too.

Ideally using --log-tbpl would print all the same information on failure that normally gets printed. Would that be a hard change to make?

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8902398 [details]
Bug 1339178 - Add python/mach_commands.py and config/mozunit.py to flake8 linter,

https://reviewboard.mozilla.org/r/173950/#review179954

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8902399 [details]
Bug 1339178 - Use pytest to run python-tests,

https://reviewboard.mozilla.org/r/173952/#review179964

::: config/mozunit.py:224
(Diff revision 1)
> +        args = list(args)
> +        if os.environ.get('MACH_STDOUT_ISATTY') and not any(a.startswith('--color') for a in args):
> +            args.append('--color=yes')
> +
> +        module = __import__('__main__')
> +        sys.exit(pytest.main(args + ['--verbose', module.__file__]))

You could also enable the pytest-mozlog plugin here, which means it would be avaialble for all tests. Currently, tests that want to use the mozlog plugin need to include pytest.ini to enable the plugin. Adding the command line option '-p mozlog.pytest_mozlog.plugin' will enable the plugin.

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Currently, adding a mozlog output to the console interleaves the existing console logging, so in places we've disabled the terminalwriter entirely using '-p no:terminalreporter'. It would be great if we could just optionally disable the test logging, which would still allow the awesome failure summary that pytest provides. I'll look into this and will comment here with my findings.

Comment 7

[Andrew Halberstadt [:ahal]]

What do you mean by test logging? Like all output from mozlog or just TEST-START/TEST-END messages or something else? We can definitely create a custom formatter for |mach python-test| (and store it in config/mozunit.py) to get whatever format we want.

We could also re-direct mozlog output to a file, e.g, --log-raw=python-test.log then leave stdout free for pytest to do its thing.

Comment 8

[Andrew Halberstadt [:ahal]]

Comment on attachment 8902398 [details]
Bug 1339178 - Add python/mach_commands.py and config/mozunit.py to flake8 linter,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/173950/diff/1-2/

Comment 9

[Andrew Halberstadt [:ahal]]

Comment on attachment 8902399 [details]
Bug 1339178 - Use pytest to run python-tests,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/173952/diff/1-2/

Comment 10

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I was able to silence the log start/report hooks from the TerminalReporter plugin by adding the following to pytest_configure in testing/mozbase/mozlog/pytest_mozlog/plugin.py:

    terminal = config.pluginmanager.get_plugin('terminal').TerminalReporter
    terminal.pytest_runtest_logstart = lambda x: x
    def logreport(self, report):
        rep = report
        res = self.config.hook.pytest_report_teststatus(report=rep)
        cat, letter, word = res
        self.stats.setdefault(cat, []).append(rep)
    terminal.pytest_runtest_logreport = logreport

This allows pytest-mozlog to direct logging to the console without the noise of the existing console logging, but still provides the pytest header and failure summary, etc. The issue here is that if you weren't directing a pytest-mozlog format to the console then you wouldn't see any output when tests start/finish. It's also not guaranteed that future versions of pytest wouldn't break this workaround.

My preference is to allow pytest to log to the console, and for pytest-mozlog to be used for logging to file. If it's possible for us to do this, and have any tools that rely on parsing this output to read it from file then this is a very clean solution.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3cfed550091b
Add python/mach_commands.py and config/mozunit.py to flake8 linter, r=davehunt
https://hg.mozilla.org/integration/autoland/rev/3f58f4dc8303
Use pytest to run python-tests, r=davehunt

Comment 12

[Andrew Halberstadt [:ahal]]

(In reply to Dave Hunt (:davehunt) from comment #10)
> I was able to silence the log start/report hooks from the TerminalReporter
> plugin by adding the following to pytest_configure in
> testing/mozbase/mozlog/pytest_mozlog/plugin.py:
> 
>     terminal = config.pluginmanager.get_plugin('terminal').TerminalReporter
>     terminal.pytest_runtest_logstart = lambda x: x
>     def logreport(self, report):
>         rep = report
>         res = self.config.hook.pytest_report_teststatus(report=rep)
>         cat, letter, word = res
>         self.stats.setdefault(cat, []).append(rep)
>     terminal.pytest_runtest_logreport = logreport
> 
> This allows pytest-mozlog to direct logging to the console without the noise
> of the existing console logging, but still provides the pytest header and
> failure summary, etc. The issue here is that if you weren't directing a
> pytest-mozlog format to the console then you wouldn't see any output when
> tests start/finish. It's also not guaranteed that future versions of pytest
> wouldn't break this workaround.

Neat! What if it only does this if it detects a log stream to stdout, e.g --log-tbpl=- ?


> My preference is to allow pytest to log to the console, and for
> pytest-mozlog to be used for logging to file. If it's possible for us to do
> this, and have any tools that rely on parsing this output to read it from
> file then this is a very clean solution.

This is fine. Though, I think we were only originally using pytest-mozlog in the marionette-harness tests to get treeherder to highlight the failure lines (which it now does anyway). So we might not even need to use pytest-mozlog at all.

I guess it wouldn't hurt to upload the raw log as an artifact so these tests get added to ActiveData. I'll file a follow-up bug.

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/3cfed550091b
https://hg.mozilla.org/mozilla-central/rev/3f58f4dc8303

Comment 14

[Mike Hommey [:glandium]]

How does one run a single test from a test module? With unittest, it was possible to pass Class.method on the command line, and only have Class::method run, but that doesn't work anymore since this change.

Comment 15

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Mike Hommey [:glandium] from comment #14)
> How does one run a single test from a test module? With unittest, it was
> possible to pass Class.method on the command line, and only have
> Class::method run, but that doesn't work anymore since this change.

Typically with pytest you would specify the path::class::method, however that's not working here. Another option is to pass the keyword argument to pytest. I'm not sure if there's a way to pass this through mach, but if you put arguments in the PYTEST_ADDOPTS environment variable they will be used. For example:

$ PYTEST_ADDOPTS="-k test_extract_tarball" mach python-test testing/mozbase/mozfile/tests/test_extract.py

That's not an ideal solution, but might work for you as a temporary workaround? It would be good to allow individual test classes/methods to be provided as part of the path.

Comment 16

[Dave Hunt [:davehunt] [he/him] ⌚BST]

A quick look indicates that the test paths are matched against the paths discovered via the manifests, so we'd potentially need to split the path at '::' for that check to pass, and preserve the full 'path' for pytest.

Comment 17

[Andrew Halberstadt [:ahal]]

++ this would be really helpful. I normally end up commenting out tests which is annoying

Comment 18

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I'm not sure how best to achieve this, as we're matching the paths up with the manifests and then running the test files in separate python processes. We'd need to preserve everything after the module path, and then pass this to the process to use as an argument for mozunit so it can reconcile them in the pytest invocation. Another option might be to allow for a command line option for the node id, but we'll still need to somehow pass this through to mozunit. The issue here is that it would be applied against all test paths, so you wouldn't be able to specify multiple node ids. See https://docs.pytest.org/en/latest/usage.html#specifying-tests-selecting-tests for more information on specifying node ids in pytest.

Comment 19

[Andrew Halberstadt [:ahal]]

I like your first suggestion, I filed bug 1406071, let's move discussion over there.