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.

Sign up for GitHub

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

Jump to bottom

Remove problematic discovery test #336

Merged
merged 2 commits into from
Jul 26, 2022
Merged

Remove problematic discovery test #336

merged 2 commits into from
Jul 26, 2022

Conversation

mjcarroll
Copy link
Contributor

This test has recently appeared to fail, we have some on-going diagnosis in #335. In the meantime, I think it makes sense to disable to get CI back into a reasonable state.

@mjcarroll
Remove problematic discovery test
73beb9e 
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll marked this pull request as ready for review July 26, 2022 16:19
@mjcarroll mjcarroll requested a review from caguero as a code owner July 26, 2022 16:19
@mjcarroll mjcarroll requested a review from chapulina July 26, 2022 16:19
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Jul 26, 2022
@chapulina
Copy link
Contributor

Can we just disable the test on the platforms where it fails, instead of removing it?

@codecov
Copy link

codecov bot commented Jul 26, 2022 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (main@012a19d). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #336   +/-   ##
=======================================
  Coverage        ?   88.11%           
=======================================
  Files           ?       52           
  Lines           ?     4848           
  Branches        ?        0           
=======================================
  Hits            ?     4272           
  Misses          ?      576           
  Partials        ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 012a19d...44501c6. Read the comment docs.

@mjcarroll
Copy link
Contributor Author

Can we just disable the test on the platforms where it fails, instead of removing it?

We can, as it turns out, we don't have a macro to disable a test only on Linux, I suppose it's never been needed before?

@mjcarroll
Add disable on linux test macro for WrongGzIp
44501c6 
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll mentioned this pull request Jul 26, 2022
Merged
8 tasks
@mjcarroll mjcarroll merged commit 80f71eb into main Jul 26, 2022
@mjcarroll mjcarroll deleted the mjcarroll/discovery_test_disable branch July 26, 2022 17:46
@mjcarroll
Copy link
Contributor Author

New warnings on windows are to be addressed elsewhere.

@chapulina
Copy link
Contributor

Would you be able to backport this to Citadel and merge forward to Fortress? Thanks!

@mjcarroll mjcarroll mentioned this pull request Jul 27, 2022
Merged
@mjcarroll
Copy link
Contributor Author

Would you be able to backport this to Citadel and merge forward to Fortress?

Done: #339

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

@chapulina chapulina chapulina approved these changes

@caguero caguero Awaiting requested review from caguero caguero is a code owner

Assignees
No one assigned
Labels
🌱 garden Ignition Garden
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mjcarroll @chapulina