Skip to content
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

test_utils_interactive_sync: add a helper for synchronizing tests #11875

Merged
merged 7 commits into from
Aug 15, 2019

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Jul 19, 2019

Contribution description

Add an implementation that waits for 's' to print 'START' and return.
If 'r' is given is prints 'READY' to allow querying for state.

This also provides a helper to use it in testrunner tests.

It should allow fixing tests where the input before reset is matched and on boards that cannot reset.

I used the function for tests/cond_order as example as it was failing on cc2650-launchpad during 2019.07-RC1 testing.

Testing procedure

Run the tests/cond_order on a board that:

This broke the test on cc2650-launchpad as shown on this output as the lines before resetting are already matched by the test and then the board resets:

make RIOT_CI_BUILD=1 --no-print-directory -C /srv/ilab-builds/workspace/git/RIOT/tests/cond_order test
make RIOT_CI_BUILD=1 CC_NOCOLOR=1 --no-print-directory -C /srv/ilab-builds/workspace/git/RIOT/tests/cond_order test
/srv/ilab-builds/workspace/git/RIOT/dist/tools/pyterm/pyterm -p "/dev/riot/ttyCC2650_LAUNCHPAD" -b "115200"
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2019-07-18 19:29:49,793 - INFO # Connect to serial port /dev/riot/ttyCC2650_LAUNCHPAD
2019-07-18 19:29:50,796 - INFO # est)
Welcome to pyterm!
Type '/exit' to exit.
2019-07-18 19:29:50,798 - INFO # Condition variable order test
2019-07-18 19:29:50,800 - INFO # Please refer to the README.md for more information
2019-07-18 19:29:50,801 - INFO # 
2019-07-18 19:29:50,802 - INFO # T3 (prio 6): waiting on condition variable now
2019-07-18 19:29:50,804 - INFO # T4 (prio 4): waiting on condition variable now
2019-07-18 19:29:50,806 - INFO # T5 (prio 0): waiting on condition variable now
2019-07-18 19:29:50,808 - INFO # T6 (prio 2): waiting on condition variable now
2019-07-18 19:29:50,810 - INFO # T7 (prio 1): waiting on condition variable now
2019-07-18 19:29:50,811 - INFO # First batch was signaled
2019-07-18 19:29:50,812 - INFO # T5 (prio 0): condition variable was signaled now
2019-07-18 19:29:50,814 - INFO # T7 (prio 1): condition variable was signaled now
2019-07-18 19:29:50,816 - INFO # T6 (prio 2): condition variable was signaled now
2019-07-18 19:29:50,817 - INFO # First batch has woken up
2019-07-18 19:29:50,818 - INFO # Second batch was signaled
2019-07-18 19:29:52,829 - INFO # T4 (prio 4): condition variablmain(): This is RIOT! (Version: buildtest)
2019-07-18 19:29:52,831 - INFO # Condition variable order test
2019-07-18 19:29:52,836 - INFO # Please refer to the README.md for more information
2019-07-18 19:29:52,836 - INFO # 
2019-07-18 19:29:52,840 - INFO # T3 (prio 6): waiting on condition variable now
2019-07-18 19:29:52,844 - INFO # T4 (prio 4): waiting on condition variable now
2019-07-18 19:29:52,848 - INFO # T5 (prio 0): waiting on condition variable now
2019-07-18 19:29:52,852 - INFO # T6 (prio 2): waiting on condition variable now
2019-07-18 19:29:52,856 - INFO # T7 (prio 1): waiting on condition variable now
2019-07-18 19:29:52,858 - INFO # First batch was signaled
2019-07-18 19:29:52,863 - INFO # T5 (prio 0): condition variable was signaled now

Traceback (most recent call last):
  File "/srv/ilab-builds/workspace/git/RIOT/tests/cond_order/tests/01-run.py", line 44, in <module>
    sys.exit(run(testfunc))
  File "/srv/ilab-builds/workspace/git/RIOT/dist/pythonlibs/testrunner/__init__.py", line 23, in run
    testfunc(child)
  File "/srv/ilab-builds/workspace/git/RIOT/tests/cond_order/tests/01-run.py", line 32, in testfunc
    assert(int(child.match.group(1)) > last)
AssertionError
/srv/ilab-builds/workspace/git/RIOT/tests/cond_order/../../Makefile.include:604: recipe for target 'test' failed
make: *** [test] Error 1

Return value: 2

This also allows testing on esp32 (when disabling RESET as it gets locked).
The test fails due to a different priority on T3 but at least can run (test does not handle that THREAD_PRIORITY_MAIN can be different.

RIOT_CI_BUILD=1 RESET=true BOARD=esp32-wroom-32 make -C tests/cond_order/ flash test
Welcome to pyterm!
Type '/exit' to exit.
2019-07-19 16:05:08,789 - INFO # READY
s
2019-07-19 16:05:08,847 - INFO # START
2019-07-19 16:05:08,850 - INFO # T3 (prio 14): waiting on condition variable now
2019-07-19 16:05:08,853 - INFO # T4 (prio 4): waiting on condition variable now
2019-07-19 16:05:08,858 - INFO # T5 (prio 0): waiting on condition variable now
2019-07-19 16:05:08,864 - INFO # T6 (prio 2): waiting on condition variable now
2019-07-19 16:05:08,865 - INFO # T7 (prio 1): waiting on condition variable now
2019-07-19 16:05:08,869 - INFO # First batch was signaled
2019-07-19 16:05:08,875 - INFO # T5 (prio 0): condition variable was signaled now
2019-07-19 16:05:08,875 - INFO # T7 (prio 1): condition variable was signaled now
2019-07-19 16:05:08,881 - INFO # T6 (prio 2): condition variable was signaled now
2019-07-19 16:05:08,886 - INFO # First batch has woken up
2019-07-19 16:05:08,886 - INFO # Second batch was signaled
2019-07-19 16:05:08,891 - INFO # T4 (prio 4): condition variable was signaled now
2019-07-19 16:05:08,897 - INFO # T3 (prio 14): condition variable was signaled now
2019-07-19 16:05:08,897 - INFO # Second batch has woken up
2019-07-19 16:05:08,897 - INFO #
2019-07-19 16:05:08,900 - INFO # Test END, check the order of priorities above.
Timeout in expect script at "child.expect(u"T%i \(prio %i\): waiting on condition variable now" % (k, thread_prio[k]))" (tests/cond_order/tests/01-run.py:29)

Where in master the output was completely missed:

2019-07-19 16:06:29,141 - INFO # Connect to serial port /dev/riot/ttyESP32_WROOM_32
Welcome to pyterm!
Type '/exit' to exit.
Timeout in expect script at "child.expect(u"T%i \(prio %i\): waiting on condition variable now" % (k, thread_prio[k]))" (tests/cond_order/tests/01-run.py:25)

All the modified tests still work (would be checked by murdock when running tests).

Issues/PRs references

Running tests on boards for 2019.07-RC1 tests RIOT-OS/Release-Specs#128 (comment)

Work with @MrKevinWeiss to allow running tests on esp32 that cannot reset using esptool when term is open.

@cladmi
Copy link
Contributor Author

cladmi commented Jul 19, 2019

@smlng would that help for your case from #8211 ?

@miri64 miri64 requested a review from MrKevinWeiss July 30, 2019 17:35
@miri64 miri64 added Area: tests Area: tests and testing framework Area: tools Area: Supplementary tools Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Jul 30, 2019
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

I was able to reproduce the issue with ccs2538dk, this PR also fixes the issue for me.

master: RIOT_CI_BUILD=1 make -C tests/cond_order/ BOARD=cc2538dk flash
make: Entering directory '/home/francisco/workspace/RIOT/tests/cond_order'
/home/francisco/workspace/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyUSB1" -b "115200"
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2019-08-08 16:13:07,023 - INFO # Connect to serial port /dev/ttyUSB1
Welcome to pyterm!
Type '/exit' to exit.
Timeout in expect script at "child.expect(u"T%i \(prio %i\): waiting on condition variable now" % (k, thread_prio[k]))" (tests/cond_order/tests/01-run.py:25)

/home/francisco/workspace/RIOT/tests/cond_order/../../Makefile.include:597: recipe for target 'test' failed
make: *** [test] Error 1
make: Leaving directory '/home/francisco/workspace/RIOT/tests/cond_order'
This PR: RIOT_CI_BUILD=1 make -C tests/cond_order/ BOARD=cc2538dk flash
make: Entering directory '/home/francisco/workspace/RIOT/tests/cond_order'
r
/home/francisco/workspace/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyUSB1" -b "115200"
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2019-08-08 16:14:21,601 - INFO # Connect to serial port /dev/ttyUSB1
Welcome to pyterm!
Type '/exit' to exit.
2019-08-08 16:14:25,809 - INFO # READY
s
2019-08-08 16:14:25,874 - INFO # START
2019-08-08 16:14:25,875 - INFO # T3 (prio 6): waiting on condition variable now
2019-08-08 16:14:25,877 - INFO # T4 (prio 4): waiting on condition variable now
2019-08-08 16:14:25,890 - INFO # T5 (prio 0): waiting on condition variable now
2019-08-08 16:14:25,890 - INFO # T6 (prio 2): waiting on condition variable now
2019-08-08 16:14:25,891 - INFO # T7 (prio 1): waiting on condition variable now
2019-08-08 16:14:25,891 - INFO # First batch was signaled
2019-08-08 16:14:25,892 - INFO # T5 (prio 0): condition variable was signaled now
2019-08-08 16:14:25,909 - INFO # T7 (prio 1): condition variable was signaled now
2019-08-08 16:14:25,912 - INFO # T6 (prio 2): condition variable was signaled now
2019-08-08 16:14:25,913 - INFO # First batch has woken up
2019-08-08 16:14:25,914 - INFO # Second batch was signaled
2019-08-08 16:14:25,922 - INFO # T4 (prio 4): condition variable was signaled now
2019-08-08 16:14:25,923 - INFO # T3 (prio 6): condition variable was signaled now
2019-08-08 16:14:25,923 - INFO # Second batch has woken up
2019-08-08 16:14:25,923 - INFO # 

make: Leaving directory '/home/francisco/workspace/RIOT/tests/cond_order'

Can you adapt tests/xtimer_usleep and tests/posix_time tom make use of this?

Also I think having the terminal output in the commit message is overkill, the PR is referenced by the commit and the output can be found there.

@cladmi
Copy link
Contributor Author

cladmi commented Aug 12, 2019

Can you adapt tests/xtimer_usleep and tests/posix_time tom make use of this?

Also I think having the terminal output in the commit message is overkill, the PR is referenced by the commit and the output can be found there.

Sure I will add them too and add a note for the commit message.

@cladmi
Copy link
Contributor Author

cladmi commented Aug 12, 2019

The tests/libfixmath test can also benefit from it as we were using a hack to use an xtimer delay to prevent being flooded. I include it here as we also modified other tests but can move it elsewhere if preferred.

@cladmi
Copy link
Contributor Author

cladmi commented Aug 12, 2019

I noticed I did not have any other fixup/squash in this pr so I squashed the commit message. The code change is the same.

The new commit message can be compared b8cbda4 and old was e35cd86

@fjmolinas
Copy link
Contributor

Looks good. There aren't any code changes since tests/cond_order issue with the cc2560 since I last tested. For the changed tests, murdock should take care of it.

@fjmolinas fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Aug 12, 2019
Add an implementation that waits for 's' to print 'START' and return.
If 'r' is given is prints 'READY' to allow querying for state.

The help and answered string have to be different to not match the other.
Using puts/getchar was smaller than using `stdio_read/stdio_write` on the
example I tested with `esp32`.
Use test_utils_interactive_sync for synchronizing some case treat
the output before `reset` as the start of the test,
which fails for some boards/configurations.
Use test_utils_interactive_sync for synchronizing test instead of the
custom 'getchar' handling.
Use test_utils_interactive_sync for synchronizing test instead of the
custom 'getchar' handling.
@cladmi
Copy link
Contributor Author

cladmi commented Aug 15, 2019

I was failing due to failing compilation in tests/libfixmath no idea why so I rebased.

ARRAY_SIZE is implicitly imported by "xtimer.h" which will be removed of
this file.
Replace the 'xtimer_sleep' hack to prevent flooding at startup to use
test_utils_interactive_sync for synchronizing.
@cladmi
Copy link
Contributor Author

cladmi commented Aug 15, 2019

Fixed I added a commit that includes "kernel_defines.h" to have ARRAY_SIZE as it was implicitly imported by "xtimer.h"

@cladmi
Copy link
Contributor Author

cladmi commented Aug 15, 2019

I also pushed-force to have the correct order already.

Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

Everything is looking good, ACK & GO.

@fjmolinas fjmolinas merged commit 5653004 into RIOT-OS:master Aug 15, 2019
@cladmi
Copy link
Contributor Author

cladmi commented Aug 16, 2019

Thank you for the review :) Less failing tests.

@cladmi cladmi deleted the pr/test_utils/sync branch August 16, 2019 09:15
@kb2ma kb2ma added this to the Release 2019.10 milestone Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants