-
Notifications
You must be signed in to change notification settings - Fork 73
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
E2e tests for K8sImagePuller validating webhook #840
E2e tests for K8sImagePuller validating webhook #840
Conversation
Hi @dkwon17. Thanks for your PR. I'm waiting for a codeready-toolchain member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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/test-infra repository. |
/ok-to-test |
/retest infra |
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.
Looks good. However if #836 is merged first then you won't need to disable the test anymore.
testsupport/wait/member.go
Outdated
if val := os.Getenv("skip-webhook-checks-on-setup"); val == "true" { | ||
// skipped temporarily only for setup migration test but applies for after migration test | ||
// This should be removed after PR https://github.com/codeready-toolchain/member-operator/pull/493 is merged | ||
return | ||
} |
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.
#836 is already merged, you can just rebase your branch with the latest changes and drop this workaround
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 with latest push
Signed-off-by: David Kwon <[email protected]>
@MatousJobanek @alexeykazakov I rebased the PR (since it's a small PR) and dropped the workaround |
Sorry @alexeykazakov , maybe it was better to not rebase and force push the PR in this case, I've rerun the e2e tests to make sure it passes again: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/codeready-toolchain_member-operator/493/pull-ci-codeready-toolchain-member-operator-master-e2e/1724443025058304000 |
You don't have to push with force to re-run the tests. You can do |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, dkwon17 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 |
d2e570a
into
codeready-toolchain:master
E2e tests for codeready-toolchain/member-operator#493
I've added a commit to temporarily disable validating webhook tests (just like in #809).I plan to make a PR to enable the validating webhook tests after codeready-toolchain/member-operator#493 is merged