Closed Bug 1339178 Opened 3 years ago Closed 2 years ago
[mozunit] should use pytest with the pytest-mozlog plugin
Right now most python-tests use mozunit  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 . 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.  https://dxr.mozilla.org/mozilla-central/source/config/mozunit.py  https://dxr.mozilla.org/mozilla-central/source/python/mozlint/test/test_roller.py#82
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?
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
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
Attachment #8902398 - Flags: review?(dave.hunt) → review+
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.
Attachment #8902399 - Flags: review?(dave.hunt) → review+
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.
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.
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.
Pushed by email@example.com: 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
(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.
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.
(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.
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.
++ this would be really helpful. I normally end up commenting out tests which is annoying
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.
I like your first suggestion, I filed bug 1406071, let's move discussion over there.
You need to log in before you can comment on or make changes to this bug.