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

Reduce CI flakiness by using cargo-nextest #3544

Merged
merged 8 commits into from
Aug 18, 2023
Merged

Conversation

soareschen
Copy link
Contributor

Description

The CI is getting very flaky. I'm trying to use cargo nextest with retries to retry individual tests, rather than retrying the whole integration test when one fails.


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@soareschen soareschen requested review from romac and ljoss17 August 17, 2023 08:17
@soareschen soareschen marked this pull request as ready for review August 17, 2023 10:36
Copy link
Contributor

@ljoss17 ljoss17 left a comment

Choose a reason for hiding this comment

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

Looks great!

@romac
Copy link
Member

romac commented Aug 17, 2023

Great idea!

@romac
Copy link
Member

romac commented Aug 17, 2023

Let's use this as well for all tests, including the multi chain tests

@romac romac added the CI: multi-chains CI: Run the `multi-chains` integration test job on this PR label Aug 17, 2023
@romac
Copy link
Member

romac commented Aug 17, 2023

Not sure why the multi-chains test got slower, and therefore killed after 60min, whereas they used to run in ~40min. I increased the timeout to see how long they take now.

@romac
Copy link
Member

romac commented Aug 17, 2023

Oh but everything is slower now that we don't pass --test-threads=2 anymore.

@soareschen
Copy link
Contributor Author

Yeah it looks like cargo nextest do not allow multiple threads when --nocapture is on. Makes sense considering that the output from multiple tests would interleave with each other. Or perhaps we just don't capture the output and rely on local tests for any log message?

@soareschen
Copy link
Contributor Author

Just figured there is a --failure-output option which we can use to only output logs for a test only when it failed. This should significantly reduce the CI logs and also allow us to parallelize the tests again.

@romac
Copy link
Member

romac commented Aug 18, 2023

Wow, great find, that's amazing!

@romac
Copy link
Member

romac commented Aug 18, 2023

@soareschen It looks your commits aren't signed anymore. I can still merge the branch, but that's maybe something to look into.

@romac romac changed the title Try to reduce CI flakiness Reduce CI flakiness by using cargo-nextest Aug 18, 2023
@soareschen
Copy link
Contributor Author

I think I just didn't configure about git commit signing. Will try signing with GPG in future commits.

@romac
Copy link
Member

romac commented Aug 18, 2023

You can also sign commits with your SSH key if it's easier to setup

@romac romac merged commit 92e0ac0 into master Aug 18, 2023
@romac romac deleted the soares/reduce-ci-flakiness branch August 18, 2023 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: multi-chains CI: Run the `multi-chains` integration test job on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants