Improve testing around TestCandidateSelfVoteAfterWonElection#127
Conversation
This commit improves the testing around cases where a (pre-)candidate wins an election without having voted for themselves. This was already tested by TestCandidateDeliversPreCandidateSelfVoteAfterBecomingCandidate, but this commit expands the testing to handle three related cases and to mirror the existing TestCandidateSelfVoteAfterLostElection test. Signed-off-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
|
@pav-kv when you get a chance, want to give this testing PR a pass? |
pav-kv
left a comment
pav-kv
left a comment
There was a problem hiding this comment.
LGTM % a couple of nice-to-haves
| expState = StatePreCandidate | ||
| } | ||
| if sm.state != expState { | ||
| t.Errorf("state = %v, want %v", sm.state, expState) |
There was a problem hiding this comment.
Convert these to use require package while here?
| // Its self-vote does not make its way to its ProgressTracker. | ||
| granted, _, _ := sm.trk.TallyVotes() |
There was a problem hiding this comment.
Why? Is this because the leader doesn't count votes in general (it doesn't need to already)? Or there is just special handling of leader self-vote?
Might be good to clarify the comment.
| if sm.state != expState { | ||
| t.Errorf("state = %v, want %v", sm.state, expState) | ||
| } |
There was a problem hiding this comment.
This minor comment also applies to all other places.
| if sm.state != expState { | |
| t.Errorf("state = %v, want %v", sm.state, expState) | |
| } | |
| require.Equal(t, expState, sm.state) |
| expState = StatePreCandidate | ||
| } | ||
| if sm.state != expState { | ||
| t.Errorf("state = %v, want %v", sm.state, expState) |
| // Nothing new should be sent since n1 first learned that it was a | ||
| // candidate. | ||
| if steps3 := sm.takeMessagesAfterAppend(); len(steps3) != 0 { | ||
| t.Errorf("unexpected new messages after pre-vote self-vote: %+v", steps3) | ||
| } |
There was a problem hiding this comment.
The r.msgsAfterAppend has already been reset to nil when getting the steps2 (line 1888)
| // Nothing new should be sent since n1 first learned that it was a | |
| // candidate. | |
| if steps3 := sm.takeMessagesAfterAppend(); len(steps3) != 0 { | |
| t.Errorf("unexpected new messages after pre-vote self-vote: %+v", steps3) | |
| } |
| t.Errorf("state = %v, want %v", sm.state, StateLeader) | ||
| } | ||
|
|
||
| // Its self-vote does not make its way to its ProgressTracker. |
There was a problem hiding this comment.
| // Its self-vote does not make its way to its ProgressTracker. | |
| // The delayed self-vote and pre-vote do not make its way to its ProgressTracker. |
|
Overall looks good to me. Just a couple of minor comments. |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
4 participants
This commit improves the testing around cases where a (pre-)candidate wins an election without having voted for themselves. This was already tested by TestCandidateDeliversPreCandidateSelfVoteAfterBecomingCandidate, but this commit expands the testing to handle three related cases and to mirror the existing TestCandidateSelfVoteAfterLostElection test.
cc. @pav-kv