-
Notifications
You must be signed in to change notification settings - Fork 508
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
Disable actually failing flaky OMPL test #495
Disable actually failing flaky OMPL test #495
Conversation
Disables #481 |
Codecov Report
@@ Coverage Diff @@
## main #495 +/- ##
==========================================
- Coverage 46.01% 45.19% -0.82%
==========================================
Files 170 169 -1
Lines 18731 18729 -2
==========================================
- Hits 8618 8462 -156
- Misses 10113 10267 +154
Continue to review full report at Codecov.
|
I think this is a pretty bad habit to get into... the chances of you ever re-enabling this flakey test is close to zero. Did a Clubhouse story get created to tackle this issue in the next sprint? Do we have a plan to fix it? MoveIt already has low test coverage, but disabling existing ones only makes that worse. Tagging @mamoll as this pertains to OMPL. |
Thanks for the heads up @davetcoleman. I have a clubhouse story and an issue here to re-enable this. The reason I suggested to disable this was we didn't know the cause and this flakiness was becoming an issue with a lot of PRs failing in CI in the release sprint. #511 has more information from my debugging session. I think @mamoll maybe the right person to look into this. For the OMPL and servo tests that we disabled I am trying to look whenever I can and really want to enable them back asap. |
just a side remark: if you have to disable tests, you should usually not disable compiling the whole unit. |
Thanks a lot @v4hn. Definitely learned something new today! I will make sure I do it this way if we need to disable any test in the future. I might take my time to change this as well if I can't figure out a fix soon. |
Disable actually failing flaky OMPL test that causes CI failures all over moveit2. See https://github.com/ros-planning/moveit2/runs/2798018498?check_suite_focus=true for a sample output.