-
Notifications
You must be signed in to change notification settings - Fork 33
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: split envtest/real cluster tests #788
test: split envtest/real cluster tests #788
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #788 +/- ##
===========================================
+ Coverage 31.90% 69.79% +37.89%
===========================================
Files 13 23 +10
Lines 536 1705 +1169
===========================================
+ Hits 171 1190 +1019
- Misses 361 396 +35
- Partials 4 119 +115
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
106ec61
to
f706ee7
Compare
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.
AMAZING work 👏
I left some really minor comments, this is good to be merged
@@ -36,7 +36,10 @@ jobs: | |||
- uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 | |||
with: | |||
go-version: "1.22" | |||
- run: make integration-tests | |||
- name: Install gingko |
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.
TODO: cache this step, maybe file an issue for that
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.
I am experimenting with something atm. Kubebuilder generates a Makefile that installs the needed tools in a bin
directory in the project root. We could add ginkgo to the automatically installed tools, cache the bin directory, and invalidate the cache if the Makefile changes. WDYT?
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.
That sounds fine to me, provided the tools versions are pinned in the Makefile. We have that layout in other repos.
I would prefer a GH action because they can or tend to verify the tools prior to install, even if that means that we manually sync tool version in makefile and GHA.
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.
LGTM! ❤️ , plus the CONTRIBUTING.md docs.
…llow parallel execution Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
… real-cluster Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
…test; clean-up Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
…-tests target in integration-tests-envtest and integration-tests-real-cluster Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
ac1102c
to
6a7e1a2
Compare
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Description
This PR splits the controller tests by using the fast and slow testing pattern.
It is now possible to run only certain tests against a real cluster while leveraging the envtest fake
Tests that need a real cluster to run must now be labeled with
real-cluster
.See https://onsi.github.io/ginkgo/#spec-labels.
kubernetes API server as the default test environment.
The policy server controller test was refactored to run envtest, which means that some assertions (for instance the ones on the Pod being created) are not possible anymore. However, they are not needed as we are not interested in testing that Kubernetes works.
Also, this PR:
integration-tests-envtest
andintegration-tests-real-cluster
targets to the Makefile, and updates the CI accordingly.ginkgo
test runner, adding a flag to enable GitHub action format outputNote: the test coverage seems to be dropping by 1/2%, by looking at the codecov report this is because the refactored tests are not causing errors due to conflicts so we are not passing through the error handling lines.
With these changes, the CI is taking ~6 minutes instead of ~28 minutes.