Skip to content

murdock: introduce 'TEST_ON_CI_BLACKLIST'#11743

Merged
kaspar030 merged 3 commits intoRIOT-OS:masterfrom
cladmi:pr/ci/add_test_blacklist
Jul 12, 2019
Merged

murdock: introduce 'TEST_ON_CI_BLACKLIST'#11743
kaspar030 merged 3 commits intoRIOT-OS:masterfrom
cladmi:pr/ci/add_test_blacklist

Conversation

@cladmi
Copy link
Copy Markdown
Contributor

@cladmi cladmi commented Jun 25, 2019 •
edited
Loading

Contribution description

Introduce a variable to set that a test is blacklisted.

This is a move toward enabling tests by default and adding a blacklisting
reason instead for a board instead of not whitelisting them which hides
the problem.

Currently, a test should be both whitelisted and blacklisted at the same
time to have a meaning. It is planned to whitelist all by default in
an upcoming pull request.

Testing procedure

TEST_ON_CI_BLACKLIST behaves properly

The tests/xtimer_now64_continuity and tests/xtimer_usleep must correctly run by murdock on samr21-xpro and nrf52dk, and not native.

https://ci.riot-os.org/RIOT-OS/RIOT/11743/bdd721fff476ae8ffe11b6240e7d0e0a6070999f/output/run_test/tests/xtimer_now64_continuity/nrf52dk:gnu.txt

And it is indeed not executed in native (it is in the compiler output for native).

https://ci.riot-os.org/RIOT-OS/RIOT/11743/bdd721fff476ae8ffe11b6240e7d0e0a6070999f/output/compile/tests/xtimer_now64_continuity/native:gnu.txt

It creates the .test file and test-on-ci-enabled has expected return value

export BOARD=native; RIOT_CI_BUILD=1 make -C tests/xtimer_usleep 2>/dev/null >/dev/null ; make --no-print-directory test-on-ci-enabled -C
 $_; ls $_/bin/${BOARD}/.test
/home/harter/work/git/RIOT/makefiles/murdock.inc.mk:46: recipe for target 'test-on-ci-enabled' failed
make: *** [test-on-ci-enabled] Error 1
ls: cannot access 'tests/xtimer_usleep/bin/native/.test': No such file or directory
export BOARD=samr21-xpro; RIOT_CI_BUILD=1 make -C tests/xtimer_usleep 2>/dev/null >/dev/null ; make --no-print-directory test-on-ci-enabl
ed -C $_; ls $_/bin/${BOARD}/.test
tests/xtimer_usleep/bin/samr21-xpro/.test
export BOARD=nrf52dk; RIOT_CI_BUILD=1 make -C tests/xtimer_usleep 2>/dev/null >/dev/null ; make --no-print-directory test-on-ci-enabled -
C $_; ls $_/bin/${BOARD}/.test
tests/xtimer_usleep/bin/nrf52dk/.test

Previous behavior is preserved

Tests with TEST_ON_CI_WHITELIST += all still create the .test file and test-on-ci-enabled has expected return value

export BOARD=native; RIOT_CI_BUILD=1 make -C tests/bloom_bytes/ 2>/dev/null >/dev/null ; make --no-print-directory test-on-ci-enabled -C $_; ls $_/bin/${BOARD}/.test
tests/bloom_bytes//bin/native/.test
export BOARD=samr21-xpro; RIOT_CI_BUILD=1 make -C tests/bloom_bytes/ 2>/dev/null >/dev/null ; make --no-print-directory test-on-ci-enabled -C $_; ls $_/bin/${BOARD}/.test
tests/bloom_bytes//bin/samr21-xpro/.test

With TEST_ON_CI_WHITELIST = native only native does it

export BOARD=native; RIOT_CI_BUILD=1 make -C tests/libfixmath_unittests/ 2>/dev/null >/dev/null ; make --no-print-directory test-on-ci-enabled -C $_; ls $_/bin/${BOARD}/.test
tests/libfixmath_unittests//bin/native/.test
export BOARD=samr21-xpro; RIOT_CI_BUILD=1 make -C tests/libfixmath_unittests/ 2>/dev/null >/dev/null ; make --no-print-directory test-on-ci-enabled -C $_; ls $_/bin/${BOARD}/.test
/home/harter/work/git/RIOT/makefiles/murdock.inc.mk:46: recipe for target 'test-on-ci-enabled' failed
make: *** [test-on-ci-enabled] Error 1
ls: cannot access 'tests/libfixmath_unittests//bin/samr21-xpro/.test': No such file or directory

If empty, tests are disabled

