-
Notifications
You must be signed in to change notification settings - Fork 310
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
cherrypick e2e changes regarding maverick from tm master #295
Conversation
This should be useful to understand the overall purpose and structure of the end-to-end tests. (cherry picked from commit 1570d26)
E2E tests often fail due to fast sync stalls causing the validator to miss signing blocks. This increases the tolerance for missed signatures to 2/3 to allow validators to spend more time starting up. (cherry picked from commit 66ba12d)
E2E tests often fail because validators miss signing or proposing blocks. Often this is because e.g. there's a lot of disruption in the network or it takes a long time to start up all the nodes. This changes the test criteria to only check for 3 signed/proposed blocks, rather than a fraction of the expected blocks. This should be enough to catch most issues, apart from performance problems causing nodes to miss signing/proposing, but we may want separate tests for those sorts of things. (cherry picked from commit 3d01d98)
(cherry picked from commit 5f88d6a)
(cherry picked from commit d4b0477)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see you soon, maverick
Are we going to bring it back ??? |
@@ -121,7 +121,7 @@ jobs: | |||
- run: | | |||
cat ./*profile.out | grep -v "mode: atomic" >> coverage.txt | |||
if: env.GIT_DIFF | |||
- uses: codecov/codecov-action@v1.4.0 | |||
- uses: codecov/codecov-action@v1.4.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this related to the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} else { | ||
err = startMaverick(cfg) | ||
} | ||
// FIXME: Temporarily remove maverick until it is redesigned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I see
Maverick: I'll be back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the tm team plan to redesign maverick or we? IMO: good redesign would require consensus redesign in a more modular way. Copying code is nah
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it redesigned it already as far as I undertand. It's now part of the other e2e tests (at least the most relevant parts).
Description
This PR only takes on the easy part of #292 (removing maverick) but I actually think we do not need to get tendermint/tendermint#6276 and related changes to close #292 and unblock work on the consensus reactor (#293).
Each commit here was cherry-picked from tendermint master (you can look at corresponding squash merged PRs in tendermint). I've included some that seemed to further help with e2e flakiness. So hopefully this also improves e2e test reliability for this repo.
The last commit (6628237) bumps the codecov action version (see https://twitter.com/thomasrockhu/status/1384619989634797576).
Closes: #292