-
Notifications
You must be signed in to change notification settings - Fork 251
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
[test]Add e2e testcases, integrates with CI #206
Conversation
Signed-off-by: zhu733756 <[email protected]>
Signed-off-by: zhu733756 <[email protected]>
Signed-off-by: zhu733756 <[email protected]>
7d6a984
to
4c70406
Compare
Signed-off-by: zhu733756 <[email protected]>
@@ -63,7 +63,7 @@ ENVTEST_ASSETS_DIR=$(shell pwd)/testbin | |||
test: manifests generate fmt vet ## Run tests. | |||
mkdir -p ${ENVTEST_ASSETS_DIR} | |||
test -f ${ENVTEST_ASSETS_DIR}/setup-envtest.sh || curl -sSLo ${ENVTEST_ASSETS_DIR}/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/v0.8.3/hack/setup-envtest.sh | |||
source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out | |||
source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./apis/... -coverprofile cover.out |
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.
Currently, the function tests are located in this path. Make it not include the e2e paths.
} | ||
|
||
cfg.Spec.WatchedNamespaces = allNamespaces | ||
// Don't patch the CR, or it would requeue. | ||
cfg.Spec.WatchedNamespaces = watchedNamespaces | ||
} |
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.
Do we have to put every namespace into WatchedNamespaces?
No easier way to represent all namespace
instead of iterating every namespace?
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.
The former codes cache it, I wonder if the namespaces changed, would mix some namespaces.
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.
Let's discuss this later, I think there is no need to iterating every ns in case of all namespace
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.
OK.
5bd51da
to
5610e17
Compare
Signed-off-by: zhu733756 <[email protected]>
Signed-off-by: zhu733756 <[email protected]>
zhu733756 [email protected]
/cc @benjaminhuo @wanjunlei @wenchajun