dist/pythonlibs/testrunner: add reset option, reset before cleanterm#12520
dist/pythonlibs/testrunner: add reset option, reset before cleanterm#12520fjmolinas wants to merge 8 commits intoRIOT-OS:masterfrom
cleanterm#12520Conversation
For some board `make reset` is only possible if a serial connection is not already open or its execution might disrupt it. This casuses some tests to fail since before running a test the board is reset. run(...) now has a reset option to disable this. - The default value is `MAKE_RESET_WITH_TERM` env variable value which itself defaults to True.
- Some boards can't be resetted when the serial port is busy, e.g. when `make term` was called, since `make test` resets the device after opening a terminal this should be avoided.
|
Another good idea. I think also some esps can benefit from this. |
|
|
||
| # Some boards can't be resetted during test since this resets the serial | ||
| # connection. Default to 1 for those boards. | ||
| MAKE_RESET_WITH_TERM = int(os.environ.get('MAKE_RESET_WITH_TERM', 1)) |
There was a problem hiding this comment.
How about we turn this into a variable holding the whole command?
TESTRUNNER_RESET_CMD = "make reset"
There was a problem hiding this comment.
I pushed changes adding make reset before clean term. I changed the variable name to TESTRUNNER_RESET_SYNC I think the expected behavior is more explicit with this name
I can still change it to TESTRUNNER_RESET_SYNC_CMD if you prefer.
Could you give me more details about the issues |
|
I think there was a problem just resetting while connected to the terminal. This would solve it. Of course @cladmi knows more but now that he is gone I can try to dig into it. |
- Some boards can't be resetted when the serial port is busy, e.g. when `make term` was called, since `make test` resets the device after opening a terminal this should be avoided. Export a variable to `test` target alowing to disable reseting after the terminal is opened.
For some board `make reset` is only possible if a serial connection is not already open or its execution might disrupt it. This casuses some tests to fail since before running a test the board is reset. The reset is not performed is TESTRUNNER_RESET_SYNC env variable is set.
cleanterm
|
@MrKevinWeiss @kaspar030 I made some changes to the implementation after @kaspar030 comment. I updated the PR description accordingly. I basically also added a reset before |
Note, keeping the second reset is temporary and only while tests are still depending on resetting after |
|
If I don't specify TESTRUNNER_RESET_SYNC=1 then it doesn't work
|
Fixed, it wasn't handling an empty value properly. |
aabadie
left a comment
There was a problem hiding this comment.
Some comments but needs more testing.
| # default value (10) | ||
| TIMEOUT = int(os.environ.get('RIOT_TEST_TIMEOUT') or 10) | ||
|
|
||
|
|
There was a problem hiding this comment.
unrelated change (which introduces a pep8 btw)
| DEBUGGER_FLAGS = $(BINDIR) $(ELFFILE) | ||
| OBJDUMPFLAGS += --disassemble --source --disassembler-options=force-thumb | ||
|
|
||
| # remote use cc2538-bsl attaching to the serial PORT to reset the board. If |
There was a problem hiding this comment.
This comment is hard to read. Maybe (with typos fixed):
remote uses cc2538-bsl attached to the serial PORT to reset the board. If 'make reset' is called when the terminal is opened, the device switches to bootloader mode and the next commands will fail, leaving the device in bootloader mode. To avoid this problem, the use of reset as sync method is disabled.
|
|
||
| # Resetting the board after the terminal is opened is used by the testrunner | ||
| # to sync the device (application under test), and the test script. This can't | ||
| # be done for some boards so provide a way of disabling the behavior. |
There was a problem hiding this comment.
a way to disable this behavior
There was a problem hiding this comment.
Maybe also add: "This behavior is enabled by default" ?
| # need to reset after opening the terminal because most tests use this as a | ||
| # way to sync the device and the test. As long as this is still the behavior | ||
| # we keep both resets. | ||
| try: |
There was a problem hiding this comment.
Maybe move the try/except logic in a separate private function outside setup_child ? This would avoid some code duplication.
Why not only reset here if TESTRUNNER_RESET_SYNC is disabled ? Otherwise the device under test is reset twice is TESTRUNNER_RESET_SYNC is enabled.
|
Went with #12862 instead. |
Contribution description
For some board
make resetis only possible if a serial connection is not already open or its execution might disrupt it. This causes some tests to fail since before running a test the boardis reset.
This PR introduces two changes to address this:
make cleantermto ensure the application is always reset so one could eventually callmake testmultiple times.make cleanterm(which used as a sync feature) optional withTESTRUNNER_RESET_SYNCI include both changes in the same PR because I think it is important to always rest the board before flashing.
This is not enough to run all tests with boards like
remote-revbsince in absence of a reset there is no way to sync the test and the device. If also using #12461, tests failing go down to 14 (from 100%).Testing procedure
Since the default behavior is the same testing can be performed on boards that do need this feature, e.g.: remote-revb:
TESTRUNNER_RESET_SYNC=1TESTRUNNER_RESET_SYNC=1 make -C tests/xtimer_usleep BOARD=remote-revb flash test
make -C tests/xtimer_usleep BOARD=remote-revb flash test
Issues/PRs references
Part of #12448