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

Make TestTimerFixture.test_timer_not_ready not fail sporadically #378

Closed
durko opened this issue Jan 28, 2019 · 2 comments
Closed

Make TestTimerFixture.test_timer_not_ready not fail sporadically #378

durko opened this issue Jan 28, 2019 · 2 comments
Labels
backlog enhancement New feature or request help wanted Extra attention is needed Linux Linux support tests

Comments

@durko
Copy link

durko commented Jan 28, 2019

Bug report

Required Info:

  • Operating System:
    • Ubuntu 16.04, 18.04
  • Installation type:
    • source
  • Version or commit hash:
  • DDS implementation:
    • any
  • Client library (if applicable):
    • rcl

Steps to reproduce issue

  • Run test suite
  • Test TestTimerFixture.test_timer_not_ready fails occasionaly

Expected behavior

Tests always pass

Actual behavior

Tests randomly fail

Additional information

This is related to #188, however we are proposing a solution to one sporadic failure here.

The problem we are seeing with this test is, that the 5ms timer

&timer, &clock, this->context_ptr, RCL_MS_TO_NS(5), nullptr, rcl_get_default_allocator());
becomes ready during 1ms wait on the waitset. On our CI machine the 5ms timer sometimes run out before
ret = rcl_wait(&wait_set, RCL_MS_TO_NS(1));
is reached. Basically our runtime under moderate load is a few ms for the couple of lines in between.

Is there a specific reason for the rather tight value of 5ms on the timer, or would it be possible to significantly relax the timer constraint to lets say 1s?

@jacobperron
Copy link
Member

I don't see a reason why the timer value should be so low. Considering the nature of the test, bumping the timer value to 1 second sounds reasonable to me. I'd like to get a second opinion, but feel free to open a PR with the proposed change.

@jacobperron jacobperron added the enhancement New feature or request label Jan 30, 2019
@cottsay cottsay added help wanted Extra attention is needed backlog labels Feb 7, 2019
@chapulina chapulina added tests Linux Linux support labels Nov 4, 2020
@clalancette
Copy link
Contributor

It looks like these values were increased a while ago, and we haven't seen this flaky test in a while on the buildfarm. Closing this out, but feel free to reopen if this is still an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog enhancement New feature or request help wanted Extra attention is needed Linux Linux support tests
Projects
None yet
Development

No branches or pull requests

5 participants