From 077aff16ffdfe43e5a030033ef229a60e4413a5a Mon Sep 17 00:00:00 2001 From: Matthew Staebler Date: Mon, 20 Jul 2026 17:59:55 -0400 Subject: [PATCH] TRT-2741: fix duplicate rows on Tests page Remove the synthetic ID field from test API responses and structs, since it was misleading and no longer needed after the matview removal. Use a composite key (name + suite_name + variants) for DataGrid row identity in the frontend to prevent duplicate row rendering. Co-Authored-By: Claude Opus 4.6 --- pkg/api/tests.go | 11 ++--------- pkg/apis/api/types.go | 10 ++-------- pkg/db/query/cumulative_query.go | 4 ++-- pkg/db/query/test_queries.go | 6 +++--- pkg/flags/postgres_benchmarking_test.go | 4 ++-- sippy-ng/src/tests/TestTable.js | 5 +++++ 6 files changed, 16 insertions(+), 24 deletions(-) diff --git a/pkg/api/tests.go b/pkg/api/tests.go index 2095f987f..c46b83508 100644 --- a/pkg/api/tests.go +++ b/pkg/api/tests.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "math" "net/http" gosort "sort" "strconv" @@ -463,16 +462,14 @@ func (spec *TestResultsSpec) buildTestsResultsPGGenerator(ctx context.Context, d errs = append(errs, err) return } - collapsedColumns := append([]string{"id"}, append(testMetadataColumns, query.QueryTestFields)...) + collapsedColumns := append(testMetadataColumns, query.QueryTestFields) rawQuery := dbc.DB.WithContext(ctx). Table("(?) AS r", collapsedQuery). Select(strings.Join(collapsedColumns, ",")) // Collapsed path: inner query only has counts, so the outer layer computes // percentages and net improvements via QueryTestSummarizer. - selectColumns := []string{"id"} - selectColumns = append(selectColumns, testMetadataColumns...) - selectColumns = append(selectColumns, query.QueryTestSummarizer) + selectColumns := append(testMetadataColumns, query.QueryTestSummarizer) processedResults := dbc.DB.Table("(?) as results", rawQuery). Select(strings.Join(selectColumns, ",")). Where("current_runs > 0 or previous_runs > 0") @@ -538,7 +535,6 @@ func safePercent(numerator, denominator int) float64 { func computeOverallTest(testReports []apitype.Test) *apitype.Test { overall := &apitype.Test{ - ID: math.MaxInt32, Name: "Overall", } for _, t := range testReports { @@ -637,7 +633,6 @@ func (spec *TestResultsSpec) buildTestsResultsBQGenerator(ctx context.Context, b ), candidate_query AS ( SELECT - ROW_NUMBER() OVER() as id, test_id, name, jira_component, @@ -668,7 +663,6 @@ func (spec *TestResultsSpec) buildTestsResultsBQGenerator(ctx context.Context, b GROUP BY test_id, testsuite), unfiltered_candidate_query AS ( SELECT - ROW_NUMBER() OVER() as id, cm.cm_id as test_id, name, jira_component, @@ -729,7 +723,6 @@ func (spec *TestResultsSpec) buildTestsResultsBQGenerator(ctx context.Context, b } overallTest = &overallReports[0] - overallTest.ID = math.MaxInt32 overallTest.Name = "Overall" } diff --git a/pkg/apis/api/types.go b/pkg/apis/api/types.go index 380347b8c..2a5c56d8d 100644 --- a/pkg/apis/api/types.go +++ b/pkg/apis/api/types.go @@ -464,10 +464,9 @@ func (run JobRun) GetArrayValue(param string) ([]string, error) { } } -// Test contains the full accounting of a test's history, with a synthetic ID. The format +// Test contains the full accounting of a test's history. The format // of this struct is suitable for use in a data table. type Test struct { - ID int `json:"id,omitempty"` Name string `json:"name"` SuiteName string `json:"suite_name"` Variant string `json:"variant,omitempty"` @@ -542,8 +541,6 @@ func (test Test) GetStringValue(param string) (string, error) { // nolint:gocyclo func (test Test) GetNumericalValue(param string) (float64, error) { switch param { - case "id": - return float64(test.ID), nil case "current_successes": return float64(test.CurrentSuccesses), nil case "current_failures": @@ -620,10 +617,9 @@ func (test Test) GetArrayValue(param string) ([]string, error) { } } -// TestBQ contains the full accounting of a test's history, with a synthetic ID. The format +// TestBQ contains the full accounting of a test's history. The format // of this struct is suitable for use in a data table. type TestBQ struct { - ID int `json:"id,omitempty" bigquery:"id"` TestID string `json:"test_id" bigquery:"test_id"` Name string `json:"name" bigquery:"name"` SuiteName string `json:"suite_name" bigquery:"suite_name"` @@ -699,8 +695,6 @@ func (test TestBQ) GetStringValue(param string) (string, error) { // nolint:gocyclo func (test TestBQ) GetNumericalValue(param string) (float64, error) { switch param { - case "id": - return float64(test.ID), nil case "current_successes": return float64(test.CurrentSuccesses), nil case "current_failures": diff --git a/pkg/db/query/cumulative_query.go b/pkg/db/query/cumulative_query.go index a9054950b..b7c685ef0 100644 --- a/pkg/db/query/cumulative_query.go +++ b/pkg/db/query/cumulative_query.go @@ -329,7 +329,7 @@ func testReportPreAgg(dbc *db.DB, release string, sample, base DateRange, nameMa return dbc.DB. Table("(?) AS pre", core). - Select(`tests.id, tests.name, pre.suite_id, suites.name AS suite_name, + Select(`tests.name, pre.suite_id, suites.name AS suite_name, jira_components.name AS jira_component, jira_components.id AS jira_component_id, pre.current_successes, pre.current_failures, pre.current_flakes, pre.current_runs, pre.previous_successes, pre.previous_failures, pre.previous_flakes, pre.previous_runs, @@ -395,7 +395,7 @@ func TestReportQueryCollapsed(dbc *db.DB, release string, sample, base DateRange fmt.Fprintf(&buf, "\n GROUP BY tcs.test_id, tcs.suite_id, tcs.release\n) AS %s", alias) } - buf.WriteString(`SELECT t.id, t.name, su.name AS suite_name, + buf.WriteString(`SELECT t.name, su.name AS suite_name, jc.name AS jira_component, jc.id AS jira_component_id, e.release, COALESCE(e.ps_successes - COALESCE(m.ps_successes, 0), 0)::bigint AS current_successes, COALESCE(e.ps_failures - COALESCE(m.ps_failures, 0), 0)::bigint AS current_failures, diff --git a/pkg/db/query/test_queries.go b/pkg/db/query/test_queries.go index dec93a701..c6e7b0101 100644 --- a/pkg/db/query/test_queries.go +++ b/pkg/db/query/test_queries.go @@ -288,7 +288,7 @@ func UncollapsedTestReportWithStats(dbc *db.DB, release string, sample, base Dat // === filtered CTE: per-variant test report with percentages === buf.WriteString(`WITH filtered AS MATERIALIZED ( - SELECT tests.id, tests.name, pre.suite_id, suites.name AS suite_name, + SELECT pre.test_id, tests.name, pre.suite_id, suites.name AS suite_name, jira_components.name AS jira_component, jira_components.id AS jira_component_id, pre.current_successes, pre.current_failures, pre.current_flakes, pre.current_runs, pre.previous_successes, pre.previous_failures, pre.previous_flakes, pre.previous_runs, @@ -382,7 +382,7 @@ stats AS ( JOIN prow_jobs pj ON e.prow_job_id = pj.id AND pj.variant_combination_id IS NOT NULL LEFT JOIN test_cumulative_summaries m ON m.test_id = e.test_id AND m.prow_job_id = e.prow_job_id AND m.suite_id = e.suite_id AND m.release = e.release AND m.date = ? WHERE e.date = ? AND e.release = ? - AND e.test_id IN (SELECT DISTINCT id FROM %s) + AND e.test_id IN (SELECT DISTINCT test_id FROM %s) AND NOT EXISTS (SELECT 1 FROM variant_combinations WHERE 'never-stable' = any(variants) AND id = pj.variant_combination_id) GROUP BY e.test_id, e.suite_id, pj.variant_combination_id ) c @@ -403,7 +403,7 @@ SELECT f.*, COALESCE(s.flake_standard_deviation, 0) AS flake_standard_deviation, f.current_flake_percentage - COALESCE(s.flake_average, 0) AS delta_from_flake_average FROM %s -LEFT JOIN stats s ON f.id = s.test_id AND f.suite_id = s.suite_id`, resultSource) +LEFT JOIN stats s ON f.test_id = s.test_id AND f.suite_id = s.suite_id`, resultSource) return dbc.DB.Raw(buf.String(), args...), remainingFilter, nil } diff --git a/pkg/flags/postgres_benchmarking_test.go b/pkg/flags/postgres_benchmarking_test.go index 49cfb1734..cf6683b9d 100644 --- a/pkg/flags/postgres_benchmarking_test.go +++ b/pkg/flags/postgres_benchmarking_test.go @@ -925,7 +925,7 @@ func getAPIBenchmarkCases(asOf time.Time) []benchmarkCase { rawQuery = rawFilter.ToSQL(rawQuery, apitype.Test{}) processedResults := dbc.DB.Table("(?) as results", rawQuery). - Select("ROW_NUMBER() OVER() as id, suite_name, name, jira_component, jira_component_id, " + query.QueryTestSummarizer). + Select("suite_name, name, jira_component, jira_component_id, " + query.QueryTestSummarizer). Where("current_runs > 0 or previous_runs > 0") finalResults := dbc.DB.Table("(?) as final_results", processedResults) @@ -1021,7 +1021,7 @@ func Test_BenchmarkCumulativeQueryTestsReport(t *testing.T) { rawQuery = rawFilter.ToSQL(rawQuery, apitype.Test{}) processedResults := dbc.DB.Table("(?) as results", rawQuery). - Select("ROW_NUMBER() OVER() as id, suite_name, name, jira_component, jira_component_id, " + query.QueryTestSummarizer). + Select("suite_name, name, jira_component, jira_component_id, " + query.QueryTestSummarizer). Where("current_runs > 0 or previous_runs > 0") finalResults := dbc.DB.Table("(?) as final_results", processedResults) diff --git a/sippy-ng/src/tests/TestTable.js b/sippy-ng/src/tests/TestTable.js index a2f06d3f7..468c5ff52 100644 --- a/sippy-ng/src/tests/TestTable.js +++ b/sippy-ng/src/tests/TestTable.js @@ -1084,6 +1084,11 @@ function TestTable(props) { rows={rows} columns={gridView.columns} autoHeight={true} + getRowId={(row) => + `${row.name}\x1f${row.suite_name || ''}\x1f${( + row.variants || [] + ).join('\x1f')}` + } getRowHeight={() => (props.collapse ? 100 : 'auto')} disableColumnFilter={props.briefTable} pageSize={pageSize}