-
Notifications
You must be signed in to change notification settings - Fork 490
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
flake: TestLedgerContinuesOnVotersCallbackFailure #5454
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5454 +/- ##
==========================================
- Coverage 55.60% 55.59% -0.02%
==========================================
Files 447 447
Lines 63395 63395
==========================================
- Hits 35253 35245 -8
- Misses 25760 25766 +6
- Partials 2382 2384 +2 see 8 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
This is incorrect. The test asserts commit happened. Please skip it instead.
After digging into with Chris, the test is useless now after making |
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.
Thank you Zeph for the change!
ledger/ledger_test.go
Outdated
@@ -3091,32 +3091,6 @@ type errorCommitListener struct{} | |||
func (l *errorCommitListener) OnPrepareVoterCommit(oldBase basics.Round, newBase basics.Round, _ ledgercore.LedgerForSPBuilder) { |
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.
We also need to remove the errorCommitListener type
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.
For more background, in #5085 the OnPrepareVoterCommit method was changed to not return an error, so as part of that change the errorCommitListener was updated (it used to return an error), but no longer does anything differently from the mockCommitListener in the test above. |
Summary
Remove a flakey test deemed no longer needed along with a type that its removal orphaned.
Test Plan
CI