Add more test cases for kv range keys-only by nwnt · Pull Request #21229 · etcd-io/etcd

nwnt · 2026-02-01T06:03:08Z

Part of #20386

cc @serathius - trying to improve the readability only for the newly added test cases. If this works better than #21227, I'll adopt this approach instead.

k8s-ci-robot · 2026-02-01T06:03:13Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: nwnt
Once this PR has been reviewed and has the lgtm label, please assign ahrtr for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

codecov · 2026-02-01T06:44:59Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.39%. Comparing base (d89978e) to head (e0ba3bb).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files

see 26 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #21229      +/-   ##
==========================================
- Coverage   68.41%   68.39%   -0.02%     
==========================================
  Files         429      429              
  Lines       35288    35291       +3     
==========================================
- Hits        24142    24138       -4     
- Misses       9751     9757       +6     
- Partials     1395     1396       +1

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d89978e...e0ba3bb. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

serathius · 2026-02-02T09:06:23Z

tests/common/kv_test.go

-				slices.Reverse(reversedKvsKeysOnly)
+				allKvsSortedByValue := []*mvccpb.KeyValue{kvA, kvB, kvC, kvFoo, kvFooAbc, kvFop}
+				allKvsSortedByVersion := []*mvccpb.KeyValue{kvA, kvB, kvFoo, kvFooAbc, kvFop, kvC}
+				allKvsSortedByVersionDesc := append([]*mvccpb.KeyValue{kvC}, allKvsSortedByVersion[:5]...)


Not following how allKvsSortedByVersionDesc depends on allKvsSortedByVersion[:5]

Sorting by version descending for version doesn't reverse the non-descending version, but it moves kvC which has the highest version to the front while leaving the rest, which all have the same version number, in tact.

I was thinking that having allKvsSortedByVersion[:5] maybe would tell what happen better, but if it's not then making it {kvC, kvA, kvB, kvFoo, kvFooAbc, kvFop} is better then. What do you think?

Please not this should be readable and natural to humans. It's not obvious why SortedByVersion depends on SortedByVersionDesc. However this showed an separate interested property, how etcd sorts if the sorted key is constant across items.

Removed this and used literals instead.

serathius · 2026-02-02T09:12:40Z

tests/common/kv_test.go

+					{name: "all KVs with maximimum create revision, sorted by value", begin: "", options: config.GetOptions{Prefix: true, MaxCreateRevision: int(firstRev) + 5, Order: clientv3.SortDescend, SortBy: clientv3.SortByValue}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: reverse(allKvsSortedByValue[:4])}},
+					{name: "all KVs keys only, ascending", begin: "", options: config.GetOptions{Prefix: true, Order: clientv3.SortAscend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvs)}},
+					{name: "all KVs keys only, descending", begin: "", options: config.GetOptions{Prefix: true, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(reversedKvs)}},
+					{name: "all KVs keys only, sorted by value", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByValue, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvsSortedByValue)}},
+					{name: "all KVs keys only, sorted by value descending", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByValue, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(reverse(allKvsSortedByValue))}},
+					{name: "all KVs keys only, sorted by value descending limit 2", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByValue, Order: clientv3.SortDescend, KeysOnly: true, Limit: 2}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, More: true, Kvs: dropValue(reverse(allKvsSortedByValue)[:2])}},
+					{name: "all KVs keys only, sorted by version", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByVersion, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvsSortedByVersion)}},
+					{name: "all KVs keys only, sorted by version descending", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByVersion, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvsSortedByVersionDesc)}},
+					{name: "all KVs keys only, sorted by key", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByKey, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvs)}},
+					{name: "all KVs keys only, sorted by key", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByKey, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(reversedKvs)}},
+					{name: "all KVs keys only, sorted by create rev", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByCreateRevision, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvs)}},
+					{name: "all KVs keys only, sorted by create rev desc", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByCreateRevision, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(reversedKvs)}},
+					{name: "all KVs keys only, sorted by mod rev", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByModRevision, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvs)}},
+					{name: "all KVs keys only, sorted by mod rev desc", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByModRevision, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(reversedKvs)}},
 				}


Suggested change

{name: "all KVs with maximimum create revision, sorted by value", begin: "", options: config.GetOptions{Prefix: true, MaxCreateRevision: int(firstRev) + 5, Order: clientv3.SortDescend, SortBy: clientv3.SortByValue}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: reverse(allKvsSortedByValue[:4])}},

{name: "all KVs keys only, ascending", begin: "", options: config.GetOptions{Prefix: true, Order: clientv3.SortAscend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvs)}},

{name: "all KVs keys only, descending", begin: "", options: config.GetOptions{Prefix: true, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(reversedKvs)}},

{name: "all KVs keys only, sorted by value", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByValue, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvsSortedByValue)}},

{name: "all KVs keys only, sorted by value descending", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByValue, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(reverse(allKvsSortedByValue))}},

{name: "all KVs keys only, sorted by value descending limit 2", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByValue, Order: clientv3.SortDescend, KeysOnly: true, Limit: 2}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, More: true, Kvs: dropValue(reverse(allKvsSortedByValue)[:2])}},

{name: "all KVs keys only, sorted by version", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByVersion, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvsSortedByVersion)}},

{name: "all KVs keys only, sorted by version descending", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByVersion, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvsSortedByVersionDesc)}},

{name: "all KVs keys only, sorted by key", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByKey, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvs)}},

{name: "all KVs keys only, sorted by key", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByKey, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(reversedKvs)}},

{name: "all KVs keys only, sorted by create rev", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByCreateRevision, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvs)}},

{name: "all KVs keys only, sorted by create rev desc", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByCreateRevision, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(reversedKvs)}},

{name: "all KVs keys only, sorted by mod rev", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByModRevision, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(allKvs)}},

{name: "all KVs keys only, sorted by mod rev desc", begin: "", options: config.GetOptions{Prefix: true, SortBy: clientv3.SortByModRevision, Order: clientv3.SortDescend, KeysOnly: true}, wantResponse: &clientv3.GetResponse{Header: createHeader(firstRev + 7), Count: 6, Kvs: dropValue(reversedKvs)}},

}

}

testsWithKeysOnly := []struct{}{}

for _, t := range tests {

withKeyOnly := t

withKeyOnly.options.KeysOnly = true

withKeyOnly.wantResponse.Kvs = dropValue(withKeyOnly.wantResponse.Kvs)

testsWithKeysOnly = append(testsWithKeysOnly, withKeyOnly)

}

tests = append(tests, testsWithKeysOnly...)

Those test cases seem redundant.

Thanks for this suggestion, it's really good. Had to debug some pointer issue, but should be good now.

Signed-off-by: Nont <nont@duck.com>

serathius · 2026-02-03T14:00:37Z

tests/common/kv_test.go

+					withKeysOnly.wantResponse = &wantResponse
+					testsWithKeysOnly = append(testsWithKeysOnly, withKeysOnly)
+				}
+				for _, tt := range slices.Concat(tests, testsWithKeysOnly) {


Could you split the PR into smaller steps? There are to many changes at once to properly review it.

Yep, certainly

Broke this down to two new PRs: one adding the sorting test cases, the other making the test cases do --keys-only.

k8s-ci-robot · 2026-02-03T14:25:37Z

@nwnt: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-etcd-verify	`e0ba3bb`	link	true	`/test pull-etcd-verify`
pull-etcd-integration-1-cpu-amd64	`e0ba3bb`	link	true	`/test pull-etcd-integration-1-cpu-amd64`
pull-etcd-e2e-amd64	`e0ba3bb`	link	true	`/test pull-etcd-e2e-amd64`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions 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. I understand the commands that are listed here.

k8s-ci-robot · 2026-02-04T18:27:16Z

PR needs rebase.

Details

Instructions 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.

k8s-ci-robot added area/testing size/M labels Feb 1, 2026

nwnt force-pushed the add-range-test-additional-keys-only-combo branch from 7273c42 to 3c2e4b0 Compare February 1, 2026 06:03

serathius reviewed Feb 2, 2026

View reviewed changes

Add more test cases for kv range keys-only

e0ba3bb

Signed-off-by: Nont <nont@duck.com>

nwnt force-pushed the add-range-test-additional-keys-only-combo branch from 3c2e4b0 to e0ba3bb Compare February 3, 2026 13:50

k8s-ci-robot added size/L and removed size/M labels Feb 3, 2026

serathius reviewed Feb 3, 2026

View reviewed changes

This was referenced Feb 3, 2026

Add sorting test cases for range operations #21246

Open

Add keys only range test cases #21247

Merged

k8s-ci-robot added the needs-rebase label Feb 4, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more test cases for kv range keys-only#21229

Add more test cases for kv range keys-only#21229
nwnt wants to merge 1 commit intoetcd-io:mainfrom
nwnt:add-range-test-additional-keys-only-combo

nwnt commented Feb 1, 2026

Uh oh!

k8s-ci-robot commented Feb 1, 2026

Uh oh!

codecov bot commented Feb 1, 2026 •

edited

Loading

Uh oh!

serathius Feb 2, 2026

Uh oh!

nwnt Feb 2, 2026

Uh oh!

serathius Feb 2, 2026

Uh oh!

nwnt Feb 3, 2026

Uh oh!

serathius Feb 2, 2026

Uh oh!

nwnt Feb 3, 2026

Uh oh!

serathius Feb 3, 2026

Uh oh!

nwnt Feb 3, 2026

Uh oh!

nwnt Feb 3, 2026

Uh oh!

k8s-ci-robot commented Feb 3, 2026

Uh oh!

k8s-ci-robot commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Conversation

nwnt commented Feb 1, 2026

Uh oh!

k8s-ci-robot commented Feb 1, 2026

Uh oh!

codecov bot commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Feb 3, 2026

Uh oh!

k8s-ci-robot commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

codecov bot commented Feb 1, 2026 •

edited

Loading