tests: adapt tests so they can use `tests_utils_interactive_sync` by fjmolinas · Pull Request #12867 · RIOT-OS/RIOT

fjmolinas · 2019-12-03T19:27:03Z

Contribution description

This PR removes the need to reset after cleanterm from the remaining tests that where not using tests_utils_interactive_sync. For some of them it was just a matter of removing DISABLE_MODULE+=tests_utils_interactive_sync

Two of them had to me modified a little more:
- tests/gnrc_rpl_srh
- tests/gnrc_ipv6_ext_frag

~~tests/stdin is missing but should be fixed in #12816~~ uses a different sync method.

Testing procedure

All tests ares still working:

sudo make -C tests/gnrc_ipv6_ext_frag/ test

...................SUCCESS

sudo make -C tests/gnrc_ipv6_ext/ test --no-print-directory

...................SUCCESS

sudo make -C tests/gnrc_rpl_srh/ test --no-print-directory

..............SUCCESS

sudo make -C tests/gnrc_tcp/ test --no-print-directory

Details

03-send_data.py: success

06-receive_data_closed_conn.py: success

- test_short_payload SUCCESS

- test_short_header SUCCESS

- test_send_ack_instead_of_syn SUCCESS

- test_option_parsing_term SUCCESS

05-garbage-pkts.py: success

04-receive_data.py: success

02-conn_lifecycle_as_server.py: success

01-conn_lifecycle_as_client.py: success

make -C tests/shell/ --no-print-directory test

Details

/home/francisco/workspace/RIOT/tests/shell/bin/native/tests_shell.elf  
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.01-devel-1190-g7817c-pr_tests_no_reset_dep)
test_shell.
> start_test
start_test
[TEST_START]
> end_test
end_test
[TEST_END]
> 

123456789012345678901234567890123456789012345678901234567890

> 
> 123456789012345678901234567890123456789012345678901234567890
shell: command not found: 123456789012345678901234567890123456789012345678901234567890
> unknown_command
unknown_command
shell: command not found: unknown_command
> help
help
Command              Description
---------------------------------------
start_test           starts a test
end_test             ends a test
echo                 prints the input command
reboot               Reboot the node
ps                   Prints information about running threads.
app_metadata         Returns application metadata
> echo a string
echo a string
"echo""a""string"
> ps
ps
        pid | state    Q | pri 
          1 | pending  Q |  15
          2 | running  Q |   7
> garbage1234
help
garbage1234
> 
> help
Command              Description
---------------------------------------
start_test           starts a test
end_test             ends a test
echo                 prints the input command
reboot               Reboot the node
ps                   Prints information about running threads.
app_metadata         Returns application metadata
> reboot
reboot


                !! REBOOT !!

RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.01-devel-1190-g7817c-pr_tests_no_reset_dep)
test_shell.

sudo make -C tests/gnrc_sock_dns/ test --no-print-directory

..............SUCCESS

Issues/PRs references

Part of #12448.
Related to #12862.

fjmolinas · 2019-12-03T19:27:38Z

@miri64 please let me know how you feel about the gnrc_ipv6_ext_frag changes to be sure I haven't bodged that one.

miri64

@miri64 please let me know how you feel about the gnrc_ipv6_ext_frag changes to be sure I haven't bodged that one.

LGTM code-wise, however you bodged some styling ;-)

tests/gnrc_ipv6_ext_frag/main.c

miri64

Some more issues I figured not related to gnrc_ipv6_ext_frag:

tests/gnrc_rpl_srh/main.c

tests/shell/tests/01-run.py

aabadie · 2019-12-11T14:46:16Z

Looks good but I just have one question: why not also removed the reset before term in this PR ? IIUC, everything is in the place in the tests and it shouldn't too much change in testrunner. And this way, no need to provide this in a follow-up PR.

@miri64, I think your comments are addressed, so are you ok with the current state ?

fjmolinas · 2019-12-11T15:11:54Z

Looks good but I just have one question: why not also removed the reset before term in this PR ? IIUC, everything is in the place in the tests and it shouldn't too much change in testrunner. And this way, no need to provide this in a follow-up PR.

I'm guessing you mean reset after term? Anyway, they entail different kind of testing, and this are quite independent.

aabadie

This PR is all good for me. @miri64, mind to ACK as well ?

tests/gnrc_ipv6_ext_frag/main.c

Using the shell to run unittests allows not needing to wait for a string at the start of the test which makes the test independent having the application reset after the terminal is open. The same goes for triggering sending UDP test pkts.

Using the shell to run unittests allows not needing to wait for a string at the start of the test which makes the test independent having the application reset after the terminal is open.

miri64

All comments were addressed. Please squash

fjmolinas · 2019-12-12T14:18:06Z

All green!

fjmolinas · 2019-12-12T14:48:19Z

@miri64 @aabadie thanks for the review!

fjmolinas · 2019-12-13T13:37:08Z

@kaspar030 I'm suspecting this PR might have broke the shell test for esp32wroom, could you confirm? (I don't have any esp32 to test.)
(I realized post-merging that the tests didn't run even though I set the CI:run-tests label)

kaspar030 · 2019-12-16T10:58:50Z

@kaspar030 I'm suspecting this PR might have broke the shell test for esp32wroom, could you confirm? (I don't have any esp32 to test.)

Discussed offline: the shell test doesn't sync at the prompt at the beginning. We think that this was flakey on esp32 all the time, unrelated to this PR.

(I realized post-merging that the tests didn't run even though I set the CI:run-tests label)

That's actually an issue, there seems to be a race when both labels are set at the same time. "CI:run-tests" needs to be set before "CI: ready to build".

fjmolinas added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: tests Area: tests and testing framework labels Dec 3, 2019

fjmolinas requested review from aabadie and miri64 December 3, 2019 19:27

miri64 requested changes Dec 4, 2019

View reviewed changes

tests/gnrc_ipv6_ext_frag/main.c Outdated Show resolved Hide resolved

tests/gnrc_ipv6_ext_frag/main.c Outdated Show resolved Hide resolved

miri64 requested changes Dec 4, 2019

View reviewed changes

tests/gnrc_rpl_srh/main.c Outdated Show resolved Hide resolved

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

fjmolinas changed the title ~~tests: remove reset dependency~~ tests: adapt tests so they can use tests_utils_interactive_sync Dec 11, 2019

aabadie approved these changes Dec 12, 2019

View reviewed changes

miri64 reviewed Dec 12, 2019

View reviewed changes

tests/gnrc_ipv6_ext_frag/main.c Outdated Show resolved Hide resolved

fjmolinas added 4 commits December 12, 2019 10:13

tests/shell: fix reset dependency

4117b45

tests/gnrc_rpl_srh: run unittests through shell

eb9d27d

Using the shell to run unittests allows not needing to wait for a string at the start of the test which makes the test independent having the application reset after the terminal is open.

tests/gnrc_%: use tests_utils_interactive_sync

341a4b5

miri64 approved these changes Dec 12, 2019

View reviewed changes

fjmolinas force-pushed the pr_tests_no_reset_dep branch from 9021caa to 341a4b5 Compare December 12, 2019 09:14

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 Dec 12, 2019

miri64 merged commit 26a1348 into RIOT-OS:master Dec 12, 2019

fjmolinas deleted the pr_tests_no_reset_dep branch December 12, 2019 14:48

fjmolinas mentioned this pull request Dec 12, 2019

dist/pythonlibs/testrunner: reset before term #12862

Merged

fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019

Conversation

fjmolinas commented Dec 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

fjmolinas commented Dec 3, 2019

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aabadie commented Dec 11, 2019

Uh oh!

fjmolinas commented Dec 11, 2019

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

fjmolinas commented Dec 12, 2019

Uh oh!

fjmolinas commented Dec 12, 2019

Uh oh!

fjmolinas commented Dec 13, 2019

Uh oh!

kaspar030 commented Dec 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fjmolinas commented Dec 3, 2019 •

edited

Loading