-
Notifications
You must be signed in to change notification settings - Fork 131
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
fix: adds missing CRDs for k8s EnvTest #584
fix: adds missing CRDs for k8s EnvTest #584
Conversation
- introduces make func to download needed CRDs to config/crd/external when running make manifests target - adds missing scheme installation in test setup - marks one test as Pending due to NetworkPolicies being disabled right now
@@ -48,7 +48,8 @@ var _ = Describe("DataScienceCluster initialization", func() { | |||
Expect(foundMonitoringNamespace.Name).Should(Equal(monitoringNamespace)) | |||
}) | |||
|
|||
It("Should create default network policy", func() { | |||
// Currently commented out in the DSCI reconcile - setting test to Pending | |||
PIt("Should create default network policy", func() { |
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.
P ?
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.
Yes, otherwise the test will fail as the code for creating NetworkPolicy is commented out. PIt
- marks test case as Pending in the execution report. You can also XIt
- exclude it, and FIt
- focus test suite to run only tests with F
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 was hoping that the comment one line above makes it clear :)
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.
ah, nice ! new things learnt every day
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.
Thanks for the PR @bartoszmajsak
Just a note, this did not show up in CI, as we directly to docker build..
in CI job.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: VaishnaviHire 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 |
Description
After latest merges EvnTest tests stopped working. This PR addresses the problems by:
config/crd/external
when runningmake manifests
targetHow Has This Been Tested?
make test
Merge criteria: