-
Notifications
You must be signed in to change notification settings - Fork 38.8k
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
Move UTs that block on apiserver to integration tests. #30634
Move UTs that block on apiserver to integration tests. #30634
Conversation
I think I'm fine with this PR. But please fix tests. |
87f636b
to
e9e78d7
Compare
re: GKE tests, I don't see how this change could effect it. It's strictly a UT->Integ change and the GKE test is failing on some cluster bring-up. |
@k8s-bot test this issue: #IGNORE |
@@ -196,3 +196,6 @@ test/integration/openshift | |||
test/soak/cauldron | |||
test/soak/serve_hostnames | |||
third_party/forked/golang/expansion | |||
test/integration/discoverysummarizer |
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.
What's this "linted packages"?
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.
It's part of the verify scripts, it's why it failed last time.
lgtm |
e9e78d7
to
40fb757
Compare
Tests are failing. |
40fb757
to
cc437d2
Compare
There is no way those error(s) are from this PR. |
cc437d2
to
0d5af31
Compare
LGTM |
0d5af31
to
261f1e2
Compare
@k8s-bot test this please, issue #IGNORE |
…ver being fully stood up should be integration tests, not unit tests.
261f1e2
to
d6606a6
Compare
@k8s-bot test this please, issue #IGNORE |
GCE e2e build/test passed for commit d6606a6. |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
Sorry it is not clear to me why should these tests be in integration and not unit tests? They are not trying to bring up multiple components, just testing the Run method in their respective classes. |
GCE e2e build/test passed for commit d6606a6. |
Automatic merge from submit-queue |
@nikhiljindal If you wish to standup the apiserver using a v3client you need a running etcd3. These tests expected the apiserver to be running. |
Automatic merge from submit-queue Enable v3 Client as the default on UTs Updates the default initialization to use clientv3 interface to etcd3, and fixes the UTs. This PR includes a cherry-pick of #30634 so we can validate the tests, so do not merge until that PR is complete.
In validating etcd.v3client we had uncovered that a change in the behavior of the client etcd-io/etcd#6162 , caused a number of unit tests to fail. These test failures were due to the fact that the unit tests were trying to standup a apiserver even though there was no etcd backend stood up.
This PR simply shuffles those tests to integration tests, which is where they should be.
/cc @kubernetes/sig-scalability @wojtek-t @hongchaodeng @xiang90
This change is