Skip to content

tests/thread_float: fix test script#18255

Merged
chrysn merged 2 commits intoRIOT-OS:masterfrom
maribu:tests/thread_float
Jun 30, 2022
Merged

tests/thread_float: fix test script#18255
chrysn merged 2 commits intoRIOT-OS:masterfrom
maribu:tests/thread_float

Conversation

@maribu
Copy link
Copy Markdown
Member

@maribu maribu commented Jun 23, 2022

Contribution description

Previously the test script expected runnable threads of the same
priority to be running in a specific order. But the only tool that is
guaranteed to enforce a specific order of runnable threads is assigning
them different priority levels.

This should fix a test failure in the nightlies.

Testing procedure

Murdock will do so

Issues/PRs references

None

@maribu maribu requested a review from miri64 as a code owner June 23, 2022 15:09
@maribu maribu requested a review from MrKevinWeiss June 23, 2022 15:09
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Jun 23, 2022
@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: run tests If set, CI server will run tests on hardware for the labeled PR CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 23, 2022
miri64
miri64 reviewed Jun 23, 2022
View reviewed changes
miri64
miri64 reviewed Jun 23, 2022
View reviewed changes
@maribu maribu force-pushed the tests/thread_float branch from aff1289 to 1fcf076 Compare June 24, 2022 09:16
miri64
miri64 previously approved these changes Jun 24, 2022
View reviewed changes
@miri64 miri64 enabled auto-merge June 24, 2022 09:17
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Jun 24, 2022

Oha, this doesn't look good:

riotbuild@189ae4e4c3a5:/foo/tests/thread_float$ make test
r
/foo/tests/thread_float/bin/native/tests_thread_float.elf /dev/ttyACM0 
RIOT native interrupts/signals initialized.
RIOT native board initialized.
RIOT native hardware initialization complete.

Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: 2022.07-devel-863-g1fcf07-tests/thread_float)
THREADS CREATED

{ "threads": [{ "name": "idle", "stack_size": 8192, "stack_used": 436 }]}
{ "threads": [{ "name": "main", "stack_size": 12288, "stack_used": 2468 }]}
THREAD t1 start
THREAD t2 start
THREAD t3 start
t3: -nan
t3: 141.436065
t1: -nan
t3: -nan
t3: -nan
t1: -nan
t3: -nan
t1: -nan
t1: 141.412857
t3: -nan
t3: 141.436065
t3: 141.436065
t1: -nan
t1: 141.412857
t1: -nan
t1: -nan
t1: 141.412857
t1: -nan
t3: -nan
t3: -nan
t3: 141.436065
t1: -nan
t1: -nan
t1: 141.412857
t3: -nan
t3: 141.436065
t3: 141.436065
t3: 141.436065
t1: -nan
t1: 141.412857
t1: -nan
t3: -nan
t3: 141.436065
t1: -nan
t3: -nan
t3: 141.436065
t1: -nan
t1: 141.412857

riotbuild@189ae4e4c3a5:/foo/tests/thread_float$ echo $?
0

(That the FPU state is not properly saved and restored on context switch on native is a known issue. But the test script passing anyway is not ideal... It is present on master as well. So I guess I open a new PR to fix this.)

@maribu
Copy link
Copy Markdown
Member Author

maribu commented Jun 24, 2022

OK, the fix is rather trivial, I just add a new commit here and disable auto-merge.

@maribu maribu disabled auto-merge June 24, 2022 09:35
@maribu maribu dismissed miri64’s stale review June 24, 2022 09:39

I'm about to add a new commit to fix another issue, which will render the review outdated

chrysn
chrysn approved these changes Jun 30, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@chrysn chrysn left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks. One of the nightly regulars down, two to go.

(Please merge right away, or after applying the manual suggestion -- I want tests to pass badly enough not to insist on any change :-) )

@chrysn chrysn mentioned this pull request Jun 30, 2022
Merged
2 tasks
maribu and others added 2 commits June 30, 2022 19:15
@maribu @miri64
tests/thread_float: fix test script
0beb636 
Previously the test script expected runnable threads of the same
priority to be running in a specific order. But the only tool that is
guaranteed to enforce a specific order of runnable threads is assigning
them different priority levels.

This should fix a test failure in the nightlies.

Co-authored-by: Martine Lenders <[email protected]>
@maribu
tests/thread_float: clean up and fix test script
94ccc6c 
Split out the regex that matches the output line into a dedicated
function (as it is used three times) and make it also accept nan and
inf as double values. Previously a nan didn't match and occasional
nans were not detected as a test failure.
@maribu maribu force-pushed the tests/thread_float branch from 1a5012a to 94ccc6c Compare June 30, 2022 17:15
@chrysn chrysn enabled auto-merge June 30, 2022 17:18
@chrysn chrysn merged commit cb5d4f8 into RIOT-OS:master Jun 30, 2022
@maribu maribu deleted the tests/thread_float branch July 22, 2022 16:16
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chrysn chrysn chrysn approved these changes

@miri64 miri64 miri64 left review comments

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss

Assignees

No one assigned

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 CI: run tests If set, CI server will run tests on hardware for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2022.07

Development

Successfully merging this pull request may close these issues.

3 participants

@maribu @chrysn @miri64