export BOARD=native; RIOT_CI_BUILD=1 make -C examples/hello-world/ 2>/dev/null >/dev/null ; make --no-print-directory test-on-ci-enabled -C $_; ls $_/bin/${BOARD}/.test
/home/harter/work/git/RIOT/makefiles/murdock.inc.mk:46: recipe for target 'test-on-ci-enabled' failed
make: *** [test-on-ci-enabled] Error 1
ls: cannot access 'examples/hello-world//bin/native/.test': No such file or directory

Issues/PRs references

cladmi added 2 commits June 25, 2019 18:56
@cladmi
murdock: refactor handling of TEST_ON_CI_WHITELIST
5d2c57f 
Refactor the handling to use a variable to store if a test is enabled.
Add a 'test-on-ci-enabled' target that test if the test on ci is enabled.

This is a first commit before changing the behavior.
@cladmi
murdock: introduce 'TEST_ON_CI_BLACKLIST'
45a3182 
Introduce a variable to set that a test is blacklisted.

This is a move toward enabling tests by default and adding a blacklisting
reason instead for a board instead of not whitelisting them which hides
the problem.

Currently, a test should be both whitelisted and blacklisted at the same
time to have a meaning. It is planned to whitelist all by default in
an upcoming pull request.
@cladmi cladmi added Area: tests Area: tests and testing framework Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Jun 25, 2019
@cladmi cladmi requested a review from MrKevinWeiss June 25, 2019 17:00
@cladmi cladmi added CI: disable test cache If set, CI will always run all tests regardless of whether they have been run successfully before 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 Jun 25, 2019
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Jun 25, 2019

I disable test cache as otherwise, murdock does not include the tests results.
I want to check that indeed tests are run.

@cladmi
tests/xtimer: blacklist native instead of whitelisting
bdd721f 
Blacklist native for the xtimer tests that have timing issues.

This also enables running the test on `nrf52dk` as I think it was
forgotten when adding the support.
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Jun 26, 2019

I added a usage for TEST_ON_CI_BLACKLIST and a testing procedure.
This should also allow running the changed tests on nrf52dk.

@cladmi cladmi added this to the Release 2019.07 milestone Jun 26, 2019
@cladmi cladmi requested a review from jcarrano June 26, 2019 16:21
@kaspar030 kaspar030 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 Jul 12, 2019
@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Jul 12, 2019

This is the diff between tests run for last nightlies and this PR:

[kaspar@ng ~/tmp/foo]$ diff pr master
88a89,92
> tests/gnrc_ipv6_fwd_w_sub/nrf52dk:gnu
> tests/gnrc_ipv6_fwd_w_sub/nrf52dk:llvm
> tests/gnrc_ipv6_fwd_w_sub/samr21-xpro:gnu
> tests/gnrc_ipv6_fwd_w_sub/samr21-xpro:llvm
245a250,253
> tests/pkg_nanocbor/nrf52dk:gnu
> tests/pkg_nanocbor/nrf52dk:llvm
> tests/pkg_nanocbor/samr21-xpro:gnu
> tests/pkg_nanocbor/samr21-xpro:llvm
403,404d410
< tests/xtimer_now64_continuity/nrf52dk:gnu
< tests/xtimer_now64_continuity/nrf52dk:llvm
419,420d424
< tests/xtimer_usleep/nrf52dk:gnu
< tests/xtimer_usleep/nrf52dk:llvm

There's something wrong with the base branch of the CI build. It was merged into "eab0a8864250001567cb0cbfc1c5562ba09a3af2", which is quite old. I'm investigating.

@kaspar030 kaspar030 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 Jul 12, 2019
@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Jul 12, 2019

I think I fell over a browser-cached, previous result. Correct diff is:

[kaspar@ng ~/tmp/foo]$ diff master pr
410a411,412
> tests/xtimer_now64_continuity/nrf52dk:gnu
> tests/xtimer_now64_continuity/nrf52dk:llvm
424a427,428
> tests/xtimer_usleep/nrf52dk:gnu
> tests/xtimer_usleep/nrf52dk:llvm

kaspar030
kaspar030 approved these changes Jul 12, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK.

@kaspar030 kaspar030 merged commit 6c895e1 into RIOT-OS:master Jul 12, 2019
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Jul 12, 2019

Thank you for the review.

@cladmi cladmi deleted the pr/ci/add_test_blacklist branch July 12, 2019 16:10
This was referenced Jul 12, 2019
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kaspar030 kaspar030 kaspar030 approved these changes

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss

@jcarrano jcarrano Awaiting requested review from jcarrano

Assignees

No one assigned

Labels

Area: tests Area: tests and testing framework CI: disable test cache If set, CI will always run all tests regardless of whether they have been run successfully before 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: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Milestone

Release 2019.10

Development

Successfully merging this pull request may close these issues.

3 participants

@cladmi @kaspar030 @MrKevinWeiss