Skip to content

tests/pthread_barrier: fix test script#17196

Merged
fjmolinas merged 1 commit intoRIOT-OS:masterfrom
maribu:tests/pthread_barrier
Nov 16, 2021
Merged

tests/pthread_barrier: fix test script#17196
fjmolinas merged 1 commit intoRIOT-OS:masterfrom
maribu:tests/pthread_barrier

Conversation

@maribu
Copy link
Member

@maribu maribu commented Nov 15, 2021

Contribution description

Previously the test script relied on the exact sequence of numbers
returned by the used PRNG. This resulting e.g. in

$ USEMODULE=prng_musl_lcg make -C tests/pthread_barrier flash test

to fail, only because the order in which the children completed is
slightly different due to different sleep durations. This fixes the
issue.

Testing procedure

The command above should succeed, no with and without USEMODULE=prng_musl_lcg before the invocation of make.

Issues/PRs references

Useful for #17188

Previously the test script relied on the exact sequence of numbers
returned by the used PRNG. This resulting e.g. in

```
$ USEMODULE=prng_musl_lcg make -C tests/pthread_barrier flash test
```

to fail, only because the order in which the children completed is
slightly different due to different sleep durations. This fixes the
issue.
@maribu maribu requested a review from miri64 as a code owner November 15, 2021 08:12
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Nov 15, 2021
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Nov 15, 2021
@maribu maribu requested review from aabadie and benpicco November 15, 2021 08:13
@maribu
Copy link
Member Author

maribu commented Nov 15, 2021

I disabled compile tests (as this doesn't change C code). I think this will also skip builds and test runs for native, so reviewers should make sure that make test not only works on my machine ;-)

@maribu
Copy link
Member Author

maribu commented Nov 15, 2021

The race condition hit and Murdock is already almost through a full build. So at least on native the test is run by the CI.

@fjmolinas fjmolinas merged commit 6dd0521 into RIOT-OS:master Nov 16, 2021
@maribu
Copy link
Member Author

maribu commented Nov 18, 2021

Thanks for the review

@maribu maribu deleted the tests/pthread_barrier branch November 18, 2021 08:19
@fjmolinas fjmolinas added this to the Release 2022.01 milestone Nov 18, 2021
@chrysn
Copy link
Member

chrysn commented Nov 23, 2021

I think the use of fstrings here broke CI hardware tests, possibly because the pi workers still use an old version of Python 3:

-- running on worker pi-ef3229ad thread 1, build number 10729.
-- executing tests for tests/pthread_barrier on samr21-xpro (compiled with gnu toolchain):
- executing hook run_test_pre
- resetting USB power...
- waiting for tty to re-attach...
[...]
make: Entering directory '/tmp/dwq.0.7924475090058434/6e6f7168dc09d6596896571abdc7edba/tests/pthread_barrier'
  File "/tmp/dwq.0.7924475090058434/6e6f7168dc09d6596896571abdc7edba/tests/pthread_barrier/tests/01-run.py", line 12
    child.expect(f'Start {i + 1}')
                                ^
SyntaxError: invalid syntax
/tmp/dwq.0.7924475090058434/6e6f7168dc09d6596896571abdc7edba/makefiles/tests/tests.inc.mk:22: recipe for target 'test' failed
make: *** [test] Error 1
make: Leaving directory '/tmp/dwq.0.7924475090058434/6e6f7168dc09d6596896571abdc7edba/tests/pthread_barrier'

(From https://ci.riot-os.org/RIOT-OS/RIOT/17256/c360b13e6096968cb675e0c00d2e9afe803ff1d5/output/run_test/tests/pthread_barrier/samr21-xpro:gnu.txt)

@maribu
Copy link
Member Author

maribu commented Nov 23, 2021

Thanks for reporting. I'll wait a bit before PR'ing a fix to see if the Pis can be updated instead

@chrysn
Copy link
Member

chrysn commented Nov 29, 2021

For reference, fix is in #17283

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 CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants