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

tests: add helpers for generating entry slices #147

Merged
merged 4 commits into from
Feb 6, 2024

Conversation

pav-kv
Copy link
Contributor

@pav-kv pav-kv commented Jan 31, 2024

Part of #146

@pav-kv pav-kv requested a review from serathius January 31, 2024 03:24
@pav-kv
Copy link
Contributor Author

pav-kv commented Jan 31, 2024

@serathius PTAL. This is just a mechanical clean-up in tests.

@pav-kv pav-kv mentioned this pull request Jan 31, 2024
6 tasks
log_test.go Show resolved Hide resolved
Copy link
Contributor Author

@pav-kv pav-kv left a comment

Choose a reason for hiding this comment

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

Thank you for the review, good points. Updated. If happy with the current state, please merge.

log_test.go Show resolved Hide resolved
@pav-kv pav-kv requested a review from serathius February 2, 2024 15:23
@pav-kv pav-kv force-pushed the entry-slice-helper branch 2 times, most recently from 856c869 to e1507e6 Compare February 3, 2024 14:46
Copy link
Contributor Author

@pav-kv pav-kv left a comment

Choose a reason for hiding this comment

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

@serathius I converted a few more test files.

Once happy, please merge. Thank you.

{[]pb.Entry{{Term: 1, Index: 1}, {Term: 1, Index: 2}}, 2, 1, false},
// voter higher logterm
{[]pb.Entry{{Term: 2, Index: 1}}, 1, 1, true},
{[]pb.Entry{{Term: 2, Index: 1}}, 1, 2, true},
{[]pb.Entry{{Term: 2, Index: 1}, {Term: 1, Index: 2}}, 1, 1, true},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line was incorrect: terms in a log can not regress. Added two correct tests instead.

Signed-off-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
Signed-off-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
Signed-off-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
Signed-off-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
Copy link
Member

@serathius serathius left a comment

Choose a reason for hiding this comment

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

cc @ahrtr

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

thx for simplifying the test cases.

@ahrtr ahrtr merged commit fac6f16 into etcd-io:main Feb 6, 2024
10 checks passed
@pav-kv pav-kv deleted the entry-slice-helper branch February 6, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants