tests: add support for writing a test using `unittest` #16216

dpgeorge · 2024-11-12T05:47:44Z

Summary

All the existing tests require a .exp file (either manually specified or generated running the test first under CPython) that is used to check the output of running the test under MicroPython. The test passes if the output matches the expected output exactly.

This has worked very well for a long time now. But some of the newer hardware tests (eg UART, SPI, PWM #16147) don't really fit this model, for the following main reasons:

Some but not all parts of the test should be skipped on certain hardware targets. With the expected-output approach, skipping tests is either all or nothing.
It's often useful to output diagnostics as part of the test, which should not affect the result of the test (eg the diagnostics change from run to run, like timing values, or from target to target).
Sometimes a test will do a complex check and then print False/True if it passed or not, which obscures the actual test result.

To improve upon this, this PR adds support to ./run-tests.py for a test to use unittest. It detects this by scanning for an import unittest at the top of the file (it must be the first non-comment/non-blank line). If the test uses unittest then it should not have a .exp file, and it's not run under CPython. Instead it's run under MicroPython only and the test runner looks for the unittest output at the end of the test, either "OK" or "FAILED". It uses that to tell if the test passed or failed.

Note that (as long as unittest is installed on the target) the tests are still fully independent and you can still run them without run-tests.py: you just run it as usual, eg mpremote run <test.py>. This is very useful when creating and debugging tests.

Note that the standard test suite testing Python semantics (eg everything in tests/basics/) will probably never use unittest. Only more advanced tests will.

Testing

One of the stm32-specific tests has been updated to use unittest. It was tested on a PYBD-SF2, and passes.

The esp8266 port has a change to enable function attributes so unittest can run on it. I did test #16147 modified to use unittest and ran it on an esp8266 and it worked. This shows that esp8266 has enough resources to run unittest.

Note: I deliberately used bad indentation in this updated test so it's easier to see the required changes to convert the test to use unittest. Proper formatting will be applied once this PR is finalised and approved.

Trade-offs and Alternatives

Tests using unittest will require first installing it (only needs to be done once), and also need enough resources (RAM). But usually the tests that use it will be more complex ones that already have certain resource requirements.

We could continue to use .exp output comparison, but that is quite limiting.

We could use pytest but MicroPython doesn't support it yet.

dpgeorge · 2024-11-12T05:48:20Z

@stinos FYI, this might be interesting for you.

codecov · 2024-11-12T05:49:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.57%. Comparing base (406bccc) to head (2c80d36).
Report is 8 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #16216   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files         164      164           
  Lines       21349    21349           
=======================================
  Hits        21045    21045           
  Misses        304      304

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dpgeorge · 2024-11-13T01:49:07Z

I've also added a commit here to convert the tests/net_hosted/connect_nonblock_xfer.py test (see #16156) to use unittest. That makes it possible to print more diagnostics output, and also run on esp8266 where it can skip the SSL bits of the test.

stinos · 2024-11-13T10:06:01Z

Didn't check in detail yet but seems a welcome addition. I assume you chose this way (keeping run-tests for these tests as opposed to e.g. standard unittest discovery) so the existing tests can stay in the same directories as they are now, skipping based on platform etc still works?

It detects this by scanning for an import unittest at the top of the file (it must be the first non-comment/non-blank line).

This would make it impossible to use for us, mainly because we use isort or similar so the unittest import is usually not at the top. And we have files with """ comments at the top. It's also not compatible with from __future__ imports because those must be at the top. I get that it's a bit hard to scan a complete import block, but doing something like (pseudocode)

import_seen = False
for line in file:
  line = line.strip()
  # Skip empty lines, comments with '#' and try/catch statements which aren't uncommon in import blocks.
  if not line or line.startswith('#') or 'try:' in line or 'except' in line:
    continue
  # Skip docstrings.
  if line.startswith('"""'):
    for line in file:
      if line.rstrip().endswith('"""'):
        break
  if 'import' in line: 
    import_seen = True
  elif import_seen:
    # End of import block..
    return False
  if 'import unittest' in line or 'from unittest' in line:
    return True
return False

would support quite a lot of cases.

the test runner looks for the unittest output at the end of the test, either "OK" or "FAILED".

Is full output still printed?

dpgeorge · 2024-11-13T12:31:56Z

I assume you chose this way (keeping run-tests for these tests as opposed to e.g. standard unittest discovery) so the existing tests can stay in the same directories as they are now, skipping based on platform etc still works?

The main reason to keep run-tests.py as the top level runner is so the tests can easily be run on bare-metal targets (over a serial line). That's really hard to do with unittest discovery, it would require either mpremote mount or copying all tests to the target. Either way it would need lots of resources on the target which certain targets may not have.

Also, keeping run-tests.py as the thing to run, allows existing and new (unittest) tests to run seamlessly together.

This would make it impossible to use for us, mainly because we use isort or similar so the unittest import is usually not at the top. And we have files with """ comments at the top. It's also not compatible with from __future__ imports because those must be at the top. I get that it's a bit hard to scan a complete import block, but doing something like

My main motivation for keeping the unittest detection simple was to keep things fast. Every test needs to be checked if it's a unittest test or not before it's run, and I didn't want to scan through the entire test file.

But your suggestion to be a little more flexible/general sounds good to me.

An alternative would be to just run the test on the target and check the output, if it ends in the unittest OK/FAILURE line then interpret it as a unittest test, otherwise it's a standard test that needs/has a .exp file. I might actually try doing that instead, see how it goes.

the test runner looks for the unittest output at the end of the test, either "OK" or "FAILED".

Is full output still printed?

If the test passes then the full output is not printed nor saved. If the test fails then the full output is saved to the corresponding .py.out file.

stinos · 2024-11-13T13:24:28Z

An alternative would be to just run the test on the target and check the output, if it ends in the unittest OK/FAILURE line then interpret it as a unittest test, otherwise it's a standard test that needs/has a .exp file. I might actually try doing that instead, see how it goes

Yeah if it needs to run anyway this is probably the fastest as well.

dpgeorge · 2024-11-14T01:38:33Z

An alternative would be to just run the test on the target and check the output, if it ends in the unittest OK/FAILURE line then interpret it as a unittest test, otherwise it's a standard test that needs/has a .exp file. I might actually try doing that instead, see how it goes.

I've now pushed a commit to implement this and it's quite clean, it no longer needs to parse the test file to check for import unittest, it just parses the output of running the test to check for valid unittest result output.

This way has the added benefit of not running tests under CPython if the target requests to skip them.

projectgus

I'm very excited about this, I think it's going to unlock a lot more flexibility for testing!

There's one thing I noted while testing it out (inline below). I also have a couple of minor questions here, but I think those could be in follow-ups if needed at all.

One is about ergonomics: if unittest is not installed on the target, the runner tries to run the test under CPython and you get a failure like this:

--- /home/gus/ry/george/micropython/tests/results/net_hosted_connect_nonblock_xfer.py.exp       2024-11-20 09:58:50.383284085 +1100
+++ /home/gus/ry/george/micropython/tests/results/net_hosted_connect_nonblock_xfer.py.out       2024-11-20 09:58:50.383284085 +1100
@@ -1 +1,4 @@
-CPYTHON3 CRASH
\ No newline at end of file
+Traceback (most recent call last):
+  File "<stdin>", line 4, in <module>
+ImportError: no module named 'unittest'
+CRASH
\ No newline at end of file

run-tests.py could look for a line ImportError: no module named 'unittest' as a special case and return a friendly error, or possibly even auto-run mip install unittest and re-run the test?

The other question I have is about CPython: is there value in having a unittest based test that runs under CPython as well and the test runner checks it passes on both? I don't know if there's a subset of tests where we're looking for CPython-compatible APIs (or partly CPython-compatible), but making the output 100% the same creates difficulty.

My feeling is that we can leave than one until/unless it comes up as something that would be useful to have, rather than building it now, but I thought I'd bring it up.

tests/net_hosted/connect_nonblock_xfer.py

tests/run-tests.py

andrewleech

This change is really quite clean, I can certainly see myself using this in future! Even just the ability to print the extra details on failure can be very helpful.

FWIW I have been planning on submitting a trimmed down reimplementation of pytest for micropython, but if that is completed and provides value here support for it (or any other test framwwork) could be easily added by just updating the results matcher block to look for the format for pytest as well as unittest.

andrewleech · 2024-11-20T00:30:47Z

tests/README.md

+  and the output captured, and then run under MicroPython and the output compared to the
+  CPython output.  The test passes if the output matches exactly.  If the output differs
+  then the test fails and the outputs are saved in a `.exp` and a `.out` file
+  respectively.


This paragraph about tests without exp might make more sense to move down lower now and rewritten slightly to reflect that they're run under cpython only if there's no unittest result returned.

OK, I'm moved the unittest paragraph to the top and adjusted the text.

andrewleech · 2024-11-20T00:42:59Z

tests/run-tests.py

+                b")$",
+                output_mupy_lines[-1],
+            )
+            uses_unittest = unittest_ran_match and unittest_result_match


In future it might be worth refactoring this block into a separate uses_unittest(output_mupy) function to make it easier to add extra results parsers for any other test mechanism / framework, but I don't neccesarily think it's needed at this point. Better to wait until its needed and ensure the api makes sense.

I'll leave that for the future 😄

agatti · 2024-11-25T10:44:07Z

Would this help to provide more granularity to the feature detection system so it doesn't rely only on the test filename prefix/suffix?

Right now the full basics test suite cannot be run cleanly on a device whose MICROPY_CONFIG_ROM_LEVEL is below MICROPY_CONFIG_ROM_LEVEL_CORE_FEATURES. For example, basics/assign_expr_scope.py, basics/assign_expr.py, basics/assign_expr_syntaxerror.py, and basics/string_format_error.py depend on MICROPY_PY_ASSIGN_EXPR - there are other features involved for other files but the issue is the same.

At the moment I'm working on a port (WCH) targetting chips that may have as little flash as 128KBytes total. I haven't yet settled on the default feature set for the lower end chips so I literally started with MICROPY_CONFIG_ROM_LEVEL_MINIMUM for those models, and I have to replicate the failing tests with my own tests having those optional features disabled. If this change can let me avoid having to do all the extra work, I'd love to give it a try! :)

projectgus · 2024-11-26T22:54:54Z

is there value in having a unittest based test that runs under CPython as well and the test runner checks it passes on both?

One data point: have to jump through hoops here - #16272 (comment) - because CPython logs some warnings on interpreter shutdown. One possible path forward would be a unittest that we verify passes on both CPython and MicroPython.

projectgus · 2024-11-26T23:04:53Z

I have to replicate the failing tests with my own tests having those optional features disabled

This is something of an aside, but is it reasonable to split these tests up and do "feature detection" at the top of some with a SKIP output if they aren't supported? I realise it may be hard to accurately tell the difference between a broken core feature and a missing one, though...

agatti · 2024-11-27T12:04:23Z

I realise it may be hard to accurately tell the difference between a broken core feature and a missing one, though...

Right now I'm contemplating adding a tuple of integers to sys called _mpy_features containing a features bitfield split over multiple values. The bitfield values can be computed at build time via a regular Python script, just like the qstrs list collection scripts do. Tests can be preprocessed before being sent to the device, via CPython's ast module, to replace feature constants with the integer value computed for the current build.

This could, in theory, provide the SKIP behaviour you described, although to keep the amount of changes down I'd use this to alter the test behaviour at runtime instead of splitting tests. An issue that arises from this method is that if you fully bypass a test, you should return the value that the test harness expects. If this value is dependent on the CPython version you run this on, then that becomes a problem though.

What could go wrong with this approach, besides the added complexity (and build/test run time overhead)? :)

dpgeorge · 2024-12-03T04:19:25Z

run-tests.py could look for a line ImportError: no module named 'unittest' as a special case and return a friendly error, or possibly even auto-run mip install unittest and re-run the test?

Now done. run-tests.py now aborts if unittest is not found, with a hint how to fix that:

$ ./run-tests.py -t a0 net_hosted/connect_nonblock_xfer.py
platform=rp2 arch=armv6m
error: test net_hosted/connect_nonblock_xfer.py requires unittest
(eg run `mpremote mip install unittest` to install it)
$

The idea being, if you try to run tests that need unittest then that's a "user error" that needs to be rectified (either don't run those tests, or install unittest).

The other question I have is about CPython: is there value in having a unittest based test that runs under CPython as well and the test runner checks it passes on both?

I think this is hard to do right now because you need to distinguish between tests that use unittest and can't be run under CPython, and tests that use unittest and can be run under CPython and should match CPython in behaviour. Right now the assumption is that if you use unittest then it's because you are testing functionality outside of the scope of CPython, eg hardware tests. The README.md is updated to say a bit more about this.

github-actions · 2024-12-03T04:21:18Z

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32: +7473 +1.921% VIRT_RV32

dpgeorge · 2024-12-03T04:22:26Z

@agatti your comments about skipping tests are noted, and I agree it would be good to solve them, but I think out of scope for this PR. Ideally we would run tests against the unix minimal variant under CI... if that worked it would definitely help your case but we just didn't get around to properly supporting minimal builds in the test suite (although it's a good goal).

dpgeorge · 2024-12-03T04:23:11Z

Apart from squashing commits (I left them for easier review), this should be good to go now.

projectgus

LGTM, this will be really nice to have!

tests/run-tests.py

stinos · 2024-12-03T09:59:19Z

tests/run-tests.py

+        uses_unittest = False
+        output_mupy_lines = output_mupy.splitlines()
+        if any(
+            line == b"ImportError: no module named 'unittest'" for line in output_mupy_lines[:3]


This assumes there's no other output before the test tries to import unittest, which isn't necessarily the case. Minor issue though since the test will still fail.

If a test uses unittest it's very unlikely it will print anything before import unittest. The pattern I've been using is:

try-import modules that you need for the test, and SKIP if they don't exist

import unittest (this is done after checking if the test should be SKIPed so you don't need unittest if you won't anyway run the test)

possibly define some state, helper functions, etc

define the main test class(es)

use the standard if __name__ == "__main__": unittest.main() code at the end

That said, I could change the code here to check the last few lines (really it only needs to check the last line) of the output?

If a test uses unittest it's very unlikely it will print anything before import unittest

I agree that it is unlikely, and also it seems to be a bit not done to output during import phase but I do have such tests :)

That said, I could change the code here to check the last few lines (really it only needs to check the last line) of the output?

Seems very sensible

OK, now updated to check the last 3 lines of the output.

stinos · 2024-12-03T10:01:22Z

tests/run-tests.py

+        elif (
+            len(output_mupy_lines) > 4
+            and output_mupy_lines[-4] == b"-" * 70
+            and output_mupy_lines[-2] == b""


Is this even needed, i.e. won't the b"-" * 70 be sufficient?

It's not needed, but I wanted to be strict here with the pattern matching, in case a non-unittest test outputs similar strings.

stinos · 2024-12-03T10:01:55Z

Did some basic tests and apart from the threadpool issue this works the way it should.

All the existing tests require a .exp file (either manually specified or generated running the test first under CPython) that is used to check the output of running the test under MicroPython. The test passes if the output matches the expected output exactly. This has worked very well for a long time now. But some of the newer hardware tests (eg UART, SPI, PWM) don't really fit this model, for the following main reasons: - Some but not all parts of the test should be skipped on certain hardware targets. With the expected-output approach, skipping tests is either all or nothing. - It's often useful to output diagnostics as part of the test, which should not affect the result of the test (eg the diagnostics change from run to run, like timing values, or from target to target). - Sometimes a test will do a complex check and then print False/True if it passed or not, which obscures the actual test result. To improve upon this, this commit adds support to `run-tests.py` for a test to use `unittest`. It detects this by looking at the end of the output after running the test, looking for the test summary printed by `unittest` (or an error message saying `unittest` was not found). If the test uses `unittest` then it should not have a .exp file, and it's not run under CPython. A `unittest` based test passes or fails based on the summary printed by `unittest`. Note that (as long as `unittest` is installed on the target) the tests are still fully independent and you can still run them without `run-tests.py`: you just run it as usual, eg `mpremote run <test.py>`. This is very useful when creating and debugging tests. Note also that the standard test suite testing Python semantics (eg everything in `tests/basics/`) will probably never use unittest. Only more advanced tests will, and ones that are not runnable under CPython. Signed-off-by: Damien George <damien@micropython.org>

So that a failing unittest-based test has its entire log printed when using `run-tests.py --print-failures`. Signed-off-by: Damien George <damien@micropython.org>

This brings in: - umqtt.simple: add optional socket timeout to connect method - aioespnow,webrepl: use recommended network.WLAN.IF_[AP|STA] constants - unittest: allow SkipTest to work within a subTest - unittest: always use "raise" with an argument Signed-off-by: Damien George <damien@micropython.org>

This allows `unittest` to work on esp8266 boards. Signed-off-by: Damien George <damien@micropython.org>

So that this port can run unittest-based tests. Signed-off-by: Damien George <damien@micropython.org>

Signed-off-by: Damien George <damien@micropython.org>

This allows it to run parts of the test on esp8266 (or any target using axTLS). Signed-off-by: Damien George <damien@micropython.org>

Signed-off-by: Damien George <damien@micropython.org>

dpgeorge added the tests Relates to tests/ directory in source label Nov 12, 2024

dpgeorge requested review from projectgus and andrewleech November 12, 2024 05:47

dpgeorge mentioned this pull request Nov 12, 2024

tests/extmod_hardware: Add a test for machine.PWM freq and duty. #16147

Merged

dpgeorge force-pushed the tests-support-unittest branch from 1967a13 to 8007f84 Compare November 13, 2024 01:46

dpgeorge mentioned this pull request Nov 13, 2024

tests: get connect_nonblock_xfer.py test working again and simplify it #16156

Merged

dpgeorge force-pushed the tests-support-unittest branch from 8007f84 to eabcdbc Compare November 14, 2024 01:35

dpgeorge mentioned this pull request Nov 18, 2024

unittest: Always use "raise" with an argument. micropython/micropython-lib#937

Merged

projectgus requested changes Nov 19, 2024

View reviewed changes

tests/net_hosted/connect_nonblock_xfer.py Outdated Show resolved Hide resolved

tests/net_hosted/connect_nonblock_xfer.py Outdated Show resolved Hide resolved

tests/run-tests.py Show resolved Hide resolved

andrewleech approved these changes Nov 20, 2024

View reviewed changes

andrewleech reviewed Nov 20, 2024

View reviewed changes

dpgeorge force-pushed the tests-support-unittest branch from eabcdbc to a26aea5 Compare December 3, 2024 04:13

projectgus approved these changes Dec 3, 2024

View reviewed changes

stinos reviewed Dec 3, 2024

View reviewed changes

tests/run-tests.py Outdated Show resolved Hide resolved

stinos reviewed Dec 3, 2024

View reviewed changes

dpgeorge force-pushed the tests-support-unittest branch from dea0c41 to 8c4a65f Compare December 3, 2024 22:28

dpgeorge added 8 commits December 6, 2024 13:41

tests/run-tests.py: Print .out file when there is no .exp file.

a842243

So that a failing unittest-based test has its entire log printed when using `run-tests.py --print-failures`. Signed-off-by: Damien George <damien@micropython.org>

esp8266/mpconfigport: Enable function attributes.

42f37e9

This allows `unittest` to work on esp8266 boards. Signed-off-by: Damien George <damien@micropython.org>

qemu/Makefile: Include unittest in firmware.

d3d2976

So that this port can run unittest-based tests. Signed-off-by: Damien George <damien@micropython.org>

tests/ports/stm32_hardware: Convert DMA test to use unittest.

f62df1a

Signed-off-by: Damien George <damien@micropython.org>

tests/net_hosted: Convert connect-nonblock-xfer test to use unittest.

c7c3ffa

This allows it to run parts of the test on esp8266 (or any target using axTLS). Signed-off-by: Damien George <damien@micropython.org>

tests/extmod: Convert machine1.py test to use unittest.

2c80d36

Signed-off-by: Damien George <damien@micropython.org>

dpgeorge force-pushed the tests-support-unittest branch from 8c4a65f to 2c80d36 Compare December 6, 2024 02:49

dpgeorge merged commit 2c80d36 into micropython:master Dec 6, 2024
64 checks passed

dpgeorge deleted the tests-support-unittest branch December 6, 2024 03:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: add support for writing a test using `unittest` #16216

tests: add support for writing a test using `unittest` #16216

dpgeorge commented Nov 12, 2024

dpgeorge commented Nov 12, 2024

codecov bot commented Nov 12, 2024 •

edited

Loading

dpgeorge commented Nov 13, 2024

stinos commented Nov 13, 2024

dpgeorge commented Nov 13, 2024

stinos commented Nov 13, 2024

dpgeorge commented Nov 14, 2024

projectgus left a comment

andrewleech left a comment

andrewleech Nov 20, 2024

dpgeorge Dec 3, 2024

andrewleech Nov 20, 2024

dpgeorge Dec 3, 2024

agatti commented Nov 25, 2024

projectgus commented Nov 26, 2024 •

edited

Loading

projectgus commented Nov 26, 2024

agatti commented Nov 27, 2024 •

edited

Loading

dpgeorge commented Dec 3, 2024

github-actions bot commented Dec 3, 2024

dpgeorge commented Dec 3, 2024

dpgeorge commented Dec 3, 2024

projectgus left a comment

stinos Dec 3, 2024

dpgeorge Dec 3, 2024

stinos Dec 3, 2024

dpgeorge Dec 3, 2024

stinos Dec 3, 2024

dpgeorge Dec 3, 2024

stinos commented Dec 3, 2024

tests: add support for writing a test using unittest #16216

tests: add support for writing a test using unittest #16216

Conversation

dpgeorge commented Nov 12, 2024

Summary

Testing

Trade-offs and Alternatives

dpgeorge commented Nov 12, 2024

codecov bot commented Nov 12, 2024 • edited Loading

Codecov Report

dpgeorge commented Nov 13, 2024

stinos commented Nov 13, 2024

dpgeorge commented Nov 13, 2024

stinos commented Nov 13, 2024

dpgeorge commented Nov 14, 2024

projectgus left a comment

Choose a reason for hiding this comment

andrewleech left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agatti commented Nov 25, 2024

projectgus commented Nov 26, 2024 • edited Loading

projectgus commented Nov 26, 2024

agatti commented Nov 27, 2024 • edited Loading

dpgeorge commented Dec 3, 2024

github-actions bot commented Dec 3, 2024

dpgeorge commented Dec 3, 2024

dpgeorge commented Dec 3, 2024

projectgus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stinos commented Dec 3, 2024

tests: add support for writing a test using `unittest` #16216

tests: add support for writing a test using `unittest` #16216

codecov bot commented Nov 12, 2024 •

edited

Loading

projectgus commented Nov 26, 2024 •

edited

Loading

agatti commented Nov 27, 2024 •

edited

Loading