Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Improves lunar-devel jenkins test stability#1135

Merged
dirk-thomas merged 4 commits intoros:lunar-develfrom
kmactavish:lunar-devel-jenkins-baseline
Aug 14, 2017
Merged

Improves lunar-devel jenkins test stability#1135
dirk-thomas merged 4 commits intoros:lunar-develfrom
kmactavish:lunar-devel-jenkins-baseline

Conversation

@kmactavish
Copy link
Copy Markdown
Contributor

@kmactavish kmactavish commented Aug 12, 2017

This PR is to establish a Jenkins test-result baseline for lunar-devel. #1133 is failing stochastically, and this PR is being used to establish a baseline of which stochastic failures are expected on devel (if any).
Update: Also fixes one flaky test:

  • extend random_play.py timeout

@kmactavish
Copy link
Copy Markdown
Contributor Author

kmactavish commented Aug 12, 2017

Triggered jobs 324-331. Update: only one failed, new commit to extend that timeout.

@kmactavish kmactavish force-pushed the lunar-devel-jenkins-baseline branch from f581da6 to 50785ed Compare August 12, 2017 15:24
@kmactavish
Copy link
Copy Markdown
Contributor Author

kmactavish commented Aug 12, 2017

Triggered jobs 332-339. Update: 338 failed, but I can't find any "RESULT: FAIL" in the output...

@kmactavish kmactavish changed the title Baseline lunar-devel jenkins job Improves lunar-devel jenkins test stability Aug 12, 2017
@mikaelarguedas
Copy link
Copy Markdown
Member

@kmactavish If it's of any help: It looks like the failing test is a nosetest added( using catkin_add_nosetests) and not a rostest.
Nosetests output results as "OK" or FAILED (failures=1) or FAILED (errors=1). The "SUMMARY" is part of rostest I believe, and thus not printed by "pure" nose of gtest tests.

@kmactavish
Copy link
Copy Markdown
Contributor Author

Build 345 failed (not this branch). It was in raceConditionCallback, which is flawed.
Explanation (main is the main thread, add is the spawned thread):

  • main gets to line 471 (queue is cleared)
  • add sees sync is true, does its thing (setting sync to false, adding the callback to the queue)
  • main sets sync back to true! and calls the callback (setting one to false)
  • add sees sync is true, does its thing (setting one to true)
  • main wakes up, and having called the callback, checks to make sure one is false. It's not!
    So, ironically, there was a secondary, unintentional race condition in the race condition check 😱.

Fixed by only setting the sync flag in one thread. It means adding another check to see if the queue is empty in the second thread.

@kmactavish
Copy link
Copy Markdown
Contributor Author

@mikaelarguedas Thanks! That helped.

@kmactavish
Copy link
Copy Markdown
Contributor Author

Build 355 (not this branch) also failed due to the compression ratio test thresholds, but in the other direction, relaxed them more.

@kmactavish
Copy link
Copy Markdown
Contributor Author

@esteve, I had to change your raceConditionCallback test (see above). You might want to have a look and make sure it still tests for the right things after my modifications.

@dirk-thomas
Copy link
Copy Markdown
Member

Thank you for improving the stability of the tests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants