Skip to content
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

Testing arcade controller w/ginkgo #4

Merged
merged 7 commits into from
Dec 2, 2020
Merged

Testing arcade controller w/ginkgo #4

merged 7 commits into from
Dec 2, 2020

Conversation

adams0619
Copy link
Contributor

@adams0619 adams0619 commented Oct 30, 2020

PR to setup testing using ginkgo, specifically:

  • A tests for the main controller: arcade_controller.go.
  • Adds setup-envtest.sh for setting up mock k8s environment for envtest
    • Script will download the necessary etcd, kubectl, and kube-apiserver if not preset within testbin
  • Add generated CRD for arcade-operator in config/crd/bases/....

Testing process:

Run tests by running make test from the project root.

@adams0619 adams0619 changed the title Attempt to set up testing w/ginkgo Testing arcade controller w/ginkgo Nov 6, 2020
@adams0619 adams0619 requested a review from gebhardtr November 10, 2020 15:42
@adams0619 adams0619 mentioned this pull request Nov 11, 2020
@@ -121,7 +121,6 @@ func (r *ArcadeReconciler) SetupWithManager(mgr ctrl.Manager) error {
For(&arcadev1alpha1.Arcade{}).
Owns(&appsv1.Deployment{}).
Owns(&corev1.Service{}).
Owns(&routev1.Route{}).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're creating the Route in the controller; isn't this still required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we are creating routes, but when testing with envtest, I have not yet found a way to add third party APIs as part of the test kube-apiserver's known CRDs... still looking into it though.

@zevioso
Copy link
Contributor

zevioso commented Nov 19, 2020

reviewing. 👀

Copy link
Contributor

@zevioso zevioso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good based on the few articles I've read on testing operators.

"context"
"time"

. "github.com/onsi/ginkgo"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the . necessary here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the . imports ginkgo into the file block without requiring a qualified identifier when using functions or variables from that package... removing the need to always type something like ginkgo.BeforeSuite or gomega.Expect for every exported function or value that is used from that package.

test: generate fmt vet manifests
go test ./... -coverprofile cover.out
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.6.3/hack/setup-envtest.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor comment: any reason we shouldn't always download or never download (if the file is included in the repo)?

Copy link
Contributor Author

@adams0619 adams0619 Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The binaries used for testing like the kube-apiserver is big, at 180MB for just the server, this exceeds GitHub's file size limit of 100MB for individual files, so downloading on demand keeps things flexible without needing to introduce something like git-lfs. Likewise, since Operator-SDK 1.1.0+, this is included as part of the scaffolding by default, in the generated Makefile

@gebhardtr gebhardtr merged commit da0d93c into main Dec 2, 2020
@zevioso zevioso deleted the tests-ginkgo branch December 7, 2020 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants