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

Parallelise and randomize tests #331

Merged
merged 1 commit into from
Oct 4, 2022
Merged

Parallelise and randomize tests #331

merged 1 commit into from
Oct 4, 2022

Conversation

squaremo
Copy link
Contributor

@squaremo squaremo commented Oct 4, 2022

Many of the tests take O(1m) to run, and the more specs added the longer the runtime. Parallelising the tests will help defer the day we need to increase the overall timeout.

Randomising helps keep the tests independent, by surfacing things that work by coincidence.

Enabling either of these requires the use of the Ginkgo command line -- it's not possible to enable them programmatically. So: reinstate the use of ginkgo for running tests.

(Empirically, I found that using ginkgo -p would result in timeouts waiting for the control plane to start. -nodes=4 seems like a reasonable compromise.)

Many of the tests take O(1m) to run, and the more specs added the longer
the runtime. Parallelising the tests will help defer the day we need to
increase the overall timeout.

Randomising helps keep the tests independent, by surfacing things that
work by coincidence.

Enabling either of these requires the use of the Ginkgo command line --
it's not possible to enable them programmatically. So: reinstate the use
of `ginkgo` for running tests.

Signed-off-by: Michael Bridgen <[email protected]>
@squaremo squaremo added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Oct 4, 2022
@@ -45,7 +45,8 @@ push-image:
docker push $(IMAGE_NAME):$(VERSION)

test: codegen download-test-deps
KUBEBUILDER_ASSETS="$(shell setup-envtest --use-env use -p path)" go test -v ./test/...
KUBEBUILDER_ASSETS="$(shell setup-envtest --use-env use -p path)" \
ginkgo -nodes=4 --randomizeAllSpecs ./test/...
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this now also a dev machine dependency? Should it be added to README/CONTRIBUTING?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ginkgo is mentioned as a prerequisite in docs/build.md already (I didn't remove it when I changed the Makefile the last time).

@squaremo squaremo merged commit da50309 into master Oct 4, 2022
@squaremo squaremo deleted the parallelise-tests branch October 4, 2022 15:48
@squaremo squaremo mentioned this pull request Oct 10, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants