storage/etcd3: continue unifying test setup #109618
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Please note that we're already in Test Freeze for the |
k8s-ci-robot
added
release-note-none
k8s-ci-robot
added
cncf-cla: yes
stevekuznetsov
force-pushed
the
skuznets/common-storage-test-setup
branch
from
f3507bf
to
d6c02ab
Compare
k8s-ci-robot
removed
the
do-not-merge/invalid-commit-message
|
/sig api-machinery |
stevekuznetsov
commented
Apr 22, 2022
flake |
wojtek-t
reviewed
Apr 25, 2022
|
@wojtek-t thanks for the review! Updated |
stevekuznetsov
force-pushed
the
skuznets/common-storage-test-setup
branch
2 times, most recently
from
b5f8865
to
aec5853
Compare
|
/test pull-kubernetes-e2e-kind |
stevekuznetsov
force-pushed
the
skuznets/common-storage-test-setup
branch
from
aec5853
to
9160626
Compare
|
@wojtek-t couldn't remove |
|
/test pull-kubernetes-node-e2e-containerd |
wojtek-t
reviewed
Apr 26, 2022
k8s-ci-robot
added
the
approved
Previous work by liggitt in 0176092 improved the boilerplate required to run an embedded etcd server for tests as well as set up the `*etcd3.store{}` for testing. A number of tests were not ported to use the new helpers, though, either due to custom setup or due to inconsistent use of setup options. A follow-up by stevekuznetsov in 6aa37eb removed much of the inconsistency, meaning that most callers to `newStore()` were simply using the default boilerplate and options that `testSetup()` used. This patch moves all users to testSetup(), adding options as necessary to enable some fringe setup use-cases. With a unified setup, new tests will not copy boilerplate they do not need and it will be immediately obvious when reading a test if the client or storage setup is *not* default, improving readability. Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
|
@wojtek-t updated to do a hard cast |
stevekuznetsov
force-pushed
the
skuznets/common-storage-test-setup
branch
from
9160626
to
138faa3
Compare
|
/triage accepted |
k8s-ci-robot
added
triage/accepted
|
/lgtm Thanks! |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: stevekuznetsov, wojtek-t 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
storage/etcd3: continue unifying test setup
Previous work by liggitt in 0176092 improved the boilerplate
required to run an embedded etcd server for tests as well as set up the
*etcd3.store{}for testing. A number of tests were not ported to use thenew helpers, though, either due to custom setup or due to inconsistent
use of setup options. A follow-up by stevekuznetsov in 6aa37eb
removed much of the inconsistency, meaning that most callers to
newStore()were simply using the default boilerplate and options thattestSetup()used.This patch moves all users to testSetup(), adding options as necessary
to enable some fringe setup use-cases. With a unified setup, new tests
will not copy boilerplate they do not need and it will be immediately
obvious when reading a test if the client or storage setup is not
default, improving readability.
Signed-off-by: Steve Kuznetsov skuznets@redhat.com
/kind cleanup
/assign @wojtek-t
/cc @liggitt