Skip to content

tests: add check_unittests helper function#13642

Merged
miri64 merged 3 commits intoRIOT-OS:masterfrom
aabadie:pr/tests/check_unittests_func
Mar 19, 2020
Merged

tests: add check_unittests helper function#13642
miri64 merged 3 commits intoRIOT-OS:masterfrom
aabadie:pr/tests/check_unittests_func

Conversation

@aabadie
Copy link
Contributor

@aabadie aabadie commented Mar 16, 2020

Contribution description

This PR introduces the check_unittests helper function and make use of it with tests using the embUnit module.
For these tests all testfunc are almost the same, so this PR removes a lot of duplication. check_unittests also takes an nb_tests parameter to specify the expected number of successful tests. If the parameter is not set, the default regexp \d+ is used.

All candidate tests are updated to make use of this new function.

Testing procedure

A green Murdock with CI: run tests label set

Issues/PRs references

None

@aabadie aabadie added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation 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 labels Mar 16, 2020
@aabadie aabadie requested a review from fjmolinas March 16, 2020 14:19
@aabadie aabadie requested a review from miri64 as a code owner March 16, 2020 14:19
@aabadie aabadie force-pushed the pr/tests/check_unittests_func branch from 2e90eaf to 9324a9e Compare March 18, 2020 09:59
@aabadie
Copy link
Contributor Author

aabadie commented Mar 18, 2020

All tests are passing except this one which is unrelated.

@aabadie aabadie added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed 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 Mar 18, 2020
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

In general, good idea, but there is room for improvement IMHO.

@aabadie aabadie force-pushed the pr/tests/check_unittests_func branch from 9324a9e to ec6af8f Compare March 18, 2020 17:12
@aabadie aabadie added 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 and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 18, 2020
@aabadie
Copy link
Contributor Author

aabadie commented Mar 18, 2020

Thanks for the review and suggestion @miri64. I adapted the PR and the tests I think you talking about. In the meantime, I slightly reworked the script of some package tests.

@aabadie aabadie force-pushed the pr/tests/check_unittests_func branch from 6af7acf to cb75145 Compare March 19, 2020 08:26
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Except for the error pointed out by Murdock (and me above) this looks good now. Please squash.

aabadie added 3 commits March 19, 2020 10:39
in pkg_c25519, pkg_libcose and pkg_tweetnacl, use the check_unittests helper function and rework the way the test TIMEOUT value is determined
@aabadie aabadie force-pushed the pr/tests/check_unittests_func branch from cb75145 to 10b7d5f Compare March 19, 2020 09:40
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

ACK. Consolidating similar functionality to a common test absolutely makes sense. If something is broken, Murdock will figure that out.

@miri64
Copy link
Member

miri64 commented Mar 19, 2020

Only thing to consider is, if the placement of the function within the module is best considering #10949.

@aabadie
Copy link
Contributor Author

aabadie commented Mar 19, 2020

if the placement of the function within the module is best considering

Looking at #10949 and there will be some adaption required but that doesn't seem like a big problem. Regarding the placement, I would say that it helps.

@miri64 miri64 merged commit 200d091 into RIOT-OS:master Mar 19, 2020
@aabadie aabadie deleted the pr/tests/check_unittests_func branch March 19, 2020 12:35
@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Mar 19, 2020
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: run tests If set, CI server will run tests on hardware for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants