Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unittests/sht1x: decrease the amount of tested values on board #12054

Merged

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Aug 21, 2019

Contribution description

Reduce the amount of tested values by a 100.
This makes the testing time go from 3 minutes to 2 seconds on frdm-kw41z.

Testing that the integer calculation matches the float one does not need
to be performed on the full range on boards. Checking some values should
be enough to detect overflow issues.
The full range checking is kept on native.

Fixes #12042

Testing procedure

Running the unittests on frdm-kw41z now goes to the end of the test in time:

2019-08-21 15:00:31,531 - INFO # ��˙����main(): This is RIOT! (Version: 2019.10-devel-445-g2a24-pr/unittests/sht1x/limit_test_vectors_on_board)
2019-08-21 15:00:50,676 - INFO # .............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
2019-08-21 15:00:50,676 - INFO # OK (957 tests)
2019-08-21 15:00:50,676 - INFO # *** RIOT kernel panic:

I noticed the RIOT kernel panic, but this is also in master

On master the test was failing:

BOARD=frdm-kw41z make tests-sht1x flash test

Type '/exit' to exit.                                                                                                                                                                      
2019-08-20 17:21:58,190 - INFO # t�G�   main(): This is RIOT! (Version: 2019.10-devel-444-gd6356)                                                                                     
Timeout in expect script at "child.expect(u"OK \\([0-9]+ tests\\)")" (tests/unittests/tests/01-run.py:14)

But when run with term we could get the output after 3:30 minutes (with the HARD FAULT.

2019-08-21 15:03:34,907 - INFO # main(): This is RIOT! (Version: 2019.10-devel-444-gd6356)


2019-08-21 15:06:56,170 - INFO # .............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
2019-08-21 15:06:56,175 - INFO # OK (957 tests)
2019-08-21 15:06:56,176 - INFO # *** RIOT kernel panic:
2019-08-21 15:06:56,176 - INFO # HARD FAULT HANDLER
2019-08-21 15:06:56,176 - INFO #
2019-08-21 15:06:56,185 - INFO # *** rebooting...main(): This is RIOT! (Version: 2019.10-devel-444-gd6356)

Issues/PRs references

The test was failing during release too RIOT-OS/Release-Specs#128 (comment)

Reduce the amount of tested values by a 100.
This makes the testing time go from 3 minutes to 2 seconds on
`frdm-kw41z`.

Testing that the integer calculation matches the float one does not need
to be performed on the full range on boards. Checking some values should
be enough to detect overflow issues.
The full range checking is kept on native.
@maribu maribu added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Aug 21, 2019
@maribu
Copy link
Member

maribu commented Aug 21, 2019

I can verify that the unit tests now takes less than a second on the MSB-A2, instead of almost a minute.

@maribu maribu added Reviewed: 3-testing The PR was tested according to the maintainer guidelines CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 21, 2019
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

ACK. Thanks for the fix!

@cladmi
Copy link
Contributor Author

cladmi commented Aug 21, 2019

ACK. Thanks for the fix!

It was easy when you explained what was done :)

@cladmi
Copy link
Contributor Author

cladmi commented Aug 21, 2019

You should put "CI: run tests" and toggle "ci: ready for build" otherwise it will not execute the updated test.

@maribu maribu 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 Aug 21, 2019
@maribu maribu merged commit 938b36a into RIOT-OS:master Aug 21, 2019
@cladmi
Copy link
Contributor Author

cladmi commented Aug 21, 2019

Thank you for the review! :)

@maribu
Copy link
Member

maribu commented Aug 21, 2019

You're very welcome. Thanks for identifying and addressing the issue :-)

@cladmi cladmi deleted the pr/unittests/sht1x/limit_test_vectors_on_board branch August 21, 2019 14:38
@kb2ma kb2ma added this to the Release 2019.10 milestone Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests/unittests/tests-sht1x breaks on `frdm-kw41z
3 participants