Skip to content

tests: add interactive_sync adapted to shell #13613

Merged
aabadie merged 4 commits intoRIOT-OS:masterfrom
fjmolinas:pr_light_shell_interactive_sync
Mar 17, 2020
Merged

tests: add interactive_sync adapted to shell #13613
aabadie merged 4 commits intoRIOT-OS:masterfrom
fjmolinas:pr_light_shell_interactive_sync

Conversation

@fjmolinas
Copy link
Copy Markdown
Contributor

@fjmolinas fjmolinas commented Mar 11, 2020

Contribution description

This PR changes the way Interactive Sync is done when the shell is present, instead of using new commands it prompts the shell with a \n until > is received as an answer

This will reduce the size of a lot of test applications.

Testing procedure

  • Run all tests using shell on multiple boards (I'll run it on my setup as soon as the PR is open)

  • Green murdock

  • tests/periph_wdt and tests/struc_tm_utility/ should still work:

PASSED target time: 128[ms], actual_time: 128.534[ms]
PASSED target time: 512[ms], actual_time: 502.025[ms]
PASSED target time: 1024[ms], actual_time: 1002.731[ms]
PASSED target time: 8192[ms], actual_time: 8001.558[ms]
TEST PASSED

BOARD=samr21-xpro make -C tests/struct_tm_utility/ flash test

>  day 2018 11 28
What weekday was 2018-11-28? The 332(th) day of the year was a WED.
day 2016 2 29
>  day 2016 2 29
What weekday was 2016-02-29? The 60(th) day of the year was a MON.
day 2017 2 29
>  day 2017 2 29
The supplied date is invalid, but no error should occur.
  • tested with miniterm, socat and pyterm, can someone test for rtt?

Issues/PRs references

Fixes compilation issues in #13152

@fjmolinas fjmolinas added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: tests Area: tests and testing framework CI: run tests If set, CI server will run tests on hardware for the labeled PR CI: disable test cache If set, CI will always run all tests regardless of whether they have been run successfully before labels Mar 11, 2020
@fjmolinas fjmolinas requested a review from miri64 March 11, 2020 09:22
@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 11, 2020
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Mar 11, 2020 •
edited
Loading

triggering build since there is no queue

@fjmolinas fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 11, 2020
kaspar030
kaspar030 reviewed Mar 11, 2020
View reviewed changes
miri64
miri64 previously requested changes Mar 11, 2020
View reviewed changes
@fjmolinas fjmolinas mentioned this pull request Mar 11, 2020
Merged
miri64
miri64 reviewed Mar 11, 2020
View reviewed changes
@miri64 miri64 dismissed their stale review March 11, 2020 16:15

Comments were addressed.

miri64
miri64 reviewed Mar 11, 2020
View reviewed changes
@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 12, 2020
fjmolinas
fjmolinas commented Mar 16, 2020
View reviewed changes
dist/pythonlibs/testrunner/utils.py

Interacts through input to wait for node being ready.
"""
_test_utils_interactive_sync(child, retries, delay, '\n', '>')
Copy link
Copy Markdown
Contributor Author

@fjmolinas fjmolinas Mar 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Pyterm doesn't do anything on an empty line... so this doesn't work o pyterm.

Copy link
Copy Markdown
Contributor Author

@fjmolinas fjmolinas Mar 16, 2020

Choose a reason for hiding this comment

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

I was using term isntead of cleanterm which is used in tests

@fjmolinas fjmolinas force-pushed the pr_light_shell_interactive_sync branch from 87349a0 to f8c3426 Compare March 16, 2020 10:55
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Mar 16, 2020

Rebased, fixed failing tests. I added a test_utils_interactive_sync_shell pseudomodule.

@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Mar 16, 2020

@miri64 there are two failures but unrelated, could you give this a second look?

@aabadie aabadie self-assigned this Mar 17, 2020
@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 17, 2020

Are you sure https://ci.riot-os.org/RIOT-OS/RIOT/13613/f8c342645f36d85f6e0a2192b27c112c1f3d940f/output/run_test/tests/netstats_l2/esp32-wroom-32:gnu.txt is unrelated? With https://ci.riot-os.org/RIOT-OS/RIOT/13613/f8c342645f36d85f6e0a2192b27c112c1f3d940f/output/run_test/examples/suit_update/nrf52dk:gnu.txt I agree, however

/bin/sh: 1: arm-none-eabi-gcc: not found
/tmp/dwq.0.6171555738823028/7c4b6d9e5c842d0fd6803d8d5f2d270d/dist/tools/jlink/jlink.sh flash /tmp/dwq.0.6171555738823028/7c4b6d9e5c842d0fd6803d8d5f2d270d/examples/suit_update/bin/nrf52dk/suit_update-slot0-extended.bin

This should be easily fixable on the worker @kaspar030 or am I mistaking? ;-)

@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Mar 17, 2020

Are you sure https://ci.riot-os.org/RIOT-OS/RIOT/13613/f8c342645f36d85f6e0a2192b27c112c1f3d940f/output/run_test/tests/netstats_l2/esp32-wroom-32:gnu.txt is unrelated? With https://ci.riot-os.org/RIOT-OS/RIOT/13613/f8c342645f36d85f6e0a2192b27c112c1f3d940f/output/run_test/examples/suit_update/nrf52dk:gnu.txt I agree, however

That test fails often on nightlies and on regular tests. So to be honest, everytime I see it fail I don't look into it in detail. But I have a esp32-wroom-32 remotely, I'll test

@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Mar 17, 2020

That test fails often on nightlies and on regular tests. So to be honest, everytime I see it fail I don't look into it in detail. But I have a esp32-wroom-32 remotely, I'll test

Hmm seems its un-plugged or has some kind of issue, I can't do anything about it for now... @aabadie do you have an espr32-wroom-32?

@miri64 miri64 removed CI: disable test cache If set, CI will always run all tests regardless of whether they have been run successfully before CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Mar 17, 2020
@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 17, 2020
@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 17, 2020

While we wait for confirmation, let's run this on Murdock one last time. Please squash

@fjmolinas fjmolinas force-pushed the pr_light_shell_interactive_sync branch from f8c3426 to 80c2d0c Compare March 17, 2020 13:06
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Mar 17, 2020

While we wait for confirmation, let's run this on Murdock one last time. Please squash

Confirmation came from murdock :)

aabadie
aabadie reviewed Mar 17, 2020
View reviewed changes
aabadie
aabadie reviewed Mar 17, 2020
View reviewed changes
aabadie
aabadie reviewed Mar 17, 2020
View reviewed changes
aabadie
aabadie reviewed Mar 17, 2020
View reviewed changes
aabadie
aabadie reviewed Mar 17, 2020
View reviewed changes
@fjmolinas fjmolinas force-pushed the pr_light_shell_interactive_sync branch from 80c2d0c to 13febed Compare March 17, 2020 16:21
@fjmolinas fjmolinas force-pushed the pr_light_shell_interactive_sync branch from 13febed to e8be1f9 Compare March 17, 2020 16:23
aabadie
aabadie approved these changes Mar 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

All comments are addressed (even my nitpicks) and we had confirmation from Murdock that this PR is ok.

ACK and go!

@aabadie aabadie merged commit 6538687 into RIOT-OS:master Mar 17, 2020
@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Mar 18, 2020
@leandrolanzieri leandrolanzieri mentioned this pull request Mar 19, 2020
Merged
@fjmolinas fjmolinas deleted the pr_light_shell_interactive_sync branch March 23, 2020 09:54
fjmolinas added a commit to fjmolinas/RIOT that referenced this pull request Mar 24, 2020
@fjmolinas
tests: remove uneeded blacklist
ec33b3e 
With RIOT-OS#12941 and RIOT-OS#13613 some of the blacklisting introduced in RIOT-OS#12461
are no longer needed, since `test_interactive_test_util` is lighter
or adds no extra code.
@fjmolinas fjmolinas mentioned this pull request Mar 24, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 left review comments

@kaspar030 kaspar030 kaspar030 left review comments

@aabadie aabadie aabadie approved these changes

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss

@smlng smlng Awaiting requested review from smlng

Assignees

@aabadie aabadie

Labels

Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2020.04

Development

Successfully merging this pull request may close these issues.

5 participants

@fjmolinas @miri64 @aabadie @kaspar030 @leandrolanzieri