refactor(e2e): remove reliance on Repository.Status in wait functions#2789
refactor(e2e): remove reliance on Repository.Status in wait functions#2789zakisk wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the end-to-end tests to monitor and assert on Tekton PipelineRuns directly rather than waiting for Repository status updates. The feedback highlights a recurring issue across multiple test files where the code directly accesses the Conditions slice by index (e.g., Conditions[0]) on a PipelineRun status. Since the order of conditions is not guaranteed, it is highly recommended to use GetCondition(apis.ConditionSucceeded) to safely and reliably retrieve the 'Succeeded' condition.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for _, pr := range prs.Items { | ||
| if len(pr.Status.Conditions) > 0 && pr.Status.Conditions[0].Status != corev1.ConditionUnknown { | ||
| finished = append(finished, pr) | ||
| } | ||
| } |
There was a problem hiding this comment.
Directly accessing the Conditions slice by index (e.g., Conditions[0]) is unreliable because the order of conditions is not guaranteed. Instead, use GetCondition(apis.ConditionSucceeded) to retrieve the 'Succeeded' condition safely.
| for _, pr := range prs.Items { | |
| if len(pr.Status.Conditions) > 0 && pr.Status.Conditions[0].Status != corev1.ConditionUnknown { | |
| finished = append(finished, pr) | |
| } | |
| } | |
| for _, pr := range prs.Items { | |
| if cond := pr.Status.GetCondition(apis.ConditionSucceeded); cond != nil && cond.Status != corev1.ConditionUnknown { | |
| finished = append(finished, pr) | |
| } | |
| } |
References
- When checking a Tekton PipelineRun's status, use
GetCondition(apis.ConditionSucceeded)to reliably retrieve the 'Succeeded' condition, rather than accessing theConditionsslice by index (e.g.,Conditions[0]).
| g.Cnx.Clients.Log.Infof("Check if pipelinerun status shows succeeded") | ||
| assert.Equal(t, corev1.ConditionTrue, prs[len(prs)-1].Status.Conditions[0].Status) |
There was a problem hiding this comment.
Directly accessing the Conditions slice by index (e.g., Conditions[0]) is unreliable because the order of conditions is not guaranteed. Instead, use GetCondition(apis.ConditionSucceeded) to retrieve the 'Succeeded' condition safely.
| g.Cnx.Clients.Log.Infof("Check if pipelinerun status shows succeeded") | |
| assert.Equal(t, corev1.ConditionTrue, prs[len(prs)-1].Status.Conditions[0].Status) | |
| g.Cnx.Clients.Log.Infof("Check if pipelinerun status shows succeeded") | |
| cond := prs[len(prs)-1].Status.GetCondition(apis.ConditionSucceeded) | |
| assert.Assert(t, cond != nil) | |
| assert.Equal(t, corev1.ConditionTrue, cond.Status) |
References
- When checking a Tekton PipelineRun's status, use
GetCondition(apis.ConditionSucceeded)to reliably retrieve the 'Succeeded' condition, rather than accessing theConditionsslice by index (e.g.,Conditions[0]).
| g.Cnx.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded") | ||
| assert.Assert(t, prs[len(prs)-1].Status.Conditions[0].Status == corev1.ConditionTrue) |
There was a problem hiding this comment.
Directly accessing the Conditions slice by index (e.g., Conditions[0]) is unreliable because the order of conditions is not guaranteed. Instead, use GetCondition(apis.ConditionSucceeded) to retrieve the 'Succeeded' condition safely.
g.Cnx.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded")
cond := prs[len(prs)-1].Status.GetCondition(apis.ConditionSucceeded)
assert.Assert(t, cond != nil)
assert.Equal(t, corev1.ConditionTrue, cond.Status)References
- When checking a Tekton PipelineRun's status, use
GetCondition(apis.ConditionSucceeded)to reliably retrieve the 'Succeeded' condition, rather than accessing theConditionsslice by index (e.g.,Conditions[0]).
| g.Cnx.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded") | ||
| lastPR := prs[len(prs)-1] | ||
| assert.Equal(t, lastPR.Status.Conditions[0].Status, corev1.ConditionTrue) |
There was a problem hiding this comment.
Directly accessing the Conditions slice by index (e.g., Conditions[0]) is unreliable because the order of conditions is not guaranteed. Instead, use GetCondition(apis.ConditionSucceeded) to retrieve the 'Succeeded' condition safely.
| g.Cnx.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded") | |
| lastPR := prs[len(prs)-1] | |
| assert.Equal(t, lastPR.Status.Conditions[0].Status, corev1.ConditionTrue) | |
| g.Cnx.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded") | |
| lastPR := prs[len(prs)-1] | |
| cond := lastPR.Status.GetCondition(apis.ConditionSucceeded) | |
| assert.Assert(t, cond != nil) | |
| assert.Equal(t, corev1.ConditionTrue, cond.Status) |
References
- When checking a Tekton PipelineRun's status, use
GetCondition(apis.ConditionSucceeded)to reliably retrieve the 'Succeeded' condition, rather than accessing theConditionsslice by index (e.g.,Conditions[0]).
| topts.ParamsRun.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded") | ||
| lastPR := prs[len(prs)-1] | ||
| assert.Assert(t, lastPR.Status.Conditions[0].Status == corev1.ConditionTrue) |
There was a problem hiding this comment.
Directly accessing the Conditions slice by index (e.g., Conditions[0]) is unreliable because the order of conditions is not guaranteed. Instead, use GetCondition(apis.ConditionSucceeded) to retrieve the 'Succeeded' condition safely.
| topts.ParamsRun.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded") | |
| lastPR := prs[len(prs)-1] | |
| assert.Assert(t, lastPR.Status.Conditions[0].Status == corev1.ConditionTrue) | |
| topts.ParamsRun.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded") | |
| lastPR := prs[len(prs)-1] | |
| cond := lastPR.Status.GetCondition(apis.ConditionSucceeded) | |
| assert.Assert(t, cond != nil) | |
| assert.Equal(t, corev1.ConditionTrue, cond.Status) |
References
- When checking a Tekton PipelineRun's status, use
GetCondition(apis.ConditionSucceeded)to reliably retrieve the 'Succeeded' condition, rather than accessing theConditionsslice by index (e.g.,Conditions[0]).
| runcnx.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded") | ||
| assert.Equal(t, corev1.ConditionTrue, pr.Status.Conditions[0].Status) |
There was a problem hiding this comment.
Directly accessing the Conditions slice by index (e.g., Conditions[0]) is unreliable because the order of conditions is not guaranteed. Instead, use GetCondition(apis.ConditionSucceeded) to retrieve the 'Succeeded' condition safely.
runcnx.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded")
cond := pr.Status.GetCondition(apis.ConditionSucceeded)
assert.Assert(t, cond != nil)
assert.Equal(t, corev1.ConditionTrue, cond.Status)References
- When checking a Tekton PipelineRun's status, use
GetCondition(apis.ConditionSucceeded)to reliably retrieve the 'Succeeded' condition, rather than accessing theConditionsslice by index (e.g.,Conditions[0]).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2789 +/- ##
=======================================
Coverage 59.73% 59.73%
=======================================
Files 210 210
Lines 21117 21117
=======================================
Hits 12615 12615
Misses 7707 7707
Partials 795 795 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
b51d85b to
c211bca
Compare
The Repository CR's Status field is deprecated and will be removed. This refactors E2E test wait helpers to use PipelineRun objects directly instead of polling Repository.Status: - Remove UntilRepositoryUpdated and UntilRepositoryHasStatusReason - Remove FailOnRepoCondition from wait.Opts - Modify UntilPipelineRunCreated and UntilPipelineRunHasReason to return []PipelineRun so callers can inspect annotations and conditions - Add UntilPipelineRunsFinished for mixed-outcome tests (1 pass + 1 fail) - Rewrite Succeeded() helper to validate PipelineRun annotations instead of repo.Status fields - Migrate all ~40 call sites across 20 test files Signed-off-by: Zaki Shaikh <zashaikh@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
c211bca to
51f1574
Compare
📝 Description of the Change
The Repository CR's Status field is deprecated and will be removed. This refactors E2E test wait helpers to use PipelineRun objects directly instead of polling Repository.Status:
🔗 Linked GitHub Issue
Fixes #
🧪 Testing Strategy
🤖 AI Assistance
AI assistance can be used for various tasks, such as code generation,
documentation, or testing.
Please indicate whether you have used AI assistance
for this PR and provide details if applicable.
Important
Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Claude noreply@anthropic.com
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.