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

fix(test): unflake TestEthBlockNumberAliases #12699

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Nov 18, 2024

Noticed as flaky: https://github.com/filecoin-project/lotus/actions/runs/11851664737/job/33028556808 - that one's likely to do with null round accounting (lack thereof) but there's also the stable head problem that's not accounted for with the "safe" and "finalized" forms in this test.

@rvagg rvagg requested review from masih, snissn and Kubuxu November 18, 2024 03:41
Copy link
Member

@masih masih left a comment

Choose a reason for hiding this comment

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

Looks good. I would set up a temporary CI job to run this test repeatedly just to make sure it is indeed unflaked.

@rvagg rvagg added the skip/changelog This change does not require CHANGELOG.md update label Nov 19, 2024
@rvagg
Copy link
Member Author

rvagg commented Nov 19, 2024

I've re-run the test in CI manually >5 times rather than futz around with making a special CI job for this.

@rvagg rvagg merged commit c0ec481 into master Nov 19, 2024
84 of 85 checks passed
@rvagg rvagg deleted the rvagg/unflake-eth-blkparams branch November 19, 2024 07:39
@masih
Copy link
Member

masih commented Nov 26, 2024

futz around with making a special CI job for this

In case you'd find it useful: https://gist.github.com/masih/993149988f1cd3c2cb13ee46e86d8a5c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog This change does not require CHANGELOG.md update
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging this pull request may close these issues.

3 participants