diff --git a/docs/plans/trt-2633-triage-symptoms.md b/docs/plans/trt-2633-triage-symptoms.md new file mode 100644 index 0000000000..65a1cc1ab8 --- /dev/null +++ b/docs/plans/trt-2633-triage-symptoms.md @@ -0,0 +1,571 @@ +# TRT-2633: Support Linking Symptoms/Labels to Triage Records + +**Date:** 2026-04-30 +**JIRA:** [TRT-2633](https://redhat.atlassian.net/browse/TRT-2633) +**Author:** Stephen Goeddel + +## Problem Statement + +Triage can get messy in cases where a generic test is failing for lots of +different reasons. It's often hard to spot what's really going on across a set +of regressions tied to a single triage. + +Symptom/label usage is increasing as a way to automatically detect problems +hidden deep in job logs and artifacts. Tying this data into triage records would +let engineers see at a glance which symptoms are present across all regressions +in a triage — and what percentage of those regressions exhibit each one. + +## Current Architecture + +### Symptom Pipeline + +1. **Symptom definitions** live in PostgreSQL (`job_run_symptoms` table). + Each has an `ID` (string), `Summary`, `MatcherType`, `FilePattern`, + `MatchString`, and `LabelIDs` (labels to apply on match). + +2. **Symptom detection** runs against job run artifacts. When a symptom + matches, it creates entries in BigQuery's `job_labels` table with the + `symptom_id` field populated, linking the label application back to the + triggering symptom. + +3. **Regression tracking** queries BigQuery for test details, joining + `job_labels` to aggregate labels per job run: + ```sql + SELECT prowjob_build_id, + STRING_AGG(DISTINCT label, ',' ORDER BY label) AS job_labels + FROM .job_labels ... + ``` + These label IDs flow through the pipeline: + `TestJobRunRows.JobLabels` → `JobRunStats.JobLabels` → `RegressionJobRun.JobLabels` + +4. **Missing link**: Symptom IDs (`job_labels.symptom_id`) are not aggregated + or carried through the pipeline. Only label IDs are preserved. There is no + association between symptoms and triage records. + +### Triage Architecture + +- A `Triage` record has many `TestRegression` records (via `triage_regressions` join table). +- Each `TestRegression` has many `RegressionJobRun` records (via FK). +- Each `RegressionJobRun` has `JobLabels pq.StringArray` with label IDs from failed runs. +- The regression cache loader (`regressioncacheloader.go`) orchestrates regression + tracking and job run syncing in a single pass per view. + +## Proposed Solution + +### Two Core Changes + +1. **Carry symptom IDs through the pipeline**: Extend the BigQuery query to + also aggregate `symptom_id` from `job_labels`, and carry `JobSymptoms` + alongside `JobLabels` through `TestJobRunRows` → `JobRunStats` → + `RegressionJobRun`. + +2. **New `triage_symptoms` junction table**: A many-to-many association between + triages and symptoms. During regression tracking, after job runs are synced, + collect symptom IDs from job runs of regressions that belong to triages and + upsert the associations. + +### Why a Persistent Junction Table + +- Provides fast triage detail queries without joining through + regressions → job runs → symptom IDs → symptoms at request time. +- Creates a stable record of which symptoms were observed, even after + regression job runs age out or the regression closes. +- Enables future features like notifications when new symptoms appear on a triage. +- Satisfies AC #1 (a many-to-many association must exist between triage records and symptoms in the database). + +### Percentage Computation + +For AC #5 (show the percentage of regressions exhibiting each symptom on the triage details page), the percentage is computed at +API time via a `COUNT(DISTINCT regression_id)` query against the +`triage_symptoms` junction table, grouped by `symptom_id`. This is fast (single +indexed query) and avoids walking the full regression → job run → symptom graph. + +The junction table also stores a per-regression job run count (`job_run_count` column), +which is summed across regressions to produce `job_run_count` in the summary. + +`SyncTriageSymptoms` does a full recount of symptoms from each regression's job +runs and replaces `job_run_count` (not increments), making it idempotent. Since +the operation replaces the count entirely on each run, calling it multiple times +with the same data produces the same result without double-counting. + +## Implementation Plan + +### Phase 1: Carry Symptom IDs Through the Data Pipeline + +#### BigQuery query + +**File:** `pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go` + +Extend the `job_labels` aggregation subquery to also aggregate symptom IDs: + +```go +jobLabelsJoin := fmt.Sprintf(`LEFT JOIN ( + SELECT prowjob_build_id, + STRING_AGG(DISTINCT label, ',' ORDER BY label) AS job_labels, + STRING_AGG(DISTINCT CASE WHEN symptom_id != '' THEN symptom_id END, + ',' ORDER BY symptom_id) AS job_symptoms + FROM %s.job_labels + WHERE prowjob_start >= DATETIME(@From) + AND prowjob_start < DATETIME(@To) + GROUP BY prowjob_build_id +) agg_labels ON junit.prowjob_build_id = agg_labels.prowjob_build_id +`, client.Dataset) +``` + +Add `ANY_VALUE(agg_labels.job_symptoms) AS job_symptoms` to the SELECT list +alongside the existing `job_labels` column. + +#### Data structure changes + +**File:** `pkg/apis/api/componentreport/crstatus/types.go` +```go +type TestJobRunRows struct { + // ... existing fields ... + JobLabels []string `bigquery:"-" json:"job_labels,omitempty"` + JobSymptoms []string `bigquery:"-" json:"job_symptoms,omitempty"` // NEW + TestFailures int `bigquery:"-" json:"test_failures"` +} +``` + +**File:** `pkg/apis/api/componentreport/testdetails/types.go` +```go +type JobRunStats struct { + // ... existing fields ... + JobLabels []string `json:"job_labels,omitempty"` + JobSymptoms []string `json:"job_symptoms,omitempty"` // NEW + TestFailures int `json:"test_failures"` +} +``` + +**File:** `pkg/db/models/triage.go` +```go +type RegressionJobRun struct { + // ... existing fields ... + JobLabels pq.StringArray `json:"job_labels,omitempty" gorm:"column:job_labels;type:text[]"` + JobSymptoms pq.StringArray `json:"job_symptoms,omitempty" gorm:"column:job_symptoms;type:text[]"` // NEW +} +``` + +#### Deserialization + +**File:** `pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go` + +In `deserializeRowToJobRunTestReportStatus()`, add handling alongside the +existing `job_labels` case: +```go +case col == "job_symptoms": + if row[i] != nil { + cts.JobSymptoms = strings.Split(row[i].(string), ",") + } +``` + +#### Conversion through the pipeline + +**File:** `pkg/api/componentreadiness/test_details.go` — `getJobRunStats()`: +```go +JobSymptoms: stats.JobSymptoms, +``` + +**File:** `pkg/api/componentreadiness/regressiontracker.go` — `FailedJobRunsFromTestDetails()`: +```go +jobRun := models.RegressionJobRun{ + // ... existing fields ... + JobLabels: pq.StringArray(run.JobLabels), + JobSymptoms: pq.StringArray(run.JobSymptoms), // NEW +} +``` + +#### Update MergeJobRuns to refresh symptom data + +**File:** `pkg/api/componentreadiness/regressiontracker.go` + +`MergeJobRuns` uses `Assign` + `FirstOrCreate` so that `JobSymptoms` (and +`JobLabels`) are updated on existing records when newly-detected symptoms are +captured on subsequent loader runs: + +```go +func (prs *PostgresRegressionStore) MergeJobRuns(regressionID uint, jobRuns []models.RegressionJobRun) error { + for i := range jobRuns { + jobRuns[i].RegressionID = regressionID + res := prs.dbc.DB. + Where("regression_id = ? AND prow_job_run_id = ?", regressionID, jobRuns[i].ProwJobRunID). + Assign(models.RegressionJobRun{ + JobLabels: jobRuns[i].JobLabels, + JobSymptoms: jobRuns[i].JobSymptoms, + }). + FirstOrCreate(&jobRuns[i]) + if res.Error != nil { + return fmt.Errorf("error merging job run %s for regression %d: %w", + jobRuns[i].ProwJobRunID, regressionID, res.Error) + } + } + return nil +} +``` + +`Assign` tells GORM to always update the specified fields — whether the record +is found or created. This ensures that if new symptoms appear on a job run that +was already processed, they are captured on subsequent loader runs. The BigQuery +query returns the full set of symptoms each time (via `STRING_AGG(DISTINCT ...)`), +so the stored value is replaced entirely, not appended to. + +### Phase 2: Database Schema — Junction Table + +#### New model + +**File:** `pkg/db/models/triage.go` + +```go +type TriageSymptom struct { + TriageID uint `json:"triage_id" gorm:"primaryKey;column:triage_id"` + SymptomID string `json:"symptom_id" gorm:"primaryKey;column:symptom_id"` + RegressionID uint `json:"regression_id" gorm:"primaryKey;column:regression_id"` + JobRunCount int `json:"job_run_count" gorm:"column:job_run_count;not null;default:0"` +} +``` + +The composite key `(triage_id, symptom_id, regression_id)` records exactly which +regression(s) surfaced each symptom on a given triage. `JobRunCount` stores how +many failed job runs on that regression exhibited the symptom. + +Rows are upserted during the regression cache loader run via +`INSERT ... ON CONFLICT DO UPDATE SET job_run_count = EXCLUDED.job_run_count`. +Since `SyncTriageSymptoms` does a full recount and replaces the count, the +operation is idempotent. + +#### Add association to Triage model + +Since the junction table includes `regression_id`, GORM's built-in `many2many` +tag no longer fits (it expects a two-column join). Instead, `TriageSymptom` is +modeled as a standalone entity with a `foreignKey:TriageID` relationship. + +```go +type Triage struct { + // ... existing fields ... + TriageSymptoms []TriageSymptom `json:"-" gorm:"foreignKey:TriageID;constraint:OnDelete:CASCADE"` +} +``` + +The `json:"-"` tag prevents raw junction rows from leaking into API responses — +the curated `symptom_summaries` on `ExpandedTriage` is the intended API surface. + +The `OnDelete:CASCADE` GORM constraint ensures that deleting a triage +automatically removes its `TriageSymptom` junction rows. This follows the same +pattern used by `TestRegression.JobRuns` and `TestRegression.Views`. + +#### Foreign key constraints + +**File:** `pkg/db/db.go` + +In addition to GORM's auto-migrated `TriageSymptom` table, manual FK constraints +are added via `ensureTriageSymptomCascade()` to handle cascades that GORM's +`foreignKey` tag can't express (since `TriageSymptom` is not "owned" by either +`Symptom` or `TestRegression` in the GORM model graph): + +- `fk_triage_symptoms_symptom`: `symptom_id` → `job_run_symptoms.id` ON DELETE CASCADE +- `fk_triage_symptoms_regression`: `regression_id` → `test_regressions.id` ON DELETE CASCADE + +These ensure that deleting a symptom definition or a regression automatically +cleans up the associated `triage_symptoms` rows. The constraints are idempotent +(guarded by `IF NOT EXISTS` on `pg_constraint`). + +#### Auto-migration + +**File:** `pkg/db/db.go` + +`TriageSymptom` is in the auto-migrate list (creates the `triage_symptoms` table). + +### Phase 3: Automatic Linking During Regression Tracking + +#### New interface method + +**File:** `pkg/api/componentreadiness/regressiontracker.go` + +```go +type RegressionStore interface { + // ... existing methods ... + // SyncTriageSymptoms upserts symptom associations for triages based on regression job run data. + SyncTriageSymptoms(regressions []*models.TestRegression) error +} +``` + +#### Implementation + +`SyncTriageSymptoms` does a full recount of symptoms from each regression's job +runs and replaces `job_run_count` (not increments), making the operation +idempotent and safe to call on every loader run. The upsert uses raw SQL +`INSERT ... ON CONFLICT DO UPDATE` to perform each upsert in a single DB +round-trip (following the `UpsertRegressionView` pattern). + +```go +func (prs *PostgresRegressionStore) SyncTriageSymptoms(regressions []*models.TestRegression) error { + // Load regressions with Triages and JobRuns preloaded + // For each regression with triages: + // Count job runs per symptom (deduplicating within each run via sets.New) + // For each (triage, symptom) pair: + // INSERT INTO triage_symptoms ... ON CONFLICT DO UPDATE SET job_run_count = EXCLUDED.job_run_count +} +``` + +#### Integration in cache loader + +**File:** `pkg/dataloader/regressioncacheloader/regressioncacheloader.go` + +After the per-release regression closing loop, a global symptom sync step runs +unconditionally for all active regressions: + +```go +var allActiveRegs []*models.TestRegression +for _, result := range releaseResults { + for _, id := range result.activeIDs.UnsortedList() { + allActiveRegs = append(allActiveRegs, &models.TestRegression{ID: id}) + } +} +if len(allActiveRegs) > 0 { + if err := l.regressionStore.SyncTriageSymptoms(allActiveRegs); err != nil { + l.logger.WithError(err).Error("error syncing triage symptoms") + l.errs = append(l.errs, err) + } +} +``` + +This runs once per loader execution, after all views are processed. It is not +gated behind `!anyErrors` because the operation is additive (only upserts) and +idempotent — partial view errors should not block symptom linking for +regressions that were successfully processed. + +### Phase 4: API Changes + +#### New response type for symptom summaries + +**File:** `pkg/api/componentreadiness/triage.go` + +```go +type TriageSymptomSummary struct { + Symptom struct { + ID string `json:"id"` + Summary string `json:"summary"` + } `json:"symptom"` + RegressionCount int `json:"regression_count"` + TotalCount int `json:"total_count"` + Percentage float64 `json:"percentage"` + JobRunCount int `json:"job_run_count"` + RegressionIDs []uint `json:"regression_ids"` +} +``` + +The `Symptom` field is a slim struct exposing only `ID` and `Summary` — matching +rules, filters, and other internal fields from the full `jobrunscan.Symptom` +model are not exposed in the API response. + +`RegressionIDs` lists which regressions on this triage exhibit each symptom. +The frontend uses this to build a per-regression symptom map and to filter +the regression table by symptom — without walking job runs client-side. + +`GetTriageSymptomSummaries(dbc, triageID, totalRegressions)` queries the +`triage_symptoms` junction table with `COUNT(DISTINCT regression_id)` and +`SUM(job_run_count)` grouped by `symptom_id`, then loads symptom definitions +from `job_run_symptoms`, and finally queries the raw junction rows to build +the per-symptom `RegressionIDs` slice. Symptom IDs not found in +`job_run_symptoms` are silently skipped (handles stale/test data). + +#### Extend ExpandedTriage response + +**File:** `pkg/sippyserver/server.go` + +```go +type ExpandedTriage struct { + *models.Triage + RegressedTests map[string][]*componentreport.ReportTestSummary `json:"regressed_tests"` + SymptomSummaries []componentreadiness.TriageSymptomSummary `json:"symptom_summaries,omitempty"` +} +``` + +Errors from `GetTriageSymptomSummaries` return a 500 status response (not +logged and swallowed). + +#### Comma-separated `expand` parameter + +The `jsonGetTriageByID` handler parses `expand` as a comma-separated set of +field names (e.g., `?expand=regressions,symptoms`). Each field is handled +independently: + +- `symptoms` — computes symptom summaries via `GetTriageSymptomSummaries`. + This is cheap (single indexed query against the junction table). +- `regressions` — looks up regressed tests from the component report cache. + This is more expensive, so it is only computed when explicitly requested. + +Symptoms are also computed when `regressions` is requested (since the triage +detail page needs both), but `expand=symptoms` alone returns summaries +without the regressed test lookups. + +### Phase 5: Frontend Changes + +#### Triage Details Page Layout + +**File:** `sippy-ng/src/component_readiness/Triage.js` + +The current page layout is: +1. Header with action buttons (Ask Sippy, Update, Delete, etc.) +2. Metadata table (Resolved, Description, Type, Jira fields, etc.) +3. "Included Tests" section with the `TriagedRegressionTestList` DataGrid + +The triage detail fetch uses `?expand=regressions,symptoms` to request both +regressed test lookups and symptom summaries in a single call. + +Add a new **"Symptoms"** section between the metadata table and the +"Included Tests" section. This is the natural placement because symptoms +provide context for interpreting the regressions below. + +**Symptoms section structure:** + +- Section header: **"Symptoms"** with a count badge (e.g., "Symptoms (3)") + and a tooltip explaining that symptoms are synced periodically. +- If `symptom_summaries` is empty or absent, show a muted "No symptoms + detected" message — don't hide the section entirely, so users know it exists. +- If populated, render a MUI `Table` (not DataGrid — the list will be small + enough that pagination and filtering aren't needed) with these columns: + +| Column | Source field | Display | +|--------|-------------|---------| +| Symptom | `symptom.summary` | Colored `Chip` with deterministic background color based on symptom ID | +| Regressions | `regression_count` | Count with a filter `IconButton` (with `aria-label` and `aria-pressed` for accessibility) | +| Percentage | `percentage` | MUI `LinearProgress` bar with percentage label | +| Failed Runs | `job_run_count` | Plain number — total failed job runs across regressions exhibiting this symptom | + +- Table is sorted by percentage descending (already sorted by the API). + +**Component:** `sippy-ng/src/component_readiness/TriageSymptoms.js` + +A dedicated component renders the symptoms table. It receives +`symptomSummaries`, `symptomFilter`, and `setSymptomFilter` props. + +#### Filtering Regressions by Symptom + +The Regressions column includes a small MUI `IconButton` with a filter icon +(`FilterList`). Clicking it filters the "Included Tests" +`TriagedRegressionTestList` DataGrid below to show only regressions that have +that symptom. Clicking again clears the filter (toggle behavior). + +**Implementation:** + +- `Triage.js` holds a `symptomFilter` state (symptom ID or `null`). +- Clicking the filter button toggles `symptomFilter` to that symptom's ID + or back to `null`. +- When a filter is active, show a `Chip` above the regressions table indicating + the active filter (e.g., "Filtered by: ") with a clear (X) + button that resets `symptomFilter` to `null`. +- `TriagedRegressionTestList` receives `symptomFilter` and `symptomSummaries` + props. When `symptomFilter` is set, it filters rows to only regressions + whose IDs appear in the matching symptom summary's `regression_ids` array. + +#### Per-Regression Symptom Indicators (AC #6 — each regression row indicates which triage-associated symptoms it exhibits) + +**File:** `sippy-ng/src/component_readiness/TriagedRegressionTestList.js` + +A **"Symptoms"** column (flex: 10) appears after the Variants column, only +when `symptomSummaries` is provided. For each regression row, the component +builds a `regressionSymptomMap` from the `symptomSummaries` prop — iterating +each summary's `regression_ids` to map regression IDs to their symptom IDs. +Each matching symptom is rendered as a small MUI `Chip` with a truncated label +(first ~12 chars) and a `Tooltip` showing the full summary. Chips are colored +using a deterministic hash of the symptom ID for visual distinction. + +## Test Plan + +### Unit Tests: Pipeline Changes + +**File:** `pkg/api/componentreadiness/regressiontracker_test.go` + +Added to existing `TestFailedJobRunsFromTestDetails` table-driven tests: + +| Test Case | Input | Expected | +|-----------|-------|----------| +| preserves JobSymptoms | Report with `JobSymptoms: ["SymA","SymB"]` | `runs[0].JobSymptoms` matches | +| empty JobSymptoms results in nil | Report with no symptoms | `runs[0].JobSymptoms` is nil | +| mixed runs: only symptomatic run carries symptoms | One run with `["SymA"]`, one without | Only first run has symptoms, second is nil | + +### E2E Tests: MergeJobRuns Symptom Behavior + +**File:** `test/e2e/componentreadiness/regressiontracker/regressiontracker_test.go` + +Added to existing `Test_RegressionJobRuns`: + +| Subtest | Setup | Assert | +|---------|-------|--------| +| new job run with symptoms | Merge run with `JobSymptoms: ["SymA"]` | Stored run has `JobSymptoms` = `["SymA"]` | +| existing job run gains symptoms on re-merge | First merge with no symptoms, second with `["SymA"]` | Stored run updated to `["SymA"]` | + +### E2E Tests: SyncTriageSymptoms + +**File:** `test/e2e/componentreadiness/regressiontracker/regressiontracker_test.go` + +New top-level `Test_SyncTriageSymptoms` function. Tests seed `job_run_symptoms` +records for the test symptom IDs ("SymA", "SymB") via the shared +`util.SeedSymptom` helper so that the FK constraint +(`fk_triage_symptoms_symptom`) is satisfied. + +| Subtest | Setup | Assert | +|---------|-------|--------| +| links symptoms to triage | Regression + triage + job run with `["SymA","SymB"]`, call `SyncTriageSymptoms` | 2 junction rows, correct `regression_id` and `job_run_count` | +| idempotent | Call `SyncTriageSymptoms` twice with same data | Same row count, same `job_run_count` | +| count accuracy | 3 job runs: 2 with SymA, 1 without | `job_run_count` = 2 for SymA | +| count grows with new job runs | After first sync, merge additional run with SymA, re-sync | `job_run_count` increments | +| regression without triage is skipped | Regression with symptoms but no triage | Zero `triage_symptoms` rows | +| multiple symptoms per run | Job run with `["SymA","SymB"]` | Both symptoms get junction rows | + +### E2E Tests: Symptom Summaries in API Response + +**File:** `test/e2e/componentreadiness/triage/triageapi_test.go` + +Added to existing `Test_TriageAPI`. Tests seed `job_run_symptoms` records via +the shared `util.SeedSymptom` helper. + +| Subtest | Setup | Assert | +|---------|-------|--------| +| expanded triage includes symptom summaries | Regression + triage + job runs with symptoms, `SyncTriageSymptoms`, GET `?expand=regressions,symptoms` | `symptom_summaries` non-empty, correct `regression_ids`, `regression_count`, `job_run_count` | +| expand=symptoms only returns symptoms without regressed_tests | Same setup, GET `?expand=symptoms` | `symptom_summaries` present, `regressed_tests` nil | +| delete triage cascades to triage_symptoms | Create triage + symptoms, DELETE triage | Zero junction rows for that triage | + +### Shared Test Helpers + +**File:** `test/e2e/util/db.go` + +Exported helpers used by both e2e test packages: + +- `SeedSymptom(t, dbc, id, summary)` — creates a `jobrunscan.Symptom` record (idempotent via `FirstOrCreate`) +- `CleanupSymptoms(dbc, ids...)` — deletes symptom records by ID +- `CleanupTriageSymptoms(dbc)` — deletes all `triage_symptoms` rows + +## Cascade and Deletion Behavior + +| Action | Result | Mechanism | +|--------|--------|-----------| +| Delete triage | Junction rows in `triage_symptoms` removed | GORM `constraint:OnDelete:CASCADE` on `Triage.TriageSymptoms` | +| Delete symptom definition | Junction rows in `triage_symptoms` removed | Manual FK `fk_triage_symptoms_symptom` in `db.go` | +| Delete regression | Junction rows in `triage_symptoms` removed | Manual FK `fk_triage_symptoms_regression` in `db.go` | +| Close regression | No effect on junction. Symptoms remain as historical record | — | + +## Migration and Backward Compatibility + +1. **Zero impact on existing data.** The new `job_symptoms` column on + `regression_job_runs` is added via GORM AutoMigrate with NULL default. + Existing rows remain unchanged. + +2. **No retroactive backfill.** Per AC #8 (no retroactive linking of symptoms to existing triages), existing triages will not have + symptoms linked until the next regression cache loader run processes their + regressions. Symptoms accumulate naturally going forward. + +3. **BigQuery compatibility.** The `symptom_id` column already exists in the + `job_labels` BigQuery table. The query change only adds an aggregation of + this existing column. + +4. **API backward compatibility.** The `symptom_summaries` field is additive + on the `ExpandedTriage` response. Clients that don't use it are unaffected. + +## Out of Scope (Future Work) + +- Manual association of symptoms to triages via the UI. +- Retroactive linking of symptoms to existing/historical triages. +- Display of symptom data on the Test Details page. +- Compare Sample Failures integration with symptom filtering. diff --git a/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go b/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go index 530f2b16fa..bbb988c551 100644 --- a/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go +++ b/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go @@ -563,7 +563,9 @@ func buildTestDetailsQuery( withClause, commonParams := buildCRQueryCTEs(client.Dataset, junitTable, jobNameQueryPortion, jobRunAnnotationToIgnore, c.AdvancedOption.KeyTestNames) jobLabelsJoin := fmt.Sprintf(`LEFT JOIN ( - SELECT prowjob_build_id, STRING_AGG(DISTINCT label, ',' ORDER BY label) AS job_labels + SELECT prowjob_build_id, + STRING_AGG(DISTINCT label, ',' ORDER BY label) AS job_labels, + STRING_AGG(DISTINCT CASE WHEN symptom_id != '' THEN symptom_id END, ',') AS job_symptoms FROM %s.job_labels WHERE prowjob_start >= DATETIME(@From) AND prowjob_start < DATETIME(@To) @@ -597,6 +599,7 @@ func buildTestDetailsQuery( SUM(adjusted_success_val) AS success_count, SUM(adjusted_flake_count) AS flake_count, ANY_VALUE(agg_labels.job_labels) AS job_labels, + ANY_VALUE(agg_labels.job_symptoms) AS job_symptoms, ANY_VALUE(agg_failures.job_run_test_failure_count) AS job_run_test_failure_count, COALESCE(NULLIF(ANY_VALUE(lifecycle), ''), 'blocking') AS lifecycle, FROM deduped_testcases junit @@ -1094,6 +1097,10 @@ func deserializeRowToJobRunTestReportStatus(row []bigquery.Value, schema bigquer if row[i] != nil { cts.JobLabels = strings.Split(row[i].(string), ",") } + case col == "job_symptoms": + if row[i] != nil { + cts.JobSymptoms = strings.Split(row[i].(string), ",") + } case col == "job_run_test_failure_count": if row[i] != nil { cts.TestFailures = int(row[i].(int64)) diff --git a/pkg/api/componentreadiness/regressiontracker.go b/pkg/api/componentreadiness/regressiontracker.go index 9a72b82221..81a4982ea9 100644 --- a/pkg/api/componentreadiness/regressiontracker.go +++ b/pkg/api/componentreadiness/regressiontracker.go @@ -16,6 +16,7 @@ import ( "github.com/openshift/sippy/pkg/db" "github.com/openshift/sippy/pkg/db/models" log "github.com/sirupsen/logrus" + "k8s.io/apimachinery/pkg/util/sets" ) const ( @@ -41,6 +42,8 @@ type RegressionStore interface { UpsertRegressionView(regressionID uint, viewName string) error // DeactivateRolledOffViews sets active=false on regression_views rows for regressions that have rolled off a view. DeactivateRolledOffViews(regressionIDs []uint, activeViewMap map[uint][]string) error + // SyncTriageSymptoms upserts symptom associations for triages based on regression job run data. + SyncTriageSymptoms(regressions []*models.TestRegression) error } type PostgresRegressionStore struct { @@ -109,6 +112,10 @@ func (prs *PostgresRegressionStore) MergeJobRuns(regressionID uint, jobRuns []mo jobRuns[i].RegressionID = regressionID res := prs.dbc.DB. Where("regression_id = ? AND prow_job_run_id = ?", regressionID, jobRuns[i].ProwJobRunID). + Assign(models.RegressionJobRun{ + JobLabels: jobRuns[i].JobLabels, + JobSymptoms: jobRuns[i].JobSymptoms, + }). FirstOrCreate(&jobRuns[i]) if res.Error != nil { return fmt.Errorf("error merging job run %s for regression %d: %w", @@ -118,6 +125,60 @@ func (prs *PostgresRegressionStore) MergeJobRuns(regressionID uint, jobRuns []mo return nil } +// SyncTriageSymptoms upserts triage_symptoms junction rows by doing a full recount of +// symptoms across each regression's job runs. The resulting job_run_count is replaced +// (not incremented), making the operation idempotent and safe to call on every loader run. +func (prs *PostgresRegressionStore) SyncTriageSymptoms(regressions []*models.TestRegression) error { + if len(regressions) == 0 { + return nil + } + + regIDs := make([]uint, len(regressions)) + for i, r := range regressions { + regIDs[i] = r.ID + } + + var regs []models.TestRegression + res := prs.dbc.DB. + Preload("Triages"). + Preload("JobRuns"). + Where("id IN ?", regIDs). + Find(®s) + if res.Error != nil { + return fmt.Errorf("error loading regressions for symptom sync: %w", res.Error) + } + + for _, reg := range regs { + if len(reg.Triages) == 0 { + continue + } + symptomCounts := map[string]int{} + for _, jr := range reg.JobRuns { + seen := sets.New[string]() + for _, symptom := range jr.JobSymptoms { + if symptom != "" && !seen.Has(symptom) { + seen.Insert(symptom) + symptomCounts[symptom]++ + } + } + } + for _, triage := range reg.Triages { + for symptomID, count := range symptomCounts { + if err := prs.dbc.DB.Exec( + `INSERT INTO triage_symptoms (triage_id, symptom_id, regression_id, job_run_count) + VALUES (?, ?, ?, ?) + ON CONFLICT (triage_id, symptom_id, regression_id) DO UPDATE + SET job_run_count = EXCLUDED.job_run_count`, + triage.ID, symptomID, reg.ID, count).Error; err != nil { + return fmt.Errorf("error syncing symptom %s to triage %d regression %d: %w", + symptomID, triage.ID, reg.ID, err) + } + } + } + } + return nil +} + func (prs *PostgresRegressionStore) UpsertRegressionView(regressionID uint, viewName string) error { res := prs.dbc.DB.Exec( `INSERT INTO regression_views (test_regression_id, view_name, active, opened_at) @@ -359,6 +420,7 @@ func FailedJobRunsFromTestDetails(report testdetails.Report) []models.Regression StartTime: run.StartTime.In(time.UTC), TestFailures: run.TestFailures, JobLabels: pq.StringArray(run.JobLabels), + JobSymptoms: pq.StringArray(run.JobSymptoms), } jobRuns = append(jobRuns, jobRun) } diff --git a/pkg/api/componentreadiness/regressiontracker_test.go b/pkg/api/componentreadiness/regressiontracker_test.go index d833771fd8..46f3cc1c19 100644 --- a/pkg/api/componentreadiness/regressiontracker_test.go +++ b/pkg/api/componentreadiness/regressiontracker_test.go @@ -135,6 +135,92 @@ func TestFailedJobRunsFromTestDetails(t *testing.T) { }, expectedCount: 0, }, + { + name: "preserves JobSymptoms", + report: testdetails.Report{ + Analyses: []testdetails.Analysis{ + { + JobStats: []testdetails.JobStats{ + { + SampleJobName: "job-a", + SampleJobRunStats: []testdetails.JobRunStats{ + { + JobRunID: "run-1", + StartTime: startTime1, + TestStats: crtest.Stats{FailureCount: 1}, + JobSymptoms: []string{"SymA", "SymB"}, + }, + }, + }, + }, + }, + }, + }, + expectedCount: 1, + expectedRunIDs: []string{"run-1"}, + checkFunc: func(t *testing.T, runs []models.RegressionJobRun) { + assert.Equal(t, []string{"SymA", "SymB"}, []string(runs[0].JobSymptoms)) + }, + }, + { + name: "empty JobSymptoms results in nil", + report: testdetails.Report{ + Analyses: []testdetails.Analysis{ + { + JobStats: []testdetails.JobStats{ + { + SampleJobName: "job-a", + SampleJobRunStats: []testdetails.JobRunStats{ + { + JobRunID: "run-1", + StartTime: startTime1, + TestStats: crtest.Stats{FailureCount: 1}, + }, + }, + }, + }, + }, + }, + }, + expectedCount: 1, + expectedRunIDs: []string{"run-1"}, + checkFunc: func(t *testing.T, runs []models.RegressionJobRun) { + assert.Nil(t, runs[0].JobSymptoms) + }, + }, + { + name: "mixed runs: only symptomatic run carries symptoms", + report: testdetails.Report{ + Analyses: []testdetails.Analysis{ + { + JobStats: []testdetails.JobStats{ + { + SampleJobName: "job-a", + SampleJobRunStats: []testdetails.JobRunStats{ + { + JobRunID: "run-1", + StartTime: startTime1, + TestStats: crtest.Stats{FailureCount: 1}, + JobSymptoms: []string{"SymA"}, + }, + { + JobRunID: "run-2", + StartTime: startTime2, + TestStats: crtest.Stats{FailureCount: 1}, + }, + }, + }, + }, + }, + }, + }, + expectedCount: 2, + expectedRunIDs: []string{"run-1", "run-2"}, + checkFunc: func(t *testing.T, runs []models.RegressionJobRun) { + assert.Equal(t, []string{"SymA"}, []string(runs[0].JobSymptoms)) + assert.Nil(t, runs[1].JobSymptoms) + }, + }, } for _, tt := range tests { diff --git a/pkg/api/componentreadiness/test_details.go b/pkg/api/componentreadiness/test_details.go index 4dd7b767e8..61f17285dc 100644 --- a/pkg/api/componentreadiness/test_details.go +++ b/pkg/api/componentreadiness/test_details.go @@ -567,6 +567,7 @@ func (c *ComponentReportGenerator) getJobRunStats(stats crstatus.TestJobRunRows) JobRunID: stats.ProwJobRunID, StartTime: stats.StartTime, JobLabels: stats.JobLabels, + JobSymptoms: stats.JobSymptoms, TestFailures: stats.TestFailures, } return jobRunStats diff --git a/pkg/api/componentreadiness/triage.go b/pkg/api/componentreadiness/triage.go index c95116f13b..767bf7e1b1 100644 --- a/pkg/api/componentreadiness/triage.go +++ b/pkg/api/componentreadiness/triage.go @@ -10,6 +10,7 @@ import ( "net/url" "path" "slices" + "sort" "strings" "time" @@ -21,6 +22,7 @@ import ( v1 "github.com/openshift/sippy/pkg/apis/sippy/v1" "github.com/openshift/sippy/pkg/db" "github.com/openshift/sippy/pkg/db/models" + "github.com/openshift/sippy/pkg/db/models/jobrunscan" "github.com/openshift/sippy/pkg/db/query" log "github.com/sirupsen/logrus" "gorm.io/gorm" @@ -860,6 +862,90 @@ func generateTestDetailsURLFromRegression(regression *models.TestRegression, vie ) } +// TriageSymptomSummary represents a symptom found across a triage's regressions, +// with counts and percentages for the triage detail view. +type TriageSymptomSummary struct { + Symptom struct { + ID string `json:"id"` + Summary string `json:"summary"` + } `json:"symptom"` + RegressionCount int `json:"regression_count"` + TotalCount int `json:"total_count"` + Percentage float64 `json:"percentage"` + JobRunCount int `json:"job_run_count"` + RegressionIDs []uint `json:"regression_ids"` +} + +// GetTriageSymptomSummaries queries the triage_symptoms junction table to build +// per-symptom summaries for a triage detail response. +func GetTriageSymptomSummaries(dbc *db.DB, triageID uint, totalRegressions int) ([]TriageSymptomSummary, error) { + if totalRegressions == 0 { + return nil, nil + } + + type symptomCount struct { + SymptomID string `gorm:"column:symptom_id"` + RegressionCount int `gorm:"column:regression_count"` + JobRunCount int `gorm:"column:job_run_count"` + } + var counts []symptomCount + if err := dbc.DB.Model(&models.TriageSymptom{}). + Select("symptom_id, COUNT(DISTINCT regression_id) AS regression_count, SUM(job_run_count) AS job_run_count"). + Where("triage_id = ?", triageID). + Group("symptom_id"). + Order("regression_count DESC"). + Scan(&counts).Error; err != nil { + return nil, fmt.Errorf("error querying triage symptom counts: %w", err) + } + if len(counts) == 0 { + return nil, nil + } + + symptomIDs := make([]string, len(counts)) + for i, c := range counts { + symptomIDs[i] = c.SymptomID + } + var symptoms []jobrunscan.Symptom + if err := dbc.DB.Where("id IN ?", symptomIDs).Find(&symptoms).Error; err != nil { + return nil, fmt.Errorf("error loading symptoms: %w", err) + } + symptomMap := make(map[string]jobrunscan.Symptom, len(symptoms)) + for _, s := range symptoms { + symptomMap[s.ID] = s + } + + var tsRows []models.TriageSymptom + if err := dbc.DB.Where("triage_id = ?", triageID).Find(&tsRows).Error; err != nil { + return nil, fmt.Errorf("error loading triage symptom regressions: %w", err) + } + regIDsBySymptom := make(map[string][]uint) + for _, row := range tsRows { + regIDsBySymptom[row.SymptomID] = append(regIDsBySymptom[row.SymptomID], row.RegressionID) + } + + var summaries []TriageSymptomSummary + for _, c := range counts { + s, ok := symptomMap[c.SymptomID] + if !ok { + continue + } + summary := TriageSymptomSummary{ + RegressionCount: c.RegressionCount, + TotalCount: totalRegressions, + Percentage: float64(c.RegressionCount) / float64(totalRegressions) * 100, + JobRunCount: c.JobRunCount, + RegressionIDs: regIDsBySymptom[c.SymptomID], + } + summary.Symptom.ID = s.ID + summary.Symptom.Summary = s.Summary + summaries = append(summaries, summary) + } + sort.Slice(summaries, func(i, j int) bool { + return summaries[i].RegressionCount > summaries[j].RegressionCount + }) + return summaries, nil +} + // GetViewsForTriage returns the names of all active views associated with the triage's regressions. func GetViewsForTriage(triage *models.Triage) []string { if triage == nil { diff --git a/pkg/apis/api/componentreport/crstatus/types.go b/pkg/apis/api/componentreport/crstatus/types.go index 64fb48390d..8831cd5900 100644 --- a/pkg/apis/api/componentreport/crstatus/types.go +++ b/pkg/apis/api/componentreport/crstatus/types.go @@ -64,6 +64,7 @@ type TestJobRunRows struct { JiraComponent string `bigquery:"jira_component"` JiraComponentID *big.Rat `bigquery:"jira_component_id"` JobLabels []string `bigquery:"-" json:"job_labels,omitempty"` + JobSymptoms []string `bigquery:"-" json:"job_symptoms,omitempty"` TestFailures int `bigquery:"-" json:"test_failures"` Lifecycle string `bigquery:"lifecycle"` } diff --git a/pkg/apis/api/componentreport/testdetails/types.go b/pkg/apis/api/componentreport/testdetails/types.go index 1ea6ec2a50..d338194d55 100644 --- a/pkg/apis/api/componentreport/testdetails/types.go +++ b/pkg/apis/api/componentreport/testdetails/types.go @@ -114,5 +114,6 @@ type JobRunStats struct { // there are cases multiple junits are generated for the same test. TestStats crtest.Stats `json:"test_stats"` JobLabels []string `json:"job_labels,omitempty"` + JobSymptoms []string `json:"job_symptoms,omitempty"` TestFailures int `json:"test_failures"` } diff --git a/pkg/dataloader/regressioncacheloader/regressioncacheloader.go b/pkg/dataloader/regressioncacheloader/regressioncacheloader.go index 293405fe7f..35d13b58f6 100644 --- a/pkg/dataloader/regressioncacheloader/regressioncacheloader.go +++ b/pkg/dataloader/regressioncacheloader/regressioncacheloader.go @@ -156,8 +156,7 @@ func (l *RegressionCacheLoader) Load() { } } - // Close regressions per-release (only if no errors for that release), - // then resolve triages globally once all releases are processed. + // Close regressions per-release (only if no errors for that release) anyErrors := false for release, result := range releaseResults { if result.hadErrors { @@ -178,6 +177,20 @@ func (l *RegressionCacheLoader) Load() { } } + var allActiveRegs []*models.TestRegression + for _, result := range releaseResults { + for _, id := range result.activeIDs.UnsortedList() { + allActiveRegs = append(allActiveRegs, &models.TestRegression{ID: id}) + } + } + if len(allActiveRegs) > 0 { + l.logger.Infof("syncing triage symptoms for %d active regressions", len(allActiveRegs)) + if err := l.regressionStore.SyncTriageSymptoms(allActiveRegs); err != nil { + l.logger.WithError(err).Error("error syncing triage symptoms") + l.errs = append(l.errs, err) + } + } + // ResolveTriages is a global operation (not per-release), so we only run it // once after all releases have been processed, and only if no releases had errors. if !anyErrors { diff --git a/pkg/db/db.go b/pkg/db/db.go index f2b0b7babb..e77cdfb1ad 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -92,6 +92,7 @@ func (d *DB) UpdateSchema(reportEnd *time.Time) error { &models.RegressionJobRun{}, &models.RegressionView{}, &models.Triage{}, + &models.TriageSymptom{}, &models.AuditLog{}, &models.ChatRating{}, &models.ChatConversation{}, @@ -106,13 +107,11 @@ func (d *DB) UpdateSchema(reportEnd *time.Time) error { } } - // TODO(sgoeddel): This is temporary migration logic to backfill closed regressions with their most likely view. - // It should be removed after running for the first time. - if err := backfillClosedRegressionViews(d.DB); err != nil { + if err := createAuditLogIndexes(d.DB); err != nil { return err } - if err := createAuditLogIndexes(d.DB); err != nil { + if err := ensureTriageSymptomCascade(d.DB); err != nil { return err } @@ -214,29 +213,6 @@ func syncSchema(db *gorm.DB, hashType SchemaHashType, name, desiredSchema, dropS return updateRequired, nil } -// backfillClosedRegressionViews associates closed regressions that predate the regression_views -// table with their most likely view (-main). Historically only -main views had regression -// tracking enabled, so this is our best approximation. Only targets regressions with no existing -// view associations; open regressions are handled naturally by the loader. -func backfillClosedRegressionViews(db *gorm.DB) error { - res := db.Exec(` - INSERT INTO regression_views (test_regression_id, view_name, active, opened_at, closed_at) - SELECT tr.id, tr.release || '-main', false, tr.opened, tr.closed - FROM test_regressions tr - WHERE tr.closed IS NOT NULL - AND NOT EXISTS ( - SELECT 1 FROM regression_views rv WHERE rv.test_regression_id = tr.id - ) - ON CONFLICT (test_regression_id, view_name) DO NOTHING`) - if res.Error != nil { - return fmt.Errorf("error backfilling closed regression views: %w", res.Error) - } - if res.RowsAffected > 0 { - log.Infof("backfilled %d closed regressions with release-main view associations", res.RowsAffected) - } - return nil -} - // createAuditLogIndexes creates GIN indexes for JSONB columns in audit_logs table // for efficient JSON querying operations. func createAuditLogIndexes(db *gorm.DB) error { @@ -251,6 +227,42 @@ func createAuditLogIndexes(db *gorm.DB) error { return nil } +// ensureTriageSymptomCascade adds foreign keys to triage_symptoms with ON DELETE CASCADE +// so that deleting a symptom definition or a regression automatically cleans up the +// associated triage_symptoms rows. +func ensureTriageSymptomCascade(db *gorm.DB) error { + constraints := []struct { + name string + sql string + }{ + { + name: "fk_triage_symptoms_symptom", + sql: "ALTER TABLE triage_symptoms ADD CONSTRAINT fk_triage_symptoms_symptom FOREIGN KEY (symptom_id) REFERENCES job_run_symptoms(id) ON DELETE CASCADE", + }, + { + name: "fk_triage_symptoms_regression", + sql: "ALTER TABLE triage_symptoms ADD CONSTRAINT fk_triage_symptoms_regression FOREIGN KEY (regression_id) REFERENCES test_regressions(id) ON DELETE CASCADE", + }, + } + + for _, c := range constraints { + err := db.Exec(fmt.Sprintf(` + DO $$ + BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = '%s' + ) THEN + %s; + END IF; + END $$`, c.name, c.sql)).Error + if err != nil { + return err + } + } + return nil +} + func ParseGormLogLevel(logLevel string) (gormlogger.LogLevel, error) { switch logLevel { case "info": diff --git a/pkg/db/models/triage.go b/pkg/db/models/triage.go index 3881d6842c..d56e7a179c 100644 --- a/pkg/db/models/triage.go +++ b/pkg/db/models/triage.go @@ -48,6 +48,9 @@ type Triage struct { // If we could establish this, it may mean less data duplication. Regressions []TestRegression `json:"regressions" gorm:"constraint:OnDelete:CASCADE;many2many:triage_regressions;"` + // TriageSymptoms links symptoms discovered in regression job runs to this triage. + TriageSymptoms []TriageSymptom `json:"-" gorm:"foreignKey:TriageID;constraint:OnDelete:CASCADE"` + // Resolution is an important field presently set by a user indicating a claimed time this issue was resolved, // and thus all associated regressions should be fixed. // Setting this will immediately change the regressions icon to one indicate the issue is believed to @@ -259,4 +262,15 @@ type RegressionJobRun struct { TestFailed bool `json:"test_failed" gorm:"column:test_failed"` TestFailures int `json:"test_failures" gorm:"column:test_failures"` JobLabels pq.StringArray `json:"job_labels,omitempty" gorm:"column:job_labels;type:text[]"` + JobSymptoms pq.StringArray `json:"job_symptoms,omitempty" gorm:"column:job_symptoms;type:text[]"` +} + +// TriageSymptom records that a specific symptom was found on a specific regression +// belonging to a triage. JobRunCount tracks how many failed job runs on that +// regression exhibited the symptom. +type TriageSymptom struct { + TriageID uint `json:"triage_id" gorm:"primaryKey;column:triage_id"` + SymptomID string `json:"symptom_id" gorm:"primaryKey;column:symptom_id"` + RegressionID uint `json:"regression_id" gorm:"primaryKey;column:regression_id"` + JobRunCount int `json:"job_run_count" gorm:"column:job_run_count;not null;default:0"` } diff --git a/pkg/sippyserver/server.go b/pkg/sippyserver/server.go index e2e9ce593b..3165f605c3 100644 --- a/pkg/sippyserver/server.go +++ b/pkg/sippyserver/server.go @@ -1667,21 +1667,21 @@ func (s *Server) jsonGetTriages(w http.ResponseWriter, req *http.Request) { } // ExpandedTriage allows for additional information to be included in the triage response. -// Currently, this is only the associated ReportTestSummaries which are useful for linking to the test_details report. type ExpandedTriage struct { *models.Triage - // RegressedTests is a mapping of the view to the regressed_tests found there - RegressedTests map[string][]*componentreport.ReportTestSummary `json:"regressed_tests"` + RegressedTests map[string][]*componentreport.ReportTestSummary `json:"regressed_tests"` + SymptomSummaries []componentreadiness.TriageSymptomSummary `json:"symptom_summaries,omitempty"` } func (s *Server) jsonGetTriageByID(w http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) idStr := vars["id"] - var expandRegressions bool - expand := req.URL.Query().Get("expand") - if expand == "regressions" { - expandRegressions = true + expandFields := make(map[string]bool) + for _, field := range strings.Split(req.URL.Query().Get("expand"), ",") { + if f := strings.TrimSpace(field); f != "" { + expandFields[f] = true + } } triageID, err := strconv.Atoi(idStr) @@ -1699,36 +1699,47 @@ func (s *Server) jsonGetTriageByID(w http.ResponseWriter, req *http.Request) { failureResponse(w, http.StatusNotFound, "triage not found") return } - if !expandRegressions { + + if len(expandFields) == 0 { api.RespondWithJSON(http.StatusOK, w, triage) return } et := ExpandedTriage{ - Triage: triage, - RegressedTests: make(map[string][]*componentreport.ReportTestSummary), + Triage: triage, } - views := componentreadiness.GetViewsForTriage(triage) - for _, view := range views { - // Set the view in the request so that we can obtain the component report to get the regressed test(s) for display - q := req.URL.Query() - q.Set("view", view) - req.URL.RawQuery = q.Encode() - componentReport, err := s.getComponentReportFromRequest(req) + if expandFields["symptoms"] { + symptomSummaries, err := componentreadiness.GetTriageSymptomSummaries(s.db, triage.ID, len(triage.Regressions)) if err != nil { - failureResponse(w, http.StatusInternalServerError, fmt.Sprintf("unable to get component report: %v", err)) + failureResponse(w, http.StatusInternalServerError, fmt.Sprintf("error getting symptom summaries for triage %d: %v", triage.ID, err)) return } - var regressedTests []*componentreport.ReportTestSummary - for _, regression := range triage.Regressions { - regressedTest := componentreadiness.GetMatchingRegressedTestForRegression(regression, componentReport) - if regressedTest != nil { - regressedTests = append(regressedTests, regressedTest) + et.SymptomSummaries = symptomSummaries + } + + if expandFields["regressions"] { + et.RegressedTests = make(map[string][]*componentreport.ReportTestSummary) + views := componentreadiness.GetViewsForTriage(triage) + for _, view := range views { + q := req.URL.Query() + q.Set("view", view) + req.URL.RawQuery = q.Encode() + componentReport, err := s.getComponentReportFromRequest(req) + if err != nil { + failureResponse(w, http.StatusInternalServerError, fmt.Sprintf("unable to get component report: %v", err)) + return + } + var regressedTests []*componentreport.ReportTestSummary + for _, regression := range triage.Regressions { + regressedTest := componentreadiness.GetMatchingRegressedTestForRegression(regression, componentReport) + if regressedTest != nil { + regressedTests = append(regressedTests, regressedTest) + } + } + if len(regressedTests) > 0 { + et.RegressedTests[view] = regressedTests } - } - if len(regressedTests) > 0 { - et.RegressedTests[view] = regressedTests } } diff --git a/sippy-ng/src/component_readiness/CompReadyUtils.js b/sippy-ng/src/component_readiness/CompReadyUtils.js index 5fc5d18786..88a5ee14f8 100644 --- a/sippy-ng/src/component_readiness/CompReadyUtils.js +++ b/sippy-ng/src/component_readiness/CompReadyUtils.js @@ -898,6 +898,27 @@ export function generateTestDetailsReportLink( return generatedUrl } +const SYMPTOM_COLORS = [ + '#1976d2', + '#d32f2f', + '#388e3c', + '#f57c00', + '#7b1fa2', + '#0097a7', + '#c2185b', + '#455a64', + '#5d4037', + '#303f9f', +] + +export function symptomColor(symptomId) { + let hash = 0 + for (let i = 0; i < symptomId.length; i++) { + hash = (hash * 31 + symptomId.charCodeAt(i)) | 0 + } + return SYMPTOM_COLORS[Math.abs(hash) % SYMPTOM_COLORS.length] +} + // Helper function to check if triage has any regressions with status -1000 (failed fix). // allRegressedTests is a map of view name to array of regressed tests. export function hasFailedFixRegression(triage, allRegressedTests) { diff --git a/sippy-ng/src/component_readiness/Triage.js b/sippy-ng/src/component_readiness/Triage.js index cca99015c3..a15727c9c2 100644 --- a/sippy-ng/src/component_readiness/Triage.js +++ b/sippy-ng/src/component_readiness/Triage.js @@ -1,4 +1,4 @@ -import { Box, Button, Tooltip } from '@mui/material' +import { Box, Button, Chip, Tooltip } from '@mui/material' import { CheckCircle, Error as ErrorIcon } from '@mui/icons-material' import { CompReadyVarsContext } from './CompReadyVars' import { formatDateToSeconds, relativeTime } from '../helpers' @@ -16,7 +16,7 @@ import AskSippyButton from '../chat/AskSippyButton' import CompSeverityIcon from './CompSeverityIcon' import LaunderedLink from '../components/Laundry' import PropTypes from 'prop-types' -import React, { Fragment, useContext } from 'react' +import React, { Fragment, useContext, useState } from 'react' import Table from '@mui/material/Table' import TableBody from '@mui/material/TableBody' import TableCell from '@mui/material/TableCell' @@ -24,6 +24,7 @@ import TableRow from '@mui/material/TableRow' import TriageAuditLogsModal from './TriageAuditLogsModal' import TriagedRegressionTestList from './TriagedRegressionTestList' import TriagePotentialMatches from './TriagePotentialMatches' +import TriageSymptoms from './TriageSymptoms' import UpsertTriageModal from './UpsertTriageModal' export default function Triage({ id }) { @@ -34,6 +35,7 @@ export default function Triage({ id }) { const [triage, setTriage] = React.useState({}) const [message, setMessage] = React.useState('') const [isUpdated, setIsUpdated] = React.useState(false) + const [symptomFilter, setSymptomFilter] = useState(null) const capabilitiesContext = React.useContext(SippyCapabilitiesContext) const triageEnabled = capabilitiesContext.includes('write_endpoints') const localDBEnabled = capabilitiesContext.includes('local_db') @@ -51,14 +53,14 @@ export default function Triage({ id }) { let triageFetch // triage entries will only be available when there is a postgres connection if (localDBEnabled) { - triageFetch = fetch(`${getTriagesAPIUrl(id)}?expand=regressions`).then( - (response) => { - if (response.status !== 200) { - throw new Error('API server returned ' + response.status) - } - return response.json() + triageFetch = fetch( + `${getTriagesAPIUrl(id)}?expand=regressions,symptoms` + ).then((response) => { + if (response.status !== 200) { + throw new Error('API server returned ' + response.status) } - ) + return response.json() + }) } else { triageFetch = Promise.resolve({}) } @@ -353,11 +355,32 @@ export default function Triage({ id }) { +

Included Tests

+ {symptomFilter && ( + + ss.symptom.id === symptomFilter + )?.symptom.summary || symptomFilter + }`} + onDelete={() => setSymptomFilter(null)} + color="primary" + variant="outlined" + /> + + )} ) diff --git a/sippy-ng/src/component_readiness/TriageSymptoms.js b/sippy-ng/src/component_readiness/TriageSymptoms.js new file mode 100644 index 0000000000..d9520f5fc1 --- /dev/null +++ b/sippy-ng/src/component_readiness/TriageSymptoms.js @@ -0,0 +1,133 @@ +import { + Box, + Chip, + IconButton, + LinearProgress, + Tooltip, + Typography, +} from '@mui/material' +import { FilterList } from '@mui/icons-material' +import { symptomColor } from './CompReadyUtils' +import PropTypes from 'prop-types' +import React from 'react' +import Table from '@mui/material/Table' +import TableBody from '@mui/material/TableBody' +import TableCell from '@mui/material/TableCell' +import TableHead from '@mui/material/TableHead' +import TableRow from '@mui/material/TableRow' + +export default function TriageSymptoms({ + symptomSummaries, + symptomFilter, + setSymptomFilter, +}) { + return ( + <> + +

+ Symptoms + {symptomSummaries?.length > 0 && ` (${symptomSummaries.length})`} +

+
+ {!symptomSummaries || symptomSummaries.length === 0 ? ( + + No symptoms detected + + ) : ( + + + + + + Symptom + + + + + Regressions + + + + + Percentage + + + + + Failed Runs + + + + + + {symptomSummaries.map((ss) => ( + + + + + + + {ss.regression_count} + + + setSymptomFilter( + symptomFilter === ss.symptom.id + ? null + : ss.symptom.id + ) + } + color={ + symptomFilter === ss.symptom.id + ? 'primary' + : 'default' + } + > + + + + + + + + + + {ss.percentage.toFixed(1)}% + + + + {ss.job_run_count} + + ))} + +
+ )} + + ) +} + +TriageSymptoms.propTypes = { + symptomSummaries: PropTypes.array, + symptomFilter: PropTypes.string, + setSymptomFilter: PropTypes.func.isRequired, +} diff --git a/sippy-ng/src/component_readiness/TriagedRegressionTestList.js b/sippy-ng/src/component_readiness/TriagedRegressionTestList.js index 2e17453d31..60ae0d9d5a 100644 --- a/sippy-ng/src/component_readiness/TriagedRegressionTestList.js +++ b/sippy-ng/src/component_readiness/TriagedRegressionTestList.js @@ -1,10 +1,11 @@ import { applyFilterModel, shouldKeepFilterItem } from '../datagrid/filterUtils' +import { Chip, Tooltip, Typography } from '@mui/material' import { CompReadyVarsContext } from './CompReadyVars' import { DataGrid } from '@mui/x-data-grid' import { generateTestDetailsReportLink } from './CompReadyUtils' import { NumberParam, useQueryParam } from 'use-query-params' import { relativeTime, SafeJSONParam } from '../helpers' -import { Tooltip, Typography } from '@mui/material' +import { symptomColor } from './CompReadyUtils' import CompSeverityIcon from './CompSeverityIcon' import GridToolbar from '../datagrid/GridToolbar' import PropTypes from 'prop-types' @@ -139,6 +140,52 @@ export default function TriagedRegressionTestList(props) { ), }, + ...(props.symptomSummaries + ? [ + { + field: 'symptoms', + headerName: 'Symptoms', + flex: 10, + filterable: false, + sortable: false, + valueGetter: (params) => { + const symptomIds = regressionSymptomMap[params.row.id] + return symptomIds ? [...symptomIds] : [] + }, + renderCell: (params) => { + if (!params.value || params.value.length === 0) return null + return ( +
+ {params.value.map((symptomId) => { + const summary = + props.symptomSummaries?.find( + (ss) => ss.symptom.id === symptomId + )?.symptom.summary || symptomId + const label = + summary.length > 12 + ? summary.substring(0, 12) + '…' + : summary + return ( + + + + ) + })} +
+ ) + }, + }, + ] + : []), { field: 'opened', headerName: 'Regressed Since', @@ -233,10 +280,35 @@ export default function TriagedRegressionTestList(props) { : []), ] + const regressionSymptomMap = React.useMemo(() => { + const map = {} + if (props.symptomSummaries) { + for (const ss of props.symptomSummaries) { + for (const regId of ss.regression_ids || []) { + if (!map[regId]) map[regId] = new Set() + map[regId].add(ss.symptom.id) + } + } + } + return map + }, [props.symptomSummaries]) + + const symptomFilteredRegressions = React.useMemo(() => { + if (!props.symptomFilter || !props.symptomSummaries) { + return triagedRegressions + } + const match = props.symptomSummaries.find( + (ss) => ss.symptom.id === props.symptomFilter + ) + if (!match) return triagedRegressions + const matchingRegIds = new Set((match.regression_ids || []).map(Number)) + return triagedRegressions.filter((r) => matchingRegIds.has(r.id)) + }, [triagedRegressions, props.symptomFilter, props.symptomSummaries]) + // Apply client-side filtering using shared utility const filteredRegressions = React.useMemo( - () => applyFilterModel(triagedRegressions, filterModel, columns), - [triagedRegressions, filterModel, columns] + () => applyFilterModel(symptomFilteredRegressions, filterModel, columns), + [symptomFilteredRegressions, filterModel, columns] ) return ( @@ -292,4 +364,6 @@ TriagedRegressionTestList.propTypes = { allRegressedTests: PropTypes.object, filterVals: PropTypes.string, showOnLoad: PropTypes.bool, + symptomFilter: PropTypes.string, + symptomSummaries: PropTypes.array, } diff --git a/test/e2e/componentreadiness/regressiontracker/regressiontracker_test.go b/test/e2e/componentreadiness/regressiontracker/regressiontracker_test.go index 8298399f96..2dff90f1a2 100644 --- a/test/e2e/componentreadiness/regressiontracker/regressiontracker_test.go +++ b/test/e2e/componentreadiness/regressiontracker/regressiontracker_test.go @@ -15,10 +15,12 @@ import ( "github.com/openshift/sippy/pkg/apis/api/componentreport/testdetails" "github.com/openshift/sippy/pkg/db" "github.com/openshift/sippy/pkg/db/models" + "github.com/openshift/sippy/test/e2e/util" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/util/sets" ) func cleanupAllRegressions(dbc *db.DB) { @@ -395,6 +397,87 @@ func Test_RegressionJobRuns(t *testing.T) { assert.Len(t, stored, 3, "should have 3 unique job runs after dedup") }) + t.Run("new job run with symptoms", func(t *testing.T) { + defer cleanupAllRegressions(dbc) + defer cleanupJobRuns(dbc) + + reg, err := tracker.OpenRegression(view, componentreport.ReportTestSummary{ + TestComparison: testdetails.TestComparison{ + BaseStats: &testdetails.ReleaseStats{Release: "4.18"}, + }, + Identification: crtest.Identification{ + RowIdentification: crtest.RowIdentification{ + Component: "comp", + Capability: "cap", + TestName: "symptom test", + TestID: "symptomtestid", + }, + ColumnIdentification: crtest.ColumnIdentification{ + Variants: map[string]string{"a": "b"}, + }, + }, + }) + require.NoError(t, err) + + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + { + ProwJobRunID: "run-sym-1", + ProwJobName: "job-1", + TestFailed: true, + JobSymptoms: pq.StringArray{"SymA"}, + }, + }) + require.NoError(t, err) + + var stored []models.RegressionJobRun + res := dbc.DB.Where("regression_id = ? AND prow_job_run_id = ?", reg.ID, "run-sym-1").Find(&stored) + require.NoError(t, res.Error) + require.Len(t, stored, 1) + assert.Equal(t, []string{"SymA"}, []string(stored[0].JobSymptoms)) + }) + + t.Run("existing job run gains symptoms on re-merge", func(t *testing.T) { + defer cleanupAllRegressions(dbc) + defer cleanupJobRuns(dbc) + + reg, err := tracker.OpenRegression(view, componentreport.ReportTestSummary{ + TestComparison: testdetails.TestComparison{ + BaseStats: &testdetails.ReleaseStats{Release: "4.18"}, + }, + Identification: crtest.Identification{ + RowIdentification: crtest.RowIdentification{ + Component: "comp", + Capability: "cap", + TestName: "symptom update test", + TestID: "symptomuptestid", + }, + ColumnIdentification: crtest.ColumnIdentification{ + Variants: map[string]string{"a": "b"}, + }, + }, + }) + require.NoError(t, err) + + // First merge without symptoms + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "run-update-1", ProwJobName: "job-1", TestFailed: true}, + }) + require.NoError(t, err) + + var stored models.RegressionJobRun + dbc.DB.Where("regression_id = ? AND prow_job_run_id = ?", reg.ID, "run-update-1").First(&stored) + assert.Nil(t, stored.JobSymptoms) + + // Second merge with symptoms + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "run-update-1", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"SymA"}}, + }) + require.NoError(t, err) + + dbc.DB.Where("regression_id = ? AND prow_job_run_id = ?", reg.ID, "run-update-1").First(&stored) + assert.Equal(t, []string{"SymA"}, []string(stored.JobSymptoms)) + }) + t.Run("job runs deleted when regression is deleted", func(t *testing.T) { defer cleanupAllRegressions(dbc) defer cleanupJobRuns(dbc) @@ -433,6 +516,245 @@ func Test_RegressionJobRuns(t *testing.T) { }) } +func cleanupTriages(dbc *db.DB) { + dbc.DB.Exec("DELETE FROM triage_regressions WHERE 1=1") + dbc.DB.Where("1 = 1").Delete(&models.Triage{}) +} + +func Test_SyncTriageSymptoms(t *testing.T) { + dbc := util.CreateE2EPostgresConnection(t) + tracker := componentreadiness.NewPostgresRegressionStore(dbc, nil) + view := crview.View{ + Name: "4.19-main", + SampleRelease: reqopts.RelativeRelease{ + Release: reqopts.Release{Name: "4.19"}, + }, + } + + newRegSummary := func(testID string) componentreport.ReportTestSummary { + return componentreport.ReportTestSummary{ + TestComparison: testdetails.TestComparison{ + BaseStats: &testdetails.ReleaseStats{Release: "4.18"}, + }, + Identification: crtest.Identification{ + RowIdentification: crtest.RowIdentification{ + Component: "comp", + Capability: "cap", + TestName: "sync symptom test " + testID, + TestID: testID, + }, + ColumnIdentification: crtest.ColumnIdentification{ + Variants: map[string]string{"a": "b"}, + }, + }, + } + } + + util.SeedSymptom(t, dbc, "SymA", "Symptom A") + util.SeedSymptom(t, dbc, "SymB", "Symptom B") + defer util.CleanupSymptoms(dbc, "SymA", "SymB") + + cleanup := func() { + util.CleanupTriageSymptoms(dbc) + cleanupJobRuns(dbc) + cleanupTriages(dbc) + cleanupAllRegressions(dbc) + } + + dbCtx := dbc.DB.WithContext(context.WithValue(context.Background(), models.CurrentUserKey, "e2e-test")) + + t.Run("links symptoms to triage", func(t *testing.T) { + defer cleanup() + + reg, err := tracker.OpenRegression(view, newRegSummary("link-sym-1")) + require.NoError(t, err) + + triage := models.Triage{ + URL: "https://issues.redhat.com/browse/TEST-SYM-1", + Description: "symptom link test", + Type: models.TriageTypeProduct, + Regressions: []models.TestRegression{*reg}, + } + require.NoError(t, dbCtx.Create(&triage).Error) + + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "run-1", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"SymA", "SymB"}}, + }) + require.NoError(t, err) + + err = tracker.SyncTriageSymptoms([]*models.TestRegression{{ID: reg.ID}}) + require.NoError(t, err) + + var rows []models.TriageSymptom + dbc.DB.Where("triage_id = ?", triage.ID).Find(&rows) + assert.Len(t, rows, 2, "should have 2 symptom rows") + symptomIDs := sets.New[string]() + for _, row := range rows { + symptomIDs.Insert(row.SymptomID) + assert.Equal(t, reg.ID, row.RegressionID) + assert.Equal(t, 1, row.JobRunCount) + } + assert.True(t, symptomIDs.Has("SymA")) + assert.True(t, symptomIDs.Has("SymB")) + }) + + t.Run("idempotent", func(t *testing.T) { + defer cleanup() + + reg, err := tracker.OpenRegression(view, newRegSummary("idempotent-1")) + require.NoError(t, err) + + triage := models.Triage{ + URL: "https://issues.redhat.com/browse/TEST-SYM-2", + Description: "idempotent test", + Type: models.TriageTypeProduct, + Regressions: []models.TestRegression{*reg}, + } + require.NoError(t, dbCtx.Create(&triage).Error) + + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "run-1", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"SymA"}}, + }) + require.NoError(t, err) + + err = tracker.SyncTriageSymptoms([]*models.TestRegression{{ID: reg.ID}}) + require.NoError(t, err) + + var count1 int64 + dbc.DB.Model(&models.TriageSymptom{}).Where("triage_id = ?", triage.ID).Count(&count1) + + // Second sync + err = tracker.SyncTriageSymptoms([]*models.TestRegression{{ID: reg.ID}}) + require.NoError(t, err) + + var count2 int64 + dbc.DB.Model(&models.TriageSymptom{}).Where("triage_id = ?", triage.ID).Count(&count2) + assert.Equal(t, count1, count2, "row count should not change on re-sync") + + var row models.TriageSymptom + dbc.DB.Where("triage_id = ? AND symptom_id = ?", triage.ID, "SymA").First(&row) + assert.Equal(t, 1, row.JobRunCount, "job_run_count should remain the same") + }) + + t.Run("count accuracy", func(t *testing.T) { + defer cleanup() + + reg, err := tracker.OpenRegression(view, newRegSummary("count-acc-1")) + require.NoError(t, err) + + triage := models.Triage{ + URL: "https://issues.redhat.com/browse/TEST-SYM-3", + Description: "count accuracy test", + Type: models.TriageTypeProduct, + Regressions: []models.TestRegression{*reg}, + } + require.NoError(t, dbCtx.Create(&triage).Error) + + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "run-1", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"SymA"}}, + {ProwJobRunID: "run-2", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"SymA"}}, + {ProwJobRunID: "run-3", ProwJobName: "job-1", TestFailed: true}, + }) + require.NoError(t, err) + + err = tracker.SyncTriageSymptoms([]*models.TestRegression{{ID: reg.ID}}) + require.NoError(t, err) + + var row models.TriageSymptom + res := dbc.DB.Where("triage_id = ? AND symptom_id = ?", triage.ID, "SymA").First(&row) + require.NoError(t, res.Error) + assert.Equal(t, 2, row.JobRunCount, "job_run_count should be 2 (only runs with SymA)") + }) + + t.Run("count grows with new job runs", func(t *testing.T) { + defer cleanup() + + reg, err := tracker.OpenRegression(view, newRegSummary("count-grow-1")) + require.NoError(t, err) + + triage := models.Triage{ + URL: "https://issues.redhat.com/browse/TEST-SYM-4", + Description: "count grows test", + Type: models.TriageTypeProduct, + Regressions: []models.TestRegression{*reg}, + } + require.NoError(t, dbCtx.Create(&triage).Error) + + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "run-1", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"SymA"}}, + }) + require.NoError(t, err) + + err = tracker.SyncTriageSymptoms([]*models.TestRegression{{ID: reg.ID}}) + require.NoError(t, err) + + var row models.TriageSymptom + dbc.DB.Where("triage_id = ? AND symptom_id = ?", triage.ID, "SymA").First(&row) + assert.Equal(t, 1, row.JobRunCount) + + // Add another job run with the same symptom + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "run-2", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"SymA"}}, + }) + require.NoError(t, err) + + err = tracker.SyncTriageSymptoms([]*models.TestRegression{{ID: reg.ID}}) + require.NoError(t, err) + + dbc.DB.Where("triage_id = ? AND symptom_id = ?", triage.ID, "SymA").First(&row) + assert.Equal(t, 2, row.JobRunCount, "job_run_count should increment after new run") + }) + + t.Run("regression without triage is skipped", func(t *testing.T) { + defer cleanup() + + reg, err := tracker.OpenRegression(view, newRegSummary("no-triage-1")) + require.NoError(t, err) + + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "run-1", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"SymA"}}, + }) + require.NoError(t, err) + + err = tracker.SyncTriageSymptoms([]*models.TestRegression{{ID: reg.ID}}) + require.NoError(t, err) + + var count int64 + dbc.DB.Model(&models.TriageSymptom{}).Where("regression_id = ?", reg.ID).Count(&count) + assert.Equal(t, int64(0), count, "no triage_symptoms rows should exist for untriaged regression") + }) + + t.Run("multiple symptoms per run", func(t *testing.T) { + defer cleanup() + + reg, err := tracker.OpenRegression(view, newRegSummary("multi-sym-1")) + require.NoError(t, err) + + triage := models.Triage{ + URL: "https://issues.redhat.com/browse/TEST-SYM-6", + Description: "multi symptom test", + Type: models.TriageTypeProduct, + Regressions: []models.TestRegression{*reg}, + } + require.NoError(t, dbCtx.Create(&triage).Error) + + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "run-1", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"SymA", "SymB"}}, + }) + require.NoError(t, err) + + err = tracker.SyncTriageSymptoms([]*models.TestRegression{{ID: reg.ID}}) + require.NoError(t, err) + + var rows []models.TriageSymptom + dbc.DB.Where("triage_id = ?", triage.ID).Find(&rows) + assert.Len(t, rows, 2, "both symptoms should get junction rows") + for _, row := range rows { + assert.Equal(t, 1, row.JobRunCount, "each symptom seen once") + } + }) +} + func cleanupRegressionViews(dbc *db.DB) { res := dbc.DB.Where("1 = 1").Delete(&models.RegressionView{}) if res.Error != nil { diff --git a/test/e2e/componentreadiness/triage/triageapi_test.go b/test/e2e/componentreadiness/triage/triageapi_test.go index c8e8459d0d..a77526b91e 100644 --- a/test/e2e/componentreadiness/triage/triageapi_test.go +++ b/test/e2e/componentreadiness/triage/triageapi_test.go @@ -392,6 +392,139 @@ func Test_TriageAPI(t *testing.T) { assertTriageDataMatches(t, originalTriage, oldTriageData, "OldData") }) + t.Run("expanded triage includes symptom summaries", func(t *testing.T) { + defer cleanupAllTriages(dbc) + defer util.CleanupTriageSymptoms(dbc) + + symA := util.SeedSymptom(t, dbc, "e2e-sym-a", "E2E Symptom A") + defer dbc.DB.Delete(symA) + symB := util.SeedSymptom(t, dbc, "e2e-sym-b", "E2E Symptom B") + defer dbc.DB.Delete(symB) + + reg := createTestRegression(t, tracker, view, "sym-expand-test-1") + defer dbc.DB.Delete(reg) + + triage := models.Triage{ + URL: jiraBug.URL, + Type: models.TriageTypeProduct, + Regressions: []models.TestRegression{ + {ID: reg.ID}, + }, + } + var triageResp models.Triage + err := util.SippyPost("/api/component_readiness/triages", &triage, &triageResp) + require.NoError(t, err) + + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "sym-run-1", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"e2e-sym-a", "e2e-sym-b"}}, + {ProwJobRunID: "sym-run-2", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"e2e-sym-a"}}, + }) + require.NoError(t, err) + + err = tracker.SyncTriageSymptoms([]*models.TestRegression{{ID: reg.ID}}) + require.NoError(t, err) + + var et sippyserver.ExpandedTriage + err = util.SippyGet(fmt.Sprintf("/api/component_readiness/triages/%d?expand=regressions,symptoms", triageResp.ID), &et) + require.NoError(t, err) + require.NotNil(t, et.SymptomSummaries) + require.Len(t, et.SymptomSummaries, 2, "should have 2 symptom summaries") + + symMap := make(map[string]componentreadiness.TriageSymptomSummary) + for _, ss := range et.SymptomSummaries { + symMap[ss.Symptom.ID] = ss + } + require.Contains(t, symMap, "e2e-sym-a") + assert.Equal(t, 1, symMap["e2e-sym-a"].RegressionCount) + assert.Equal(t, 2, symMap["e2e-sym-a"].JobRunCount) + assert.Contains(t, symMap["e2e-sym-a"].RegressionIDs, reg.ID) + + require.Contains(t, symMap, "e2e-sym-b") + assert.Equal(t, 1, symMap["e2e-sym-b"].RegressionCount) + assert.Equal(t, 1, symMap["e2e-sym-b"].JobRunCount) + assert.Contains(t, symMap["e2e-sym-b"].RegressionIDs, reg.ID) + }) + + t.Run("expand=symptoms only returns symptoms without regressed_tests", func(t *testing.T) { + defer cleanupAllTriages(dbc) + defer util.CleanupTriageSymptoms(dbc) + + sym := util.SeedSymptom(t, dbc, "e2e-sym-only", "E2E Symptom Only") + defer dbc.DB.Delete(sym) + + reg := createTestRegression(t, tracker, view, "sym-only-test-1") + defer dbc.DB.Delete(reg) + + triage := models.Triage{ + URL: jiraBug.URL, + Type: models.TriageTypeProduct, + Regressions: []models.TestRegression{ + {ID: reg.ID}, + }, + } + var triageResp models.Triage + err := util.SippyPost("/api/component_readiness/triages", &triage, &triageResp) + require.NoError(t, err) + + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "sym-only-run-1", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"e2e-sym-only"}}, + }) + require.NoError(t, err) + + err = tracker.SyncTriageSymptoms([]*models.TestRegression{{ID: reg.ID}}) + require.NoError(t, err) + + var et sippyserver.ExpandedTriage + err = util.SippyGet(fmt.Sprintf("/api/component_readiness/triages/%d?expand=symptoms", triageResp.ID), &et) + require.NoError(t, err) + require.NotNil(t, et.SymptomSummaries) + assert.Len(t, et.SymptomSummaries, 1) + assert.Nil(t, et.RegressedTests, "regressed_tests should be nil when only symptoms is expanded") + }) + + t.Run("delete triage cascades to triage_symptoms", func(t *testing.T) { + defer cleanupAllTriages(dbc) + defer util.CleanupTriageSymptoms(dbc) + + sym := util.SeedSymptom(t, dbc, "e2e-sym-cascade", "E2E Symptom Cascade") + defer dbc.DB.Delete(sym) + + reg := createTestRegression(t, tracker, view, "sym-cascade-test-1") + defer dbc.DB.Delete(reg) + + triage := models.Triage{ + URL: jiraBug.URL, + Type: models.TriageTypeProduct, + Regressions: []models.TestRegression{ + {ID: reg.ID}, + }, + } + var triageResp models.Triage + err := util.SippyPost("/api/component_readiness/triages", &triage, &triageResp) + require.NoError(t, err) + + err = tracker.MergeJobRuns(reg.ID, []models.RegressionJobRun{ + {ProwJobRunID: "cascade-run-1", ProwJobName: "job-1", TestFailed: true, JobSymptoms: pq.StringArray{"e2e-sym-cascade"}}, + }) + require.NoError(t, err) + + err = tracker.SyncTriageSymptoms([]*models.TestRegression{{ID: reg.ID}}) + require.NoError(t, err) + + // Verify junction row exists + var count int64 + dbc.DB.Model(&models.TriageSymptom{}).Where("triage_id = ?", triageResp.ID).Count(&count) + require.Equal(t, int64(1), count, "should have 1 junction row before delete") + + // Delete via API + err = util.SippyDelete(fmt.Sprintf("/api/component_readiness/triages/%d", triageResp.ID)) + require.NoError(t, err) + + // Verify junction rows are gone + dbc.DB.Model(&models.TriageSymptom{}).Where("triage_id = ?", triageResp.ID).Count(&count) + assert.Equal(t, int64(0), count, "triage_symptoms should be cascade deleted with triage") + }) + t.Run("audit endpoint returns full lifecycle operations", func(t *testing.T) { defer cleanupAllTriages(dbc) diff --git a/test/e2e/util/db.go b/test/e2e/util/db.go index 1948233990..9ea6bc55f9 100644 --- a/test/e2e/util/db.go +++ b/test/e2e/util/db.go @@ -6,6 +6,8 @@ import ( "github.com/openshift/sippy/pkg/db" "github.com/openshift/sippy/pkg/db/models" + "github.com/openshift/sippy/pkg/db/models/jobrunscan" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" "gorm.io/gorm/logger" ) @@ -27,3 +29,30 @@ func CreateE2EPostgresConnection(t *testing.T) *db.DB { return dbc } + +func SeedSymptom(t *testing.T, dbc *db.DB, id, summary string) *jobrunscan.Symptom { + sym := &jobrunscan.Symptom{ + SymptomContent: jobrunscan.SymptomContent{ + ID: id, + Summary: summary, + MatcherType: jobrunscan.MatcherTypeString, + MatchString: "e2e-test-match", + }, + } + res := dbc.DB.Where("id = ?", id).FirstOrCreate(sym) + require.NoError(t, res.Error) + return sym +} + +func CleanupSymptoms(dbc *db.DB, ids ...string) { + for _, id := range ids { + dbc.DB.Where("id = ?", id).Delete(&jobrunscan.Symptom{}) + } +} + +func CleanupTriageSymptoms(dbc *db.DB) { + res := dbc.DB.Where("1 = 1").Delete(&models.TriageSymptom{}) + if res.Error != nil { + log.Errorf("error deleting triage symptoms: %v", res.Error) + } +}