tests/posix_semaphore: unify and increase allowed test4 margin#11467
Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom May 6, 2019
Merged
tests/posix_semaphore: unify and increase allowed test4 margin#11467miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64 merged 1 commit intoRIOT-OS:masterfrom
Conversation
kaspar030
added
Area: tests
Contributor
|
As mentioned IRL I am for a change like this. Would just like a justification as a comment and the |
Previously, there was a very tight allowed margin (100us), then some special cases for platforms for which the test would otherwise fail, increasing the margin. This turned out to be a maintanance burden, as each slightly special board needed a PR adding the special case. This commit sets a quite large margin (1000us, 0.1% of total delay), which should be large enough to not trip over platform-induced timer inaccuracies, but still verify that the module is using timers correctly. (This is not a timer accuracy test.)
kaspar030
force-pushed
the
increase_posix_semaphore_timeout
branch
from
3c13e1f to
a9cd943
Compare
Contributor
Author
|
Contributor
Author
|
Thanks @miri64! |
cladmi
reviewed
May 6, 2019
| /* | ||
| * Allowed margin for waiting too long. | ||
| * | ||
| * Waiting too short is forbidden by POSIX, but is checked elsewhere. |
Contributor
There was a problem hiding this comment.
It is checked here, not elsewhere:
if (elapsed < exp) {
printf("first: waited only %s usec => FAILED\n", uint64_str);
}
Contributor
There was a problem hiding this comment.
The check in the code that verifies it does not wait too short.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
The test configured tight margins for how much too long a semaphore wait can take. This is becoming a maintenance burden.
Instead of adding more special cases, this PR changes the allowed margin to 1ms (which is 0.1percent of the 1sec delay). This should be high enough to not trip over slightly inaccurate timers or slow platforms, but high enough to show fundamental problems with the interaction between timers and posix semaphores.
Remember, this should test semaphores, not timer subsystem accuracy.
Testing procedure
Validate soundness, execute tests on some boards (or let CI do it),
Issues/PRs references
#11449