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

Improve workflow tests robustness #419

Merged
merged 1 commit into from
Jun 16, 2022

Conversation

Blast545
Copy link
Contributor

Attempts to fix #58, WIP

Summary

Before trying to change the underlying virtual buffer used to run GUI tests, this PR changes the arguments passed to the after_make.sh script to make it more robust.

This is already done in gz-rendering, see: gazebosim/gz-rendering#255

Running CI a couple of times before asking for reviews.

Checklist

  • Signed all commits for DCO
  • Added tests -> potentially fixes tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

@Blast545 Blast545 self-assigned this Jun 16, 2022
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Jun 16, 2022
@codecov
Copy link

codecov bot commented Jun 16, 2022

Codecov Report

Merging #419 (4a521b8) into ign-gui3 (c7bfb8a) will not change coverage.
The diff coverage is n/a.

❗ Current head 4a521b8 differs from pull request most recent head 672275e. Consider uploading reports for the commit 672275e to get more accurate results

@@            Coverage Diff            @@
##           ign-gui3     #419   +/-   ##
=========================================
  Coverage     65.45%   65.45%           
=========================================
  Files            29       29           
  Lines          3216     3216           
=========================================
  Hits           2105     2105           
  Misses         1111     1111           

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 c7bfb8a...672275e. Read the comment docs.

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh for some reason I thought we had already applied that PR to GUI! Well spotted! CI looks unusually happy, this is a great sign! 🙏🏽

@Blast545 Blast545 changed the title Improve workflow tests (WIP) Improve workflow tests robustness Jun 16, 2022
@Blast545
Copy link
Contributor Author

I took some time to "stress test" this PR before merging to have some confidence that it improves our current state with Github Actions.

The whole history of attempts can be seen here: https://github.com/gazebosim/gz-gui/actions/runs/2506612183
Out of 5 attempts, it failed once on Bionic CI and twice on Focal CI exactly in the same place, I've opened an issue to track this error: #421

Failure on Windows is being tracked with #420, so I'll merge this PR. 🚀

@Blast545 Blast545 merged commit 6c83500 into ign-gui3 Jun 16, 2022
@Blast545 Blast545 deleted the blast545/improve_workflow_tests branch June 16, 2022 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants