-
Notifications
You must be signed in to change notification settings - Fork 24
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 e2e test #197
Fix e2e test #197
Conversation
Unfortunately the test is still failing :-( |
35e2d20
to
7b8ed41
Compare
This PR is to fix TestKubeletConfigRemediation e2e test
What was the issue? |
There was some issue with the rule in the test content image |
@@ -3557,7 +3529,7 @@ func TestE2E(t *testing.T) { | |||
|
|||
// We need to check that the remediation is auto-applied and save |
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.
We could check that the check we're remediating failed before we attempt to fix it. I'm thinking about if the defaults change or for some reason the check starts to pass automatically, we wouldn't be testing the remediation, would we?
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.
If the check pass, it will not generate a remediation here
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.
Don't we want to test that it does?
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 think it is being checked, see line #3523 https://github.com/ComplianceAsCode/compliance-operator/pull/197/files#diff-7d1fc1e232b55677582b51e2623aae6d8c15b5f0960ac6100fc7d733cb795dacR3523
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 you mean check the result of the first scan:
err = waitForSuiteScansStatus(t, f, namespace, suiteName, compv1alpha1.PhaseDone, compv1alpha1.ResultNonCompliant)
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.
Aha - got it, thanks!
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 GitHub diff sometimes doesn't show the full code haha
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 review!
Rationale: "To be tested", | ||
}, | ||
{ | ||
Name: "ocp4-version-detect-in-ocp", |
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.
We're not checking the status of this below anywhere. Do we need it, or should we add an assertion for it?
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 you mean the checkResult, I think it is being check at the end, on line 3565,
err = assertHasCheck(f, suiteName, scanName, checkResult)
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhmdnd, Vincent056 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 |
adding qe, px, and docs approve because it is a e2e test change |
This PR is to fix TestKubeletConfigRemediation e2e test