CMP-4616: Fix flaky TestScheduledSuiteNoStorage e2e test - #1369
Conversation
|
@yuumasato: This pull request references CMP-4616 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.1.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
🤖 To deploy this PR, run the following command: |
|
@guzalv: changing LGTM is restricted to collaborators DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Hey @yuumasato, everything looks good. Claude did find little nit though: The commit message says err = f.WaitForSuiteScansStatusAnyResult(f.OperatorNamespace, suiteName, compv1alpha1.PhaseDone,
compv1alpha1.ResultCompliant, compv1alpha1.ResultNonCompliant, compv1alpha1.ResultError)If tolerating ERROR is intentionally undesirable, it may be worth dropping it from the commit message instead. |
TestScheduledSuiteNoStorage validates that no PVCs are created when RawResultStorage is disabled. The test was brittle because it only accepted ResultCompliant, but under cluster pressure from ~70 parallel tests the scan can finish with a different result, causing an immediate hard failure in WaitForSuiteScansStatus. Since the test validates storage behavior and not compliance outcome, accept COMPLIANT, NON_COMPLIANT, and ERROR results using WaitForSuiteScansStatusAnyResult. Also move the PVC assertion after WaitForSuiteScansStatusAnyResult so it runs once the scan has actually completed, matching the pattern used by TestScheduledSuitePlatformNoStorage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6dfabb6 to
9535c01
Compare
|
@abushkin-redhat Nice catch, updated. |
|
/lgtm |
|
🤖 To deploy this PR, run the following command: |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abushkin-redhat, guzalv, yuumasato The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
LGTM overall — accepting NON_COMPLIANT is right for a storage-behavior test, and moving the PVC assertion after suite completion is strictly stronger than the old early check (it now catches PVCs created at any point of the run). One follow-up worth considering: accepting |
Summary
TestScheduledSuiteNoStoragevalidates that no PVCs are created whenRawResultStorageis disabled — it tests storage behavior, not compliance outcomeResultCompliant, but under cluster pressure from ~70 parallel tests the scan can returnNON_COMPLIANTorERROR, causing an immediate hard failureWaitForSuiteScansStatusAnyResultaccepting bothCOMPLIANTandNON_COMPLIANTTestScheduledSuitePlatformNoStorageTest plan
TestScheduledSuiteNoStorageflakingJira: CMP-4616
🤖 Generated with Claude Code