Skip to content

tests: Improve test hardware configuration by putting board-specific settings in target_wiring module #17954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

dpgeorge
Copy link
Member

Summary

There are currently a few "hardware" tests that need external board connections to be made for them to work, eg bridging a pair of pins. Eg all tests in tests/extmod_hardware need some kind of connection.

Along with the physical connections -- which are different for each board -- there needs to be corresponding Python settings, eg which UART instance to use and which pins for TX/RX.

These settings are currently hard-coded in each test file. That has a few problems:

  • settings are repeated, eg all the UART tests have pretty much the same settings code duplicated across them
  • changing the settings means changing many files
  • adding a new board means adding a lot of code
  • the tests get bigger and bigger with each new board that they support, meaning they may not fit on targets with a small amount of RAM (that's already the case with tests/extmod_hardware/machine_pwm.py)
  • if you have a custom board you have to manually edit the test to add your own settings

This PR aims to solve all the above problems by splitting the board-specific settings out into separate files, one for each board (or port). They are placed in the tests/target_wiring/ directory. The run-tests.py test runner then loads the appropriate configuration for the target that is being tested, sends it to the board so it's available as import target_wiring (without needing a filesystem), and then executes the test (only tests that need target_wiring have it loaded).

This PR adds support for this target_wiring module, and converts all the existing machine.UART tests to use it.

Testing

Tested locally on PYBV10 and ESP32-S2. Still needs further testing once we decide this is the way forward.

Trade-offs and Alternatives

Although the idea is straightforward, there were many decisions to make in the implementation:

  1. Selection of the correct target_wiring module follows a hierarchy: if an exact match for the board name is found (via sys.implementation._build) then that's used, otherwise there's a possible wildcard match on the board name (eg for PYB* boards, they can all use the same config), otherwise there's a match on the port name (eg rp2 boards can all use the same config). This allows a lot of flexibility (maybe too much?).
  2. I used a trick to send the target_wiring module to the board without using the filesystem or VFS hook. It just evaluates a new class called target_wiring with the body of the class the script with the settings. This should be pretty minimal and work on targets without a filesystem/VFS.
  3. The same configuration is reused for extmod/machine_uart*.py tests ... they are not strictly hardware tests (they don't require external connections) but they do need to know which UART instance to use, and they can reuse the same instance used by extmod_hardware/machine_uart*.py tests. Hopefully this won't be an issue, but it does confuse somewhat which tests require target_wiring configuration and which don't (ie it's no longer as simple as saying that only extmod_hardware/ tests need target_wiring).
  4. Eventually there may be other UART tests that need to be connected to a separate board. As such I named the config options uart_loopback_args and uart_loopback_kwargs. This shows the general problem that the approach may not scale well if lots of tests need subtly different configurations, eg I2CTarget will need a loopback configuration (4 pins) and a non-loopback configuration (another 2 pins). To make it work, the config for these either need to be named differently, or selected very carefully so the pins can overlap.
  5. Tests like extmod_hardware/machine_pwm.py can reuse the UART loopback pins, but probably they should be named differently, in case they can't be reused.
  6. Need to think about how to handle zephyr boards, how to detect which configuration to use (need to either add special handling in run-tests.py for zephyr, or expose sys.implementation._build on that port).
  7. To manually run a test using this new target_wiring module (without using run-tests.py) requires the user to explicitly copy across the correct target_wiring.py file to their device.
  8. If there's a target_wiring.py on the device then it is overridden by the one injected by run-tests.py. I think this is desired behaviour, so you don't have a stale/old file on there that messes up the test.
  9. Need to add a way in run-tests.py for the user to select/override which target_wiring.py module to use, eg for custom boards, eg for Octoprobe.

Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge added the tests Relates to tests/ directory in source label Aug 19, 2025
@dpgeorge dpgeorge requested a review from projectgus August 19, 2025 05:21
@dpgeorge
Copy link
Member Author

@projectgus as we discussed, this isn't 100% complete but should be good enough for a first review.

@hmaerki this should solve a lot of our issues with Octoprobe and configuring the wiring for each board.

Copy link

codecov bot commented Aug 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.38%. Comparing base (989abae) to head (2e79719).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #17954   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22296    22296           
=======================================
  Hits        21937    21937           
  Misses        359      359           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Damien George <damien@micropython.org>
This is, eg, `rpi_pico`.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge force-pushed the tests-add-target-wiring branch from 685c657 to 29854e3 Compare August 19, 2025 06:47
Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
Copy link
Member Author

8. If there's a target_wiring.py on the device then it is overridden by the one injected by run-tests.py. I think this is desired behaviour, so you don't have a stale/old file on there that messes up the test.

Actually, maybe the other way around is better: if there's a target_wiring.py on the filesystem on the device then that should take precedence? That way, you can set up your own boards for testing with custom wiring and run the tests as normal (without any special arguments) and they just work.

The alternative would be new CLI options to run-tests.py to override the behaviour, eg run-tests.py --skip-target-wiring.

Copy link
Contributor

@projectgus projectgus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach looks very good and simple to me, cuts out a lot of complex code!

It also lowers the bar for adding more tests, I've hesitated to add small hardware tests in the past because it seems "expensive" to copy the big block of hardware config and pay the cost of maintaining it.

There is a bit of new user documentation we should probably repeat even though it's redundant, for example in each file where we have # For UART loopback tests we should be explicit about # For UART loopback tests (connect these pins together).

Actually, maybe the other way around is better: if there's a target_wiring.py on the filesystem on the device then that should take precedence? That way, you can set up your own boards for testing with custom wiring and run the tests as normal (without any special arguments) and they just work.

I think that's a good approach (test runner defaults to a module on the target filesystem if it's there). Two additional suggestions:

  • run-tests.py should log a line i.e. Using host file PATH as target_wiring or Using target_wiring module from target so you never have to guess which file is being used.
  • Add a command line option for run-test.py --target-wiring PATH so can also override the selection for custom boards or experiments without needing to copy anything to the target.

@dpgeorge
Copy link
Member Author

Thanks for looking at this!

It also lowers the bar for adding more tests, I've hesitated to add small hardware tests in the past because it seems "expensive" to copy the big block of hardware config and pay the cost of maintaining it.

Yes, definitely will make it much easier to add new hardware-based tests.

I think that's a good approach (test runner defaults to a module on the target filesystem if it's there).

OK, I'll implement that.

  • run-tests.py should log a line i.e. Using host file PATH as target_wiring or Using target_wiring module from target so you never have to guess which file is being used.

It actually already does log this info (which file is used from the host), it's in the first line that also includes other target info (eg arch, threading, unicode, float). Do you think it should be a separate line?

(Also, I think I should make it so that info is only printed if it's going to run one of the tests that requires target_wiring.py. That provides further feedback about what's actually going to be tested.)

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
Copy link
Member Author

Added a commit that:

  • only detects target_wiring if needed, ie if it's going to run a test that needs it
  • if import target_wiring passes on the target then don't inject anything, to let the target override it (eg via a file on its filesystem, or frozen into the firmware)
  • print an explicit message on a separate line about which target_wiring is used

Example output when host file is used:

$  ./run-tests.py -t a0 extmod/machine_uart_*.py
platform=pyboard arch=armv7emsp inlineasm=thumb float=32-bit unicode
using target_wiring module from host file PYBx.py
skip  extmod/machine_uart_irq_txidle.py
pass  extmod/machine_uart_tx.py 
1 tests performed (3 individual testcases)
1 tests passed
1 tests skipped: extmod/machine_uart_irq_txidle.py

and when there's already a file on the target filesystem:

./run-tests.py -t a0 extmod/machine_uart_*.py
platform=pyboard arch=armv7emsp inlineasm=thumb float=32-bit unicode
using target_wiring module from target's filesystem
skip  extmod/machine_uart_irq_txidle.py
pass  extmod/machine_uart_tx.py 
1 tests performed (3 individual testcases)
1 tests passed
1 tests skipped: extmod/machine_uart_irq_txidle.py

Signed-off-by: Damien George <damien@micropython.org>
@projectgus
Copy link
Contributor

It actually already does log this info (which file is used from the host), it's in the first line that also includes other target info (eg arch, threading, unicode, float). Do you think it should be a separate line?

Oh sorry, I didn't see that was already in there when I reviewed the code. I think that's also fine, if you prefer to do it that way. It might actually be easier to read out quickly that way.

@projectgus
Copy link
Contributor

(Also, I think I should make it so that info is only printed if it's going to run one of the tests that requires target_wiring.py. That provides further feedback about what's actually going to be tested.)

Fully agree!

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
Copy link
Member Author

Added a commit that puts the target_wiring information back on the first line with all the other target info. (Easy to revert that change if we don't like it.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Relates to tests/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants