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
storage: confirm that paging and predicate filtering work together #88674
Conversation
@@ -1263,6 +1275,12 @@ func TestListContinuation(t *testing.T) { | |||
t.Logf("continue token was %d %s %v", rv, key, err) | |||
t.Fatalf("Unexpected second page: %#v", out.Items) | |||
} | |||
if transformer.reads != 3 { |
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.
I might feel slightly better if you reset the transformer and recorder reads count before you listed, so each test tracked the count for that section
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.
Done.
One minor structural comment (resetting the count between each segment for more clarity) but otherwise this is excellent |
1f787f7
to
544ef64
Compare
@@ -1275,7 +1302,17 @@ func TestListContinuation(t *testing.T) { | |||
if len(out.Items) != 1 || !reflect.DeepEqual(&out.Items[0], preset[1].storedObj) { | |||
t.Fatalf("Unexpected second page: %#v", out.Items) | |||
} | |||
if transformer.reads != 1 { | |||
t.Errorf("expected 4 reads, got %d", transformer.reads) |
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.
I would change all these messages to "unexpected reads: %d", generically, since now they're wrong.
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.
Thanks for catching this. Fixed.
This change adds the TestListContinuationWithFilter test which confirms that paging with a predicate that does not match everything results in the correct amount of calls to TransformFromStorage and KV.Get. The partial result of each paging call is also asserted. Signed-off-by: Monis Khan <mok@vmware.com>
544ef64
to
002c754
Compare
/lgtm Thanks |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enj, smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/milestone v1.18 |
This change adds the TestListContinuationWithFilter test which
confirms that paging with a predicate that does not match everything
results in the correct amount of calls to TransformFromStorage and
KV.Get. The partial result of each paging call is also asserted.
Signed-off-by: Monis Khan mok@vmware.com
/kind bug
/priority important-soon