TRT-2273: Show Regressions for Test Analysis#3775
TRT-2273: Show Regressions for Test Analysis#3775openshift-trt-agent[bot] wants to merge 3 commits into
Conversation
Add an Active Regressions section to the Issues card on the Test Analysis page. The section fetches Component Readiness regressions for the current test and release, displays them in a table with variant, timing, and triage info, and filters by any variant filters applied on the page. Backend: add test_name query parameter to the GET /api/component_readiness/regressions endpoint for server-side filtering. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci
Bot
added
the
ready-for-human-review
|
@openshift-trt-agent[bot]: This pull request references TRT-2273 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 story to target the "5.0.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. |
openshift-ci-robot
added
the
jira/valid-reference
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: openshift-trt-agent[bot] The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
needs-ok-to-test
|
Hi @openshift-trt-agent[bot]. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
| func ListRegressions(dbc *db.DB, release, testName string, views []crview.View, releases []v1.Release, crTimeRoundingFactor, crTimeRoundingOffset time.Duration, req *http.Request) ([]models.TestRegression, error) { | ||
| var regressions []models.TestRegression | ||
| var err error | ||
| regressions, err = query.ListRegressions(dbc, release) | ||
| regressions, err = query.ListRegressions(dbc, release, testName) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
I'd rather not see this reused in this manner. We would be better off with a GetRegressionForTest function that was only (currently) used for this purpose.
There was a problem hiding this comment.
Created a dedicated GetRegressionsForTest function in both triage_queries.go and triage.go, and reverted ListRegressions back to its original signature. The handler in server.go now dispatches to GetRegressionsForTest when the test parameter is provided, and to ListRegressions otherwise.
| "job": nameRegexp, | ||
| "job_name": nameRegexp, | ||
| "test": regexp.MustCompile(`^.+$`), // tests can be anything, so always parameterize in sql | ||
| "test_name": regexp.MustCompile(`^.+$`), // test names can be anything, always parameterize in sql |
There was a problem hiding this comment.
can't we use "test" here for this? how is that any different? what would "test" be if not a test name?
There was a problem hiding this comment.
Good point. Switched to using the existing test parameter instead of adding test_name. Removed the test_name entry from param.go.
| <TableCell> | ||
| {triaged ? ( | ||
| regression.triages.map((t) => ( | ||
| <Chip | ||
| key={t.id} | ||
| label={ | ||
| t.url ? t.url.split('/').pop() : `Triage #${t.id}` | ||
| } | ||
| component={Link} | ||
| to={`/sippy-ng/component_readiness/triages/${t.id}`} | ||
| clickable | ||
| size="small" | ||
| color="primary" | ||
| variant="outlined" | ||
| sx={{ marginRight: 0.5 }} | ||
| /> | ||
| )) | ||
| ) : ( | ||
| <Chip label="Untriaged" size="small" color="warning" /> | ||
| )} | ||
| </TableCell> |
There was a problem hiding this comment.
Multiple Triages can exist for a single regression. I don't think we should surface triages here at all as it just makes things too complex.
There was a problem hiding this comment.
Agreed. Removed the Triage column entirely from the table.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Caution Review failedAn error occurred during the review process. Please try again later. WalkthroughThe component readiness regressions API now supports exact ChangesComponent readiness regressions
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TestAnalysis
participant TestRegressionsTable
participant ComponentReadinessAPI
participant RegressionQuery
TestAnalysis->>TestRegressionsTable: pass release, testName, filterModel
TestRegressionsTable->>ComponentReadinessAPI: request regressions
ComponentReadinessAPI->>RegressionQuery: filter by release and test_name
RegressionQuery-->>ComponentReadinessAPI: return regressions
ComponentReadinessAPI-->>TestRegressionsTable: return regression records
TestRegressionsTable-->>TestAnalysis: render active regressions
Suggested reviewers: 🚥 Pre-merge checks | ✅ 18 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (18 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
- Create dedicated GetRegressionsForTest function instead of modifying ListRegressions, keeping the existing function's contract unchanged - Use existing "test" query parameter instead of adding "test_name" - Remove Triage column from TestRegressionsTable to avoid complexity from multiple triages per regression Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The Implementation is entirely untested. We need to add the proper unit and e2e tests. No frontend tests are necessary. |
Test the new `test` query parameter on the /api/component_readiness/regressions endpoint that was added to support the Test Analysis regressions widget. Covers filtering by test name with and without release, and verifying empty results for non-existent test names. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Added e2e tests for the
Per project testing conventions, the DB query layer ( |
|
/ok-to-test |
openshift-ci
Bot
added
ok-to-test
|
Scheduling required tests: |
|
@openshift-trt-agent[bot]: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
2 participants
Summary
test_namequery parameter toGET /api/component_readiness/regressionsfor efficient server-side filteringTestRegressionsTablecomponent that displays regression ID, variant combination, regressed since, last failure, and triage status (with links to triage/regression detail pages)Jira: https://issues.redhat.com/browse/TRT-2273
Screenshots
Full Test Analysis page with Active Regressions
Variant filtering applied (arm64 excluded)
Test plan
gotestsum ./pkg/...- 13773 tests)CI=true npm test)golangci-lint run ./...- 0 issues)npx eslint .)Generated with Claude Code
Summary by CodeRabbit
New Features
test_namefilter to the regressions API.Bug Fixes