diff --git a/README.md b/README.md index c4042d47b0..0647adb06d 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,11 @@ See [the API documentation](pkg/api/README.md) See [the front end documentation](sippy-ng/README.md) +## Database + +See [database tuning](docs/database-tuning.md) for required PostgreSQL +parameter group settings. + ## Chat See [the chat documentation](chat/README.md) diff --git a/cmd/sippy/annotatejobruns.go b/cmd/sippy/annotatejobruns.go index 8022662676..e5d02d8d3a 100644 --- a/cmd/sippy/annotatejobruns.go +++ b/cmd/sippy/annotatejobruns.go @@ -11,6 +11,7 @@ import ( bqprovider "github.com/openshift/sippy/pkg/api/componentreadiness/dataprovider/bigquery" "github.com/openshift/sippy/pkg/apis/api/componentreport/crstatus" "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" "github.com/openshift/sippy/pkg/apis/cache" bqcachedclient "github.com/openshift/sippy/pkg/bigquery" "github.com/openshift/sippy/pkg/bigquery/bqlabel" @@ -180,7 +181,7 @@ Example run: sippy annotate-job-runs --google-service-account-credential-file=f return errors.WithMessage(err, "couldn't get DB client") } - allVariants, errs := componentreadiness.GetJobVariants(ctx, bqprovider.NewBigQueryProvider(bigQueryClient)) + allVariants, errs := componentreadiness.GetJobVariants(ctx, bqprovider.NewBigQueryProvider(bigQueryClient), reqopts.RequestOptions{}) if len(errs) > 0 { return fmt.Errorf("failed to get job variants: %v", errs) } diff --git a/cmd/sippy/automatejira.go b/cmd/sippy/automatejira.go index 64b3e084da..c14c92c469 100644 --- a/cmd/sippy/automatejira.go +++ b/cmd/sippy/automatejira.go @@ -16,6 +16,7 @@ import ( "github.com/openshift/sippy/pkg/api/componentreadiness" "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" "github.com/openshift/sippy/pkg/apis/cache" jiratype "github.com/openshift/sippy/pkg/apis/jira/v1" bqcachedclient "github.com/openshift/sippy/pkg/bigquery" @@ -168,7 +169,7 @@ func NewAutomateJiraCommand() *cobra.Command { if err != nil { return err } - allVariants, errs := componentreadiness.GetJobVariants(ctx, provider) + allVariants, errs := componentreadiness.GetJobVariants(ctx, provider, reqopts.RequestOptions{}) if len(errs) > 0 { return fmt.Errorf("failed to get job variants: %v", errs) } diff --git a/docs/database-tuning.md b/docs/database-tuning.md new file mode 100644 index 0000000000..83cdb257e5 --- /dev/null +++ b/docs/database-tuning.md @@ -0,0 +1,21 @@ +# Database Tuning + +This document is the source of truth for PostgreSQL parameter group +customizations required by Sippy. These settings must be configured in the +RDS parameter group (or equivalent server-level configuration) and are not +controlled by the application code. + +Session-level parameters set by the application on each connection are in +`pkg/db/db.go`. Per-query transaction overrides are in the query code itself +(look for `SET LOCAL`). This document covers only the server-level settings +that live outside the codebase. + +## RDS Parameter Group Settings + +Differences from the default `default.postgres14` parameter group: + +| Parameter | Value | Default | Rationale | +|-----------|-------|---------|-----------| +| `effective_io_concurrency` | 200 | 1 | Number of concurrent I/O operations the OS can handle. Set high for SSD/NVMe-backed storage (gp3, io1). | +| `max_parallel_workers` | 16 | `GREATEST({DBInstanceVCPU/2},8)` | Total parallel workers available across all concurrent queries. Component readiness queries request 4 workers each via `SET LOCAL`, and multiple queries run concurrently. Fixed to avoid dependence on instance vCPU count. | +| `random_page_cost` | 1.1 | 4.0 | Favors index scans over sequential scans, appropriate for SSD/NVMe storage. Also set per-session by the application in `pkg/db/db.go`. | diff --git a/pkg/api/cache.go b/pkg/api/cache.go index 791460af7f..cdfa6fac60 100644 --- a/pkg/api/cache.go +++ b/pkg/api/cache.go @@ -10,7 +10,6 @@ import ( "github.com/openshift/sippy/pkg/apis/cache" "github.com/openshift/sippy/pkg/util" - "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -96,10 +95,11 @@ func GetDataFromCacheOrGenerate[T any]( "type": reflect.TypeOf(defaultVal).String(), }).Infof("cache hit") var cr T - if err := json.Unmarshal(res, &cr); err != nil { - return defaultVal, []error{errors.WithMessagef(err, "failed to unmarshal cached item. cacheKey=%+v", cacheKey)} + unmarshalErr := json.Unmarshal(res, &cr) + if unmarshalErr == nil { + return cr, nil } - return cr, nil + logrus.WithError(unmarshalErr).WithField("key", string(cacheKey)).Warn("cached item failed to unmarshal, regenerating") } else if strings.Contains(err.Error(), "connection refused") { logrus.WithError(err).Fatalf("redis URL specified but got connection refused, exiting due to cost issues in this configuration") } diff --git a/pkg/api/cache_test.go b/pkg/api/cache_test.go index 335e2455e9..e3889891b7 100644 --- a/pkg/api/cache_test.go +++ b/pkg/api/cache_test.go @@ -505,6 +505,31 @@ func TestNewCacheSpec_Prefix(t *testing.T) { assert.NotContains(t, string(keyWithout), "pfx~") } +// TestGetDataFromCacheOrGenerate_MalformedCacheEntry verifies that a cached +// entry that fails to unmarshal triggers regeneration instead of returning an error. +func TestGetDataFromCacheOrGenerate_MalformedCacheEntry(t *testing.T) { + mc := newMockCache() + spec := NewCacheSpec(testCacheKey{Query: "q1"}, "prefix~", nil) + + // Pre-populate the cache with invalid JSON + cacheKey, err := spec.GetCacheKey() + require.NoError(t, err) + mc.store[string(cacheKey)] = []byte(`{not valid json`) + + var generateCalls int + expected := testResult{Value: "regenerated"} + result, errs := GetDataFromCacheOrGenerate( + context.Background(), mc, cache.RequestOptions{}, spec, + makeGenerateFn(expected, &generateCalls), testResult{}, + ) + + assert.Empty(t, errs) + assert.Equal(t, expected, result, "should return regenerated value") + assert.Equal(t, 1, generateCalls, "should call generateFn when cached data is malformed") + assert.Equal(t, 1, mc.getCalls, "should attempt cache read") + assert.Equal(t, 1, mc.setCalls, "should cache the regenerated value") +} + func timePtr(t time.Time) *time.Time { return &t } diff --git a/pkg/api/componentreadiness/component_report.go b/pkg/api/componentreadiness/component_report.go index ae67863eaa..cf2f2ca99c 100644 --- a/pkg/api/componentreadiness/component_report.go +++ b/pkg/api/componentreadiness/component_report.go @@ -2,7 +2,6 @@ package componentreadiness import ( "context" - "encoding/json" "fmt" "maps" "os" @@ -65,9 +64,8 @@ func GetComponentTestVariants(ctx context.Context, provider dataprovider.DataPro api.NewCacheSpec(generator, "TestVariants~", nil), generator.GenerateCacheVariants, CacheVariants{}) } -func GetJobVariants(ctx context.Context, provider dataprovider.DataProvider) (crtest.JobVariants, - []error) { - return provider.QueryJobVariants(ctx) +func GetJobVariants(ctx context.Context, provider dataprovider.DataProvider, reqOptions reqopts.RequestOptions) (crtest.JobVariants, []error) { + return provider.QueryJobVariants(ctx, reqOptions) } func GetComponentReport( @@ -194,7 +192,8 @@ type GeneratorCacheKey struct { AdvancedOption reqopts.Advanced TestFilters reqopts.TestFilters TestIDOptions []reqopts.TestIdentification - IncludeAllTests bool `json:"include_all_tests,omitempty"` + IncludeAllTests bool `json:"include_all_tests,omitempty"` + DataSource string `json:",omitempty"` } // GetCacheKey creates a cache key using the generator properties that we want included for uniqueness in what @@ -210,6 +209,7 @@ func (c *ComponentReportGenerator) GetCacheKey() GeneratorCacheKey { TestFilters: c.ReqOptions.TestFilters, TestIDOptions: c.ReqOptions.TestIDOptions, IncludeAllTests: c.ReqOptions.IncludeAllTests, + DataSource: c.ReqOptions.DataSource, } // TestIDOptions initialization differences caused many cache misses. This hacky bit of code attempts to handle @@ -425,29 +425,22 @@ func goInterruptible(ctx context.Context, wg *sync.WaitGroup, closure func()) { }() } -var componentAndCapabilityGetter func(test crtest.KeyWithVariants, stats crstatus.TestStatus) (string, []string) +var componentAndCapabilityGetter func(stats crstatus.TestStatus) (string, []string) -func testToComponentAndCapability(_ crtest.KeyWithVariants, stats crstatus.TestStatus) (string, []string) { +func testToComponentAndCapability(stats crstatus.TestStatus) (string, []string) { return stats.Component, stats.Capabilities } // getRowColumnIdentifications defines the rows and columns since they are variable. For rows, different pages have different row titles (component, capability etc) // Columns titles depends on the columnGroupBy parameter user requests. A particular test can belong to multiple rows of different capabilities. -func (c *ComponentReportGenerator) getRowColumnIdentifications(testIDStr string, stats crstatus.TestStatus) ([]crtest.RowIdentification, []crtest.ColumnID, error) { - var test crtest.KeyWithVariants +func (c *ComponentReportGenerator) getRowColumnIdentifications(stats crstatus.TestStatus) ([]crtest.RowIdentification, []crtest.ColumnID) { columnGroupByVariants := c.ReqOptions.VariantOption.ColumnGroupBy // We show column groups by DBGroupBy only for the last page before test details if len(c.ReqOptions.TestIDOptions) > 0 && c.ReqOptions.TestIDOptions[0].TestID != "" { columnGroupByVariants = c.ReqOptions.VariantOption.DBGroupBy } - // TODO: is this too slow? - err := json.Unmarshal([]byte(testIDStr), &test) - if err != nil { - return []crtest.RowIdentification{}, []crtest.ColumnID{}, err - } - - testComponent, testCapabilities := componentAndCapabilityGetter(test, stats) + testComponent, testCapabilities := componentAndCapabilityGetter(stats) rows := []crtest.RowIdentification{} // First Page with no component requested requestedComponent, requestedCapability, requestedTestID := "", "", "" @@ -464,9 +457,13 @@ func (c *ComponentReportGenerator) getRowColumnIdentifications(testIDStr string, } else if requestedComponent == testComponent { // A component filter was specified and this test matches that component: + if stats.TestName == "" { + return rows, nil + } + row := crtest.RowIdentification{ Component: testComponent, - TestID: test.TestID, + TestID: stats.TestID, TestName: stats.TestName, TestSuite: stats.TestSuite, } @@ -494,18 +491,14 @@ func (c *ComponentReportGenerator) getRowColumnIdentifications(testIDStr string, columns := []crtest.ColumnID{} column := crtest.ColumnIdentification{Variants: map[string]string{}} - for key, value := range test.Variants { + for key, value := range stats.Variants { if columnGroupByVariants.Has(key) { column.Variants[key] = value } } - columnKeyBytes, err := json.Marshal(column) - if err != nil { - return []crtest.RowIdentification{}, []crtest.ColumnID{}, err - } - columns = append(columns, crtest.ColumnID(columnKeyBytes)) + columns = append(columns, column.Encode()) - return rows, columns, nil + return rows, columns } type cellStatus struct { @@ -638,10 +631,7 @@ func (c *ComponentReportGenerator) generateComponentTestReport(basisStatusMap, s if !sampleThere { status = basisStatus } - testKey, err := utils.DeserializeTestKey(status, testKeyStr) - if err != nil { - return crtype.ComponentReport{}, err - } + testKey := utils.IdentificationFromStatus(status) if !sampleThere { // we use this to find tests associated with the basis that we don't see now in sample, @@ -666,20 +656,14 @@ func (c *ComponentReportGenerator) generateComponentTestReport(basisStatusMap, s } } - rowIdentifications, columnIdentifications, err := c.getRowColumnIdentifications(testKeyStr, status) - if err != nil { - return crtype.ComponentReport{}, err - } + rowIdentifications, columnIdentifications := c.getRowColumnIdentifications(status) updateCellStatus( rowIdentifications, columnIdentifications, testKey, cellReport, includeAllTests, // inputs aggregatedStatus, allRows, allColumns, // these three are maps to be updated ) } - rows, err := buildReport(sortRowIdentifications(allRows), sortColumnIdentifications(allColumns), aggregatedStatus, includeAllTests) - if err != nil { - return crtype.ComponentReport{}, err - } + rows := buildReport(sortRowIdentifications(allRows), sortColumnIdentifications(allColumns), aggregatedStatus, includeAllTests) return crtype.ComponentReport{Rows: rows}, nil } @@ -715,7 +699,7 @@ func sortColumnIdentifications(allColumns map[crtest.ColumnID]struct{}) []crtest return sortedColumns } -func buildReport(sortedRows []crtest.RowIdentification, sortedColumns []crtest.ColumnID, aggregatedStatus map[crtest.RowIdentification]map[crtest.ColumnID]cellStatus, includeAllTests bool) ([]crtype.ReportRow, error) { +func buildReport(sortedRows []crtest.RowIdentification, sortedColumns []crtest.ColumnID, aggregatedStatus map[crtest.RowIdentification]map[crtest.ColumnID]cellStatus, includeAllTests bool) []crtype.ReportRow { // Now build the report var regressionRows, goodRows []crtype.ReportRow for _, rowID := range sortedRows { @@ -729,11 +713,7 @@ func buildReport(sortedRows []crtest.RowIdentification, sortedColumns []crtest.C if reportRow.Columns == nil { reportRow.Columns = []crtype.ReportColumn{} } - var colIDStruct crtest.ColumnIdentification - err := json.Unmarshal([]byte(columnID), &colIDStruct) - if err != nil { - return nil, err - } + colIDStruct := crtest.DecodeColumnID(columnID) reportColumn := crtype.ReportColumn{ColumnIdentification: colIDStruct} status, ok := columns[columnID] if !ok { @@ -766,7 +746,7 @@ func buildReport(sortedRows []crtest.RowIdentification, sortedColumns []crtest.C } regressionRows = append(regressionRows, goodRows...) - return regressionRows, nil + return regressionRows } func getRegressionStatus(basisPassPercentage, samplePassPercentage float64) crtest.Status { @@ -938,7 +918,7 @@ func (c *ComponentReportGenerator) fischerExactTest(confidenceRequired, sampleFa func (c *ComponentReportGenerator) getUniqueJUnitColumnValuesLast60Days(ctx context.Context, field string, nested bool) ([]string, error) { - return c.dataProvider.QueryUniqueVariantValues(ctx, field, nested) + return c.dataProvider.QueryUniqueVariantValues(ctx, c.ReqOptions, field, nested) } func init() { diff --git a/pkg/api/componentreadiness/component_report_test.go b/pkg/api/componentreadiness/component_report_test.go index 32e56e85fc..fa665e155a 100644 --- a/pkg/api/componentreadiness/component_report_test.go +++ b/pkg/api/componentreadiness/component_report_test.go @@ -2,7 +2,6 @@ package componentreadiness import ( - "encoding/json" "fmt" "strings" "testing" @@ -22,7 +21,7 @@ import ( crtype "github.com/openshift/sippy/pkg/apis/api/componentreport" ) -func fakeComponentAndCapabilityGetter(test crtest.KeyWithVariants, stats crstatus.TestStatus) (string, []string) { +func fakeComponentAndCapabilityGetter(stats crstatus.TestStatus) (string, []string) { name := stats.TestName known := map[string]struct { component string @@ -158,6 +157,12 @@ var ( } ) +func withTestKey(key crtest.KeyWithVariants, status crstatus.TestStatus) crstatus.TestStatus { + status.TestID = key.TestID + status.Variants = key.Variants + return status +} + func filterColumnIDByDefault(id crtest.ColumnIdentification) crtest.ColumnIdentification { ret := crtest.ColumnIdentification{Variants: map[string]string{}} for _, variant := range strings.Split(DefaultDBGroupBy, ",") { @@ -182,10 +187,7 @@ func TestGenerateComponentReport(t *testing.T) { "Installer": "ipi", }, } - awsAMD64OVNTestBytes, err := json.Marshal(awsAMD64OVNTest) - if err != nil { - assert.NoError(t, err, "error marshalling awsAMD64OVNTest") - } + awsAMD64OVNTestKey := awsAMD64OVNTest.Encode() awsAMD64SDNTest := crtest.KeyWithVariants{ TestID: "2", Variants: map[string]string{ @@ -199,10 +201,7 @@ func TestGenerateComponentReport(t *testing.T) { "Installer": "ipi", }, } - awsAMD64SDNTestBytes, err := json.Marshal(awsAMD64SDNTest) - if err != nil { - assert.NoError(t, err, "error marshalling awsAMD64SDNTest") - } + awsAMD64SDNTestKey := awsAMD64SDNTest.Encode() awsAMD64SDNInstallerUPITest := crtest.KeyWithVariants{ TestID: "2", Variants: map[string]string{ @@ -216,10 +215,7 @@ func TestGenerateComponentReport(t *testing.T) { "Installer": "upi", }, } - awsAMD64SDNInstallerUPITestBytes, err := json.Marshal(awsAMD64SDNInstallerUPITest) - if err != nil { - assert.NoError(t, err, "error marshalling awsAMD64SDNInstallerUPITest") - } + awsAMD64SDNInstallerUPITestKey := awsAMD64SDNInstallerUPITest.Encode() awsAMD64OVN2Test := crtest.KeyWithVariants{ TestID: "3", Variants: map[string]string{ @@ -229,10 +225,7 @@ func TestGenerateComponentReport(t *testing.T) { "Upgrade": "upgrade-micro", }, } - awsAMD64OVN2TestBytes, err := json.Marshal(awsAMD64OVN2Test) - if err != nil { - assert.NoError(t, err, "error marshalling awsAMD64OVN2Test") - } + awsAMD64OVN2TestKey := awsAMD64OVN2Test.Encode() awsAMD64OVNInstallerIPITest := crtest.KeyWithVariants{ TestID: "1", Variants: map[string]string{ @@ -246,13 +239,10 @@ func TestGenerateComponentReport(t *testing.T) { "Installer": "ipi", }, } - awsAMD64OVNVariantsTestBytes, err := json.Marshal(awsAMD64OVNInstallerIPITest) - if err != nil { - assert.NoError(t, err, "error marshalling awsAMD64OVNInstallerIPITest") - } + awsAMD64OVNVariantsTestKey := awsAMD64OVNInstallerIPITest.Encode() awsAMD64OVNBaseTestStats90Percent := crstatus.TestStatus{ TestName: "test 1", - Variants: []string{"standard"}, + Variants: nil, Count: crtest.Count{ TotalCount: 1000, FlakeCount: 10, @@ -261,7 +251,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64OVNBaseTestStats50Percent := crstatus.TestStatus{ TestName: "test 1", - Variants: []string{"standard"}, + Variants: nil, Count: crtest.Count{ TotalCount: 1000, FlakeCount: 10, @@ -270,7 +260,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64OVNBaseTestStatsVariants90Percent := crstatus.TestStatus{ TestName: "test 1", - Variants: []string{"standard", "fips"}, + Variants: nil, Count: crtest.Count{ TotalCount: 1000, FlakeCount: 10, @@ -279,7 +269,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64OVNSampleTestStats90Percent := crstatus.TestStatus{ TestName: "test 1", - Variants: []string{"standard"}, + Variants: nil, Count: crtest.Count{ TotalCount: 100, FlakeCount: 1, @@ -288,7 +278,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64OVNSampleTestStats85Percent := crstatus.TestStatus{ TestName: "test 1", - Variants: []string{"standard"}, + Variants: nil, Count: crtest.Count{ TotalCount: 100, FlakeCount: 1, @@ -297,7 +287,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64OVNSampleTestStats50Percent := crstatus.TestStatus{ TestName: "test 1", - Variants: []string{"standard"}, + Variants: nil, Count: crtest.Count{ TotalCount: 100, FlakeCount: 1, @@ -306,7 +296,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64OVNSampleTestStatsTiny := crstatus.TestStatus{ TestName: "test 1", - Variants: []string{"standard"}, + Variants: nil, Count: crtest.Count{ TotalCount: 3, FlakeCount: 0, @@ -315,7 +305,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64OVNSampleTestStatsVariants90Percent := crstatus.TestStatus{ TestName: "test 1", - Variants: []string{"standard", "fips"}, + Variants: nil, Count: crtest.Count{ TotalCount: 100, FlakeCount: 1, @@ -324,7 +314,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64SDNBaseTestStats90Percent := crstatus.TestStatus{ TestName: "test 2", - Variants: []string{"standard"}, + Variants: nil, Count: crtest.Count{ TotalCount: 1000, FlakeCount: 10, @@ -333,7 +323,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64SDNBaseTestStats50Percent := crstatus.TestStatus{ TestName: "test 2", - Variants: []string{"standard"}, + Variants: nil, Count: crtest.Count{ TotalCount: 1000, FlakeCount: 10, @@ -342,7 +332,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64SDNSampleTestStats90Percent := crstatus.TestStatus{ TestName: "test 2", - Variants: []string{"standard"}, + Variants: nil, Count: crtest.Count{ TotalCount: 100, FlakeCount: 1, @@ -351,7 +341,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64OVN2BaseTestStats90Percent := crstatus.TestStatus{ TestName: "test 3", - Variants: []string{"standard"}, + Variants: nil, Count: crtest.Count{ TotalCount: 1000, FlakeCount: 10, @@ -360,7 +350,7 @@ func TestGenerateComponentReport(t *testing.T) { } awsAMD64OVN2SampleTestStats80Percent := crstatus.TestStatus{ TestName: "test 3", - Variants: []string{"standard"}, + Variants: nil, Count: crtest.Count{ TotalCount: 100, FlakeCount: 1, @@ -453,12 +443,12 @@ func TestGenerateComponentReport(t *testing.T) { name: "top page test no significant and missing data", generator: defaultComponentReportGenerator, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats90Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNSampleTestStats85Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNSampleTestStats85Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -499,14 +489,14 @@ func TestGenerateComponentReport(t *testing.T) { name: "top page test with both improvement and regression", generator: defaultComponentReportGenerator, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64OVN2TestBytes): awsAMD64OVN2BaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats50Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64OVN2TestKey: withTestKey(awsAMD64OVN2Test, awsAMD64OVN2BaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats50Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNSampleTestStats50Percent, - string(awsAMD64OVN2TestBytes): awsAMD64OVN2SampleTestStats80Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNSampleTestStats50Percent), + awsAMD64OVN2TestKey: withTestKey(awsAMD64OVN2Test, awsAMD64OVN2SampleTestStats80Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -629,12 +619,12 @@ func TestGenerateComponentReport(t *testing.T) { name: "component page test no significant and missing data", generator: componentPageGenerator, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats90Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNSampleTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNSampleTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -671,12 +661,12 @@ func TestGenerateComponentReport(t *testing.T) { name: "component page test with both improvement and regression", generator: componentPageGenerator, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats50Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats50Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats50Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats50Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -713,12 +703,12 @@ func TestGenerateComponentReport(t *testing.T) { name: "capability page test no significant and missing data", generator: capabilityPageGenerator, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats90Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNSampleTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNSampleTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -742,12 +732,12 @@ func TestGenerateComponentReport(t *testing.T) { name: "capability page test with both improvement and regression", generator: capabilityPageGenerator, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats50Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats50Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNSampleTestStats50Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNSampleTestStats50Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -771,12 +761,12 @@ func TestGenerateComponentReport(t *testing.T) { name: "test page test no significant and missing data", generator: testPageGenerator, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats90Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNSampleTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNSampleTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -800,12 +790,12 @@ func TestGenerateComponentReport(t *testing.T) { name: "test page test with both improvement and regression", generator: testPageGenerator, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats50Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats50Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNSampleTestStats50Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNSampleTestStats50Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -840,12 +830,12 @@ func TestGenerateComponentReport(t *testing.T) { }, }, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats90Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNSampleTestStats85Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNSampleTestStats85Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -937,12 +927,12 @@ func TestGenerateComponentReport(t *testing.T) { }, }, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats90Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNSampleTestStats85Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNSampleTestStats85Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -979,12 +969,12 @@ func TestGenerateComponentReport(t *testing.T) { name: "top page test minimum failure no regression", generator: defaultComponentReportGenerator, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats90Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNSampleTestStatsTiny, - string(awsAMD64SDNTestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNSampleTestStatsTiny), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -1021,12 +1011,12 @@ func TestGenerateComponentReport(t *testing.T) { name: "top page test group by installer", generator: groupByInstallerComponentReportGenerator, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNVariantsTestBytes): awsAMD64OVNBaseTestStatsVariants90Percent, - string(awsAMD64SDNInstallerUPITestBytes): awsAMD64SDNBaseTestStats90Percent, + awsAMD64OVNVariantsTestKey: withTestKey(awsAMD64OVNInstallerIPITest, awsAMD64OVNBaseTestStatsVariants90Percent), + awsAMD64SDNInstallerUPITestKey: withTestKey(awsAMD64SDNInstallerUPITest, awsAMD64SDNBaseTestStats90Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNVariantsTestBytes): awsAMD64OVNSampleTestStatsVariants90Percent, - string(awsAMD64SDNInstallerUPITestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNVariantsTestKey: withTestKey(awsAMD64OVNInstallerIPITest, awsAMD64OVNSampleTestStatsVariants90Percent), + awsAMD64SDNInstallerUPITestKey: withTestKey(awsAMD64SDNInstallerUPITest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ @@ -1067,14 +1057,14 @@ func TestGenerateComponentReport(t *testing.T) { name: "top page test with both improvement and regression flake as failure", generator: flakeFailComponentReportGenerator, baseStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNBaseTestStats90Percent, - string(awsAMD64OVN2TestBytes): awsAMD64OVN2BaseTestStats90Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNBaseTestStats50Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNBaseTestStats90Percent), + awsAMD64OVN2TestKey: withTestKey(awsAMD64OVN2Test, awsAMD64OVN2BaseTestStats90Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNBaseTestStats50Percent), }, sampleStatus: map[string]crstatus.TestStatus{ - string(awsAMD64OVNTestBytes): awsAMD64OVNSampleTestStats50Percent, - string(awsAMD64OVN2TestBytes): awsAMD64OVN2SampleTestStats80Percent, - string(awsAMD64SDNTestBytes): awsAMD64SDNSampleTestStats90Percent, + awsAMD64OVNTestKey: withTestKey(awsAMD64OVNTest, awsAMD64OVNSampleTestStats50Percent), + awsAMD64OVN2TestKey: withTestKey(awsAMD64OVN2Test, awsAMD64OVN2SampleTestStats80Percent), + awsAMD64SDNTestKey: withTestKey(awsAMD64SDNTest, awsAMD64SDNSampleTestStats90Percent), }, expectedReport: crtype.ComponentReport{ Rows: []crtype.ReportRow{ diff --git a/pkg/api/componentreadiness/dataprovider/bigquery/provider.go b/pkg/api/componentreadiness/dataprovider/bigquery/provider.go index 73ff3b5190..76e12336a8 100644 --- a/pkg/api/componentreadiness/dataprovider/bigquery/provider.go +++ b/pkg/api/componentreadiness/dataprovider/bigquery/provider.go @@ -51,7 +51,7 @@ func (p *BigQueryProvider) Cache() apiCache.Cache { // --- TestStatusQuerier --- func (p *BigQueryProvider) QueryBaseTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions) (map[string]crstatus.TestStatus, []error) { - allJobVariants, errs := p.QueryJobVariants(ctx) + allJobVariants, errs := p.QueryJobVariants(ctx, reqOptions) if len(errs) > 0 { return nil, errs } @@ -70,7 +70,7 @@ func (p *BigQueryProvider) QueryBaseTestStatus(ctx context.Context, reqOptions r func (p *BigQueryProvider) QuerySampleTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions, includeVariants map[string][]string, start, end time.Time) (map[string]crstatus.TestStatus, []error) { - allJobVariants, errs := p.QueryJobVariants(ctx) + allJobVariants, errs := p.QueryJobVariants(ctx, reqOptions) if len(errs) > 0 { return nil, errs } @@ -89,7 +89,7 @@ func (p *BigQueryProvider) QuerySampleTestStatus(ctx context.Context, reqOptions // --- TestDetailsQuerier --- func (p *BigQueryProvider) QueryBaseJobRunTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions) (map[string][]crstatus.TestJobRunRows, []error) { - allJobVariants, errs := p.QueryJobVariants(ctx) + allJobVariants, errs := p.QueryJobVariants(ctx, reqOptions) if len(errs) > 0 { return nil, errs } @@ -113,7 +113,7 @@ func (p *BigQueryProvider) QueryBaseJobRunTestStatus(ctx context.Context, reqOpt func (p *BigQueryProvider) QuerySampleJobRunTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions, includeVariants map[string][]string, start, end time.Time) (map[string][]crstatus.TestJobRunRows, []error) { - allJobVariants, errs := p.QueryJobVariants(ctx) + allJobVariants, errs := p.QueryJobVariants(ctx, reqOptions) if len(errs) > 0 { return nil, errs } @@ -135,7 +135,7 @@ type jobVariantsCacheKey struct { Dataset string } -func (p *BigQueryProvider) QueryJobVariants(ctx context.Context) (crtest.JobVariants, []error) { +func (p *BigQueryProvider) QueryJobVariants(ctx context.Context, _ reqopts.RequestOptions) (crtest.JobVariants, []error) { return apiPkg.GetDataFromCacheOrGenerate[crtest.JobVariants]( ctx, p.client.Cache, apiCache.RequestOptions{}, apiPkg.NewCacheSpec(jobVariantsCacheKey{Dataset: p.client.Dataset}, "BQJobVariants~", nil), @@ -202,7 +202,7 @@ func (p *BigQueryProvider) QueryReleases(ctx context.Context) ([]v1.Release, err return apiPkg.GetReleasesFromBigQuery(ctx, p.client) } -func (p *BigQueryProvider) QueryUniqueVariantValues(ctx context.Context, field string, nested bool) ([]string, error) { +func (p *BigQueryProvider) QueryUniqueVariantValues(ctx context.Context, _ reqopts.RequestOptions, field string, nested bool) ([]string, error) { unnest := "" if nested { unnest = fmt.Sprintf(", UNNEST(%s) nested", field) @@ -226,7 +226,7 @@ func (p *BigQueryProvider) QueryUniqueVariantValues(ctx context.Context, field s func (p *BigQueryProvider) QueryJobRuns(ctx context.Context, reqOptions reqopts.RequestOptions, release string, start, end time.Time) (map[string]dataprovider.JobRunStats, error) { - allJobVariants, errs := p.QueryJobVariants(ctx) + allJobVariants, errs := p.QueryJobVariants(ctx, reqOptions) if len(errs) > 0 { return nil, fmt.Errorf("fetching job variants: %w", errors.Join(errs...)) } @@ -316,7 +316,7 @@ func (p *BigQueryProvider) QueryJobRuns(ctx context.Context, reqOptions reqopts. return results, nil } -func (p *BigQueryProvider) QueryJobVariantValues(ctx context.Context, jobNames []string, +func (p *BigQueryProvider) QueryJobVariantValues(ctx context.Context, _ reqopts.RequestOptions, jobNames []string, variantKeys []string) (map[string]map[string]string, error) { if len(jobNames) == 0 { return map[string]map[string]string{}, nil @@ -364,7 +364,7 @@ func (p *BigQueryProvider) QueryJobVariantValues(ctx context.Context, jobNames [ return results, nil } -func (p *BigQueryProvider) LookupJobVariants(ctx context.Context, jobName string) (map[string]string, error) { +func (p *BigQueryProvider) LookupJobVariants(ctx context.Context, _ reqopts.RequestOptions, jobName string) (map[string]string, error) { queryString := fmt.Sprintf(` SELECT variant_name, variant_value FROM %s.job_variants diff --git a/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go b/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go index 0fdbbbdecd..e0897439de 100644 --- a/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go +++ b/pkg/api/componentreadiness/dataprovider/bigquery/querygenerators.go @@ -846,7 +846,9 @@ func deserializeRowToTestStatus(row []bigquery.Value, schema bigquery.Schema) (s } } - return tid.KeyOrDie(), cts, nil + cts.TestID = tid.TestID + cts.Variants = tid.Variants + return tid.Encode(), cts, nil } // sortedKeys is a helper that sorts the keys of a variant group map for consistent ordering. @@ -1125,7 +1127,7 @@ func deserializeRowToJobRunTestReportStatus(row []bigquery.Value, schema bigquer } // Serialize the test key once only so we don't have to keep recalculating - cts.TestKeyStr = cts.TestKey.KeyOrDie() + cts.TestKeyStr = cts.TestKey.Encode() return cts, nil } diff --git a/pkg/api/componentreadiness/dataprovider/interface.go b/pkg/api/componentreadiness/dataprovider/interface.go index 3babe3c860..53c916f206 100644 --- a/pkg/api/componentreadiness/dataprovider/interface.go +++ b/pkg/api/componentreadiness/dataprovider/interface.go @@ -34,7 +34,7 @@ type TestDetailsQuerier interface { // MetadataQuerier fetches reference data used to configure and parameterize reports. type MetadataQuerier interface { // QueryJobVariants returns all variant names and their possible values. - QueryJobVariants(ctx context.Context) (crtest.JobVariants, []error) + QueryJobVariants(ctx context.Context, reqOptions reqopts.RequestOptions) (crtest.JobVariants, []error) // QueryReleaseDates returns the time ranges for each known release. QueryReleaseDates(ctx context.Context, reqOptions reqopts.RequestOptions) ([]crtest.ReleaseTimeRange, []error) @@ -44,7 +44,7 @@ type MetadataQuerier interface { // QueryUniqueVariantValues returns distinct values for a variant column // from the past 60 days. - QueryUniqueVariantValues(ctx context.Context, field string, nested bool) ([]string, error) + QueryUniqueVariantValues(ctx context.Context, reqOptions reqopts.RequestOptions, field string, nested bool) ([]string, error) } // JobQuerier fetches job-level data for the view-jobs and diagnose endpoints. @@ -54,11 +54,11 @@ type JobQuerier interface { release string, start, end time.Time) (map[string]JobRunStats, error) // QueryJobVariantValues returns variant key/value pairs for the given jobs. - QueryJobVariantValues(ctx context.Context, jobNames []string, + QueryJobVariantValues(ctx context.Context, reqOptions reqopts.RequestOptions, jobNames []string, variantKeys []string) (map[string]map[string]string, error) // LookupJobVariants returns all variant key/value pairs for a single job. - LookupJobVariants(ctx context.Context, jobName string) (map[string]string, error) + LookupJobVariants(ctx context.Context, reqOptions reqopts.RequestOptions, jobName string) (map[string]string, error) } // DataProvider combines all query capabilities needed by Component Readiness. diff --git a/pkg/api/componentreadiness/dataprovider/mixed/provider.go b/pkg/api/componentreadiness/dataprovider/mixed/provider.go index 64377cc485..ece43f927f 100644 --- a/pkg/api/componentreadiness/dataprovider/mixed/provider.go +++ b/pkg/api/componentreadiness/dataprovider/mixed/provider.go @@ -20,6 +20,7 @@ var _ dataprovider.DataProvider = &MixedProvider{} // MixedProvider wraps both a BigQuery and PostgreSQL provider, routing // release metadata queries to PostgreSQL and everything else to BigQuery. +// When reqOptions.DataSource is DataSourcePostgres, test status queries route to PG instead. type MixedProvider struct { bq *bigquery.BigQueryProvider pg *postgres.PostgresProvider @@ -32,6 +33,13 @@ func NewMixedProvider(bqClient *bqcachedclient.Client, dbc *db.DB, cacheClient c } } +func (p *MixedProvider) providerFor(reqOptions reqopts.RequestOptions) dataprovider.DataProvider { + if reqOptions.DataSource == reqopts.DataSourcePostgres { + return p.pg + } + return p.bq +} + func (p *MixedProvider) Cache() cache.Cache { return p.bq.Cache() } @@ -44,38 +52,38 @@ func (p *MixedProvider) QueryReleaseDates(ctx context.Context, reqOptions reqopt return p.pg.QueryReleaseDates(ctx, reqOptions) } -func (p *MixedProvider) QueryJobVariants(ctx context.Context) (crtest.JobVariants, []error) { - return p.bq.QueryJobVariants(ctx) +func (p *MixedProvider) QueryJobVariants(ctx context.Context, reqOptions reqopts.RequestOptions) (crtest.JobVariants, []error) { + return p.providerFor(reqOptions).QueryJobVariants(ctx, reqOptions) } -func (p *MixedProvider) QueryUniqueVariantValues(ctx context.Context, field string, nested bool) ([]string, error) { - return p.bq.QueryUniqueVariantValues(ctx, field, nested) +func (p *MixedProvider) QueryUniqueVariantValues(ctx context.Context, reqOptions reqopts.RequestOptions, field string, nested bool) ([]string, error) { + return p.providerFor(reqOptions).QueryUniqueVariantValues(ctx, reqOptions, field, nested) } func (p *MixedProvider) QueryBaseTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions) (map[string]crstatus.TestStatus, []error) { - return p.bq.QueryBaseTestStatus(ctx, reqOptions) + return p.providerFor(reqOptions).QueryBaseTestStatus(ctx, reqOptions) } func (p *MixedProvider) QuerySampleTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions, includeVariants map[string][]string, start, end time.Time) (map[string]crstatus.TestStatus, []error) { - return p.bq.QuerySampleTestStatus(ctx, reqOptions, includeVariants, start, end) + return p.providerFor(reqOptions).QuerySampleTestStatus(ctx, reqOptions, includeVariants, start, end) } func (p *MixedProvider) QueryBaseJobRunTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions) (map[string][]crstatus.TestJobRunRows, []error) { - return p.bq.QueryBaseJobRunTestStatus(ctx, reqOptions) + return p.providerFor(reqOptions).QueryBaseJobRunTestStatus(ctx, reqOptions) } func (p *MixedProvider) QuerySampleJobRunTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions, includeVariants map[string][]string, start, end time.Time) (map[string][]crstatus.TestJobRunRows, []error) { - return p.bq.QuerySampleJobRunTestStatus(ctx, reqOptions, includeVariants, start, end) + return p.providerFor(reqOptions).QuerySampleJobRunTestStatus(ctx, reqOptions, includeVariants, start, end) } func (p *MixedProvider) QueryJobRuns(ctx context.Context, reqOptions reqopts.RequestOptions, release string, start, end time.Time) (map[string]dataprovider.JobRunStats, error) { - return p.bq.QueryJobRuns(ctx, reqOptions, release, start, end) + return p.providerFor(reqOptions).QueryJobRuns(ctx, reqOptions, release, start, end) } -func (p *MixedProvider) QueryJobVariantValues(ctx context.Context, jobNames, variantKeys []string) (map[string]map[string]string, error) { - return p.bq.QueryJobVariantValues(ctx, jobNames, variantKeys) +func (p *MixedProvider) QueryJobVariantValues(ctx context.Context, reqOptions reqopts.RequestOptions, jobNames, variantKeys []string) (map[string]map[string]string, error) { + return p.providerFor(reqOptions).QueryJobVariantValues(ctx, reqOptions, jobNames, variantKeys) } -func (p *MixedProvider) LookupJobVariants(ctx context.Context, jobName string) (map[string]string, error) { - return p.bq.LookupJobVariants(ctx, jobName) +func (p *MixedProvider) LookupJobVariants(ctx context.Context, reqOptions reqopts.RequestOptions, jobName string) (map[string]string, error) { + return p.providerFor(reqOptions).LookupJobVariants(ctx, reqOptions, jobName) } diff --git a/pkg/api/componentreadiness/dataprovider/postgres/cr_queries.go b/pkg/api/componentreadiness/dataprovider/postgres/cr_queries.go new file mode 100644 index 0000000000..f777f9b8d7 --- /dev/null +++ b/pkg/api/componentreadiness/dataprovider/postgres/cr_queries.go @@ -0,0 +1,480 @@ +package postgres + +import ( + "context" + "database/sql" + "fmt" + "sync" + + "github.com/lib/pq" + "gorm.io/gorm" + "k8s.io/apimachinery/pkg/util/sets" + + log "github.com/sirupsen/logrus" + + "github.com/openshift/sippy/pkg/apis/api/componentreport/crstatus" + "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + sippyv1 "github.com/openshift/sippy/pkg/apis/sippyprocessing/v1" + "github.com/openshift/sippy/pkg/db" + "github.com/openshift/sippy/pkg/db/query" +) + +// variantQuerySetup holds the shared prologue results used by both the +// prefix-sum and GA query paths. +type variantQuerySetup struct { + groupMapping variantGroupMapping + filterArgs []any + variantSubquery string + minimumFailure int +} + +func prepareVariantQuery( + ctx context.Context, + dbc *db.DB, + includeVariants map[string][]string, + dbGroupBy sets.Set[string], + minimumFailure int, +) (*variantQuerySetup, error) { + if includeVariants == nil { + includeVariants = map[string][]string{} + } + + variantLookup, err := lookupVariantValues(ctx, dbc, includeVariants, dbGroupBy) + if err != nil { + return nil, err + } + if len(variantLookup) == 0 { + return nil, nil + } + + groupMapping := buildVariantGroupMapping(variantLookup) + + filterClause, filterArgs := buildVariantFilterClause(includeVariants) + + variantSubquery := "SELECT vc.id FROM variant_combinations vc" + if filterClause != "" { + variantSubquery += " WHERE " + filterClause + } + + return &variantQuerySetup{ + groupMapping: groupMapping, + filterArgs: filterArgs, + variantSubquery: variantSubquery, + minimumFailure: minimumFailure, + }, nil +} + +// lastFailureLateral wraps a failure aggregation subquery with a LATERAL join +// that finds the most recent failure timestamp for each (test_id, suite_id). +// The LATERAL is not scoped to the variant group or includeVariants filter, +// so the timestamp may come from a run outside the reported cell. This matches +// the documented BQ/PG parity gap (see PR description for known gaps). +// Parameters after the inner query args: release, rangeStart, rangeEnd. +const lastFailureLateral = ` + SELECT fi.test_id, fi.suite_id, fi.variant_group_id, + fi.total_count, fi.success_count, fi.flake_count, + lf.last_failure + FROM (%s) fi + LEFT JOIN LATERAL ( + SELECT MAX(pjrt.prow_job_run_timestamp) AS last_failure + FROM prow_job_run_tests pjrt + WHERE pjrt.test_id = fi.test_id + AND (pjrt.suite_id = fi.suite_id OR (fi.suite_id = 0 AND pjrt.suite_id IS NULL)) + AND pjrt.prow_job_run_release = ? + AND pjrt.prow_job_run_timestamp >= ? AND pjrt.prow_job_run_timestamp < ? + AND pjrt.status = %d + ) lf ON true` + +// drilldownFilters holds optional SQL WHERE fragments for TestID and +// Capability filtering when drilling down to a specific test + environment. +type drilldownFilters struct { + // innerClause filters on test_id via subquery (for the inner aggregation) + innerClause string + innerArgs []any + // outerClause filters on tow.unique_id and tow.capabilities (for outerQuery and placeholder) + outerClause string + outerArgs []any +} + +// buildDrilldownFilters returns SQL filter fragments for TestID and Capability +// from reqOptions.TestIDOptions[0], matching the BQ provider's behavior in +// BuildComponentReportQuery. The tableAlias is the alias of the table that +// has test_id in the inner aggregation query ("e" for prefix-sum, "raw" for GA). +func buildDrilldownFilters(reqOptions reqopts.RequestOptions) drilldownFilters { + if len(reqOptions.TestIDOptions) != 1 { + return drilldownFilters{} + } + tid := reqOptions.TestIDOptions[0] + var f drilldownFilters + + if tid.TestID != "" { + f.innerClause = " AND e.test_id IN (SELECT test_id FROM test_ownerships WHERE unique_id = ?)" + f.innerArgs = []any{tid.TestID} + f.outerClause += " AND tow.unique_id = ?" + f.outerArgs = append(f.outerArgs, tid.TestID) + } + + if tid.Capability != "" { + f.outerClause += " AND ? = ANY(tow.capabilities)" + f.outerArgs = append(f.outerArgs, tid.Capability) + } + + return f +} + +// testStatusSpec parameterizes the shared query structure used by both the +// prefix-sum and GA query paths. The two paths differ only in their source +// table, aggregation expressions, and date/window filter. +type testStatusSpec struct { + fromTemplate string // FROM clause template with two %s for variantSubquery and groupMapping + preJoinArgs []any // args bound in the FROM clause before filterArgs (e.g. lookupStart) + totalExpr string // SQL expression for total runs + successExpr string // SQL expression for successes + flakeExpr string // SQL expression for flakes + whereFilter string // WHERE fragment like "e.release = ? AND e.date = ?" + whereArgs []any // args for whereFilter + release string // for LATERAL join on prow_job_run_tests + dateRange query.DateRange +} + +// queryTestStatus builds and executes the failure + placeholder query pair +// that both queryTestStatusPrefixSum and queryBaseTestStatusGA share. +// +// Two queries run in parallel: +// - Failure query: tests with >= MinimumFailure failures (regression candidates), +// wrapped in a LATERAL join to find the most recent failure timestamp +// - Placeholder query: (component, col_group_id) pairs with any runs (for grid gating) +// +// Grid placeholders are only injected for cells where data confirms +// tests actually ran, so that cells without data on one side correctly show +// MissingSample / MissingBasis instead of NotSignificant. +func (p *PostgresProvider) queryTestStatus( + ctx context.Context, + reqOptions reqopts.RequestOptions, + includeVariants map[string][]string, + spec testStatusSpec, +) (map[string]crstatus.TestStatus, []error) { + + includeVariants = mergeRequestedVariants(includeVariants, reqOptions) + filters := buildDrilldownFilters(reqOptions) + + setup, err := prepareVariantQuery(ctx, p.dbc, includeVariants, reqOptions.VariantOption.DBGroupBy, reqOptions.AdvancedOption.MinimumFailure) + if err != nil { + return nil, []error{err} + } + if setup == nil { + return map[string]crstatus.TestStatus{}, nil + } + + fromClause := fmt.Sprintf(spec.fromTemplate, setup.variantSubquery, setup.groupMapping.valuesClause) + + joinArgs := make([]any, 0, len(spec.preJoinArgs)+len(setup.filterArgs)+len(spec.whereArgs)) + joinArgs = append(joinArgs, spec.preJoinArgs...) + joinArgs = append(joinArgs, setup.filterArgs...) + joinArgs = append(joinArgs, spec.whereArgs...) + + failureAgg := fmt.Sprintf(` + SELECT + e.test_id, e.suite_id, vg.group_id AS variant_group_id, + SUM(%s) AS total_count, + SUM(%s) AS success_count, + SUM(%s) AS flake_count + %s + WHERE %s`+filters.innerClause+` + GROUP BY e.test_id, e.suite_id, vg.group_id + HAVING SUM(%s) > 0 + AND SUM(%s) - SUM(%s) - SUM(%s) >= ?`, + spec.totalExpr, spec.successExpr, spec.flakeExpr, + fromClause, + spec.whereFilter, + spec.totalExpr, spec.totalExpr, spec.successExpr, spec.flakeExpr) + + failureInner := fmt.Sprintf(lastFailureLateral, failureAgg, sippyv1.TestStatusFailure) + + failureArgs := make([]any, len(joinArgs)) + copy(failureArgs, joinArgs) + failureArgs = append(failureArgs, filters.innerArgs...) + failureArgs = append(failureArgs, setup.minimumFailure, spec.release, spec.dateRange.Start, spec.dateRange.End) + + colMapping := buildColumnGroupMapping(setup.groupMapping.groupToVariants, reqOptions.VariantOption.ColumnGroupBy) + + placeholderQuery := fmt.Sprintf(` + SELECT + 'grid:' || tow.component AS test_id, + '' AS test_name, + '' AS test_suite, + tow.component, + tow.capabilities, + cm.col_group_id AS variant_group_id, + 1 AS total_count, + 1 AS success_count, + 0 AS flake_count, + NULL::timestamptz AS last_failure + %s + JOIN test_ownerships tow ON tow.test_id = e.test_id + AND (tow.suite_id = e.suite_id OR (tow.suite_id IS NULL AND e.suite_id = 0)) + AND tow.staff_approved_obsolete = false + JOIN (%s) AS cm(group_id, col_group_id) ON cm.group_id = vg.group_id + WHERE %s`+filters.outerClause+` + GROUP BY tow.component, tow.capabilities, cm.col_group_id + HAVING SUM(%s) > 0`, + fromClause, colMapping.valuesClause, spec.whereFilter, spec.totalExpr) + + placeholderArgs := make([]any, len(joinArgs)) + copy(placeholderArgs, joinArgs) + placeholderArgs = append(placeholderArgs, filters.outerArgs...) + + return p.runFailureAndPlaceholder(ctx, failureInner, failureArgs, placeholderQuery, placeholderArgs, + setup.groupMapping, filters) +} + +// queryTestStatusPrefixSum queries test_cumulative_summaries using a +// 2-way self-join on prefix sums to compute aggregated counts for a date range. +func (p *PostgresProvider) queryTestStatusPrefixSum( + ctx context.Context, + reqOptions reqopts.RequestOptions, + release string, + includeVariants map[string][]string, + dateRange query.DateRange, +) (map[string]crstatus.TestStatus, []error) { + + if err := query.ResolveDateRanges(p.dbc, release, &dateRange); err != nil { + return nil, []error{err} + } + lookupEnd := dateRange.End.AddDays(-1) + lookupStart := dateRange.Start.AddDays(-1) + + return p.queryTestStatus(ctx, reqOptions, includeVariants, testStatusSpec{ + fromTemplate: ` + FROM test_cumulative_summaries e + LEFT JOIN test_cumulative_summaries s + ON s.release = e.release AND s.test_id = e.test_id + AND s.prow_job_id = e.prow_job_id AND s.suite_id = e.suite_id + AND s.date = ? + JOIN prow_jobs pj ON pj.id = e.prow_job_id AND pj.deleted_at IS NULL + AND pj.variant_combination_id IN (%s) + JOIN (%s) AS vg(vcid, group_id) ON vg.vcid = pj.variant_combination_id`, + preJoinArgs: []any{lookupStart}, + totalExpr: "e.prefix_sum_runs - COALESCE(s.prefix_sum_runs, 0)", + successExpr: "e.prefix_sum_successes - COALESCE(s.prefix_sum_successes, 0)", + flakeExpr: "e.prefix_sum_flakes - COALESCE(s.prefix_sum_flakes, 0)", + whereFilter: "e.release = ? AND e.date = ?", + whereArgs: []any{release, lookupEnd}, + release: release, + dateRange: dateRange, + }) +} + +// queryBaseTestStatusGA queries prow_ga_raw_test_data to compute aggregated +// base test status for GA releases. +func (p *PostgresProvider) queryBaseTestStatusGA( + ctx context.Context, + reqOptions reqopts.RequestOptions, + baseRange query.DateRange, +) (map[string]crstatus.TestStatus, []error) { + + release := reqOptions.BaseRelease.Name + windowDays := baseRange.End.AddDays(-1).DaysSince(baseRange.Start) + + return p.queryTestStatus(ctx, reqOptions, reqOptions.VariantOption.IncludeVariants, testStatusSpec{ + fromTemplate: ` + FROM prow_ga_raw_test_data e + JOIN prow_jobs pj ON pj.id = e.prow_job_id AND pj.deleted_at IS NULL + AND pj.variant_combination_id IN (%s) + JOIN (%s) AS vg(vcid, group_id) ON vg.vcid = pj.variant_combination_id`, + totalExpr: "e.runs", + successExpr: "e.passes", + flakeExpr: "e.flakes", + whereFilter: "e.release = ? AND e.window_days = ?", + whereArgs: []any{release, windowDays}, + release: release, + dateRange: baseRange, + }) +} + +// runFailureAndPlaceholder runs the failure query and a placeholder query in +// parallel. The placeholder query groups by (component, col_group_id) to +// identify grid cells that have data, returning rows in the same 10-column +// format as the failure query. After both complete, placeholder entries are +// merged into the failure map for cells that have data but no failures. +func (p *PostgresProvider) runFailureAndPlaceholder( + ctx context.Context, + failureInner string, + failureArgs []any, + placeholderQuery string, + placeholderArgs []any, + groupMapping variantGroupMapping, + filters drilldownFilters, +) (map[string]crstatus.TestStatus, []error) { + + var failureResult map[string]crstatus.TestStatus + var failureErrs []error + var placeholderResult map[string]crstatus.TestStatus + var placeholderErrs []error + + var wg sync.WaitGroup + wg.Go(func() { + failureResult, failureErrs = p.queryAndScan(ctx, failureInner, failureArgs, groupMapping, filters) + }) + wg.Go(func() { + placeholderResult, placeholderErrs = p.scanGroupedResults(ctx, placeholderQuery, placeholderArgs, groupMapping) + }) + wg.Wait() + + if len(failureErrs) > 0 || len(placeholderErrs) > 0 { + var errs []error + errs = append(errs, failureErrs...) + errs = append(errs, placeholderErrs...) + return nil, errs + } + + merged := 0 + for k, v := range placeholderResult { + if _, exists := failureResult[k]; !exists { + failureResult[k] = v + merged++ + } + } + log.WithField("placeholders", len(placeholderResult)). + WithField("merged", merged). + WithField("failures", len(failureResult)-merged). + WithField("total", len(failureResult)). + Info("placeholder query complete") + return failureResult, nil +} + +// outerQuery wraps an inner aggregation subquery with the shared outer SELECT +// that joins tests, test_ownerships, and suites to produce the final result +// columns. Both sample and base queries use the same outer structure. +const outerQuery = `SELECT + tow.unique_id AS test_id, + t.name AS test_name, + COALESCE(su.name, '') AS test_suite, + tow.component, + tow.capabilities, + pa.variant_group_id, + pa.total_count, + pa.success_count, + pa.flake_count, + pa.last_failure +FROM (%s) pa +JOIN tests t ON t.id = pa.test_id +JOIN test_ownerships tow ON tow.test_id = pa.test_id + AND (tow.suite_id = pa.suite_id OR (tow.suite_id IS NULL AND pa.suite_id = 0)) +LEFT JOIN suites su ON su.id = pa.suite_id +WHERE tow.staff_approved_obsolete = false` + +// queryAndScan wraps an inner aggregation subquery with the shared outer query, +// appends any drill-down filter clauses, and executes with parallel worker hints. +func (p *PostgresProvider) queryAndScan( + ctx context.Context, + innerQuery string, + innerArgs []any, + groupMapping variantGroupMapping, + filters drilldownFilters, +) (map[string]crstatus.TestStatus, []error) { + fullQuery := fmt.Sprintf(outerQuery, innerQuery) + filters.outerClause + allArgs := make([]any, 0, len(innerArgs)+len(filters.outerArgs)) + allArgs = append(allArgs, innerArgs...) + allArgs = append(allArgs, filters.outerArgs...) + return p.scanWithParallelHints(ctx, fullQuery, allArgs, groupMapping) +} + +// scanGroupedResults executes a query that is already grouped by +// variant_group_id (not variant_combination_id), mapping each group ID back +// to dimension values via the group mapping. +func (p *PostgresProvider) scanGroupedResults( + ctx context.Context, + sqlQuery string, + args []any, + groupMapping variantGroupMapping, +) (map[string]crstatus.TestStatus, []error) { + return p.scanWithParallelHints(ctx, sqlQuery, args, groupMapping) +} + +// scanWithParallelHints runs the query inside a transaction that enables +// parallel workers, then scans the results into a test status map. +func (p *PostgresProvider) scanWithParallelHints( + ctx context.Context, + sqlQuery string, + args []any, + groupMapping variantGroupMapping, +) (map[string]crstatus.TestStatus, []error) { + var result map[string]crstatus.TestStatus + txErr := p.dbc.DB.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + if err := tx.Exec("SET LOCAL max_parallel_workers_per_gather = 4; SET LOCAL parallel_setup_cost = 0; SET LOCAL parallel_tuple_cost = 0").Error; err != nil { + return fmt.Errorf("setting parallel query hints: %w", err) + } + var err error + result, err = scanRows(tx, sqlQuery, args, groupMapping) + return err + }) + if txErr != nil { + return nil, []error{txErr} + } + return result, nil +} + +func scanRows( + gormDB *gorm.DB, + sqlQuery string, + args []any, + groupMapping variantGroupMapping, +) (map[string]crstatus.TestStatus, error) { + + rows, err := gormDB.Raw(sqlQuery, args...).Rows() + if err != nil { + return nil, fmt.Errorf("querying test status: %w", err) + } + defer rows.Close() + + result := make(map[string]crstatus.TestStatus) + + for rows.Next() { + var testID, testName, testSuite, component string + var capabilities pq.StringArray + var variantGroupID int + var totalCount, successCount, flakeCount int + var lastFailure sql.NullTime + + if err := rows.Scan( + &testID, &testName, &testSuite, &component, &capabilities, + &variantGroupID, &totalCount, &successCount, &flakeCount, + &lastFailure, + ); err != nil { + return nil, fmt.Errorf("scanning row: %w", err) + } + + variantMap := groupMapping.groupToVariants[variantGroupID] + + key := crtest.KeyWithVariants{ + TestID: testID, + Variants: variantMap, + } + keyStr := key.Encode() + + ts := crstatus.TestStatus{ + TestID: testID, + TestName: testName, + TestSuite: testSuite, + Component: component, + Capabilities: capabilities, + Variants: variantMap, + Count: crtest.Count{ + TotalCount: totalCount, + SuccessCount: successCount, + FlakeCount: flakeCount, + }, + } + if lastFailure.Valid { + ts.LastFailure = lastFailure.Time + } + result[keyStr] = ts + } + + if err := rows.Err(); err != nil { + return nil, err + } + return result, nil +} diff --git a/pkg/api/componentreadiness/dataprovider/postgres/provider.go b/pkg/api/componentreadiness/dataprovider/postgres/provider.go index edba44f127..81a2685588 100644 --- a/pkg/api/componentreadiness/dataprovider/postgres/provider.go +++ b/pkg/api/componentreadiness/dataprovider/postgres/provider.go @@ -2,6 +2,7 @@ package postgres import ( "context" + "errors" "fmt" "math/big" "slices" @@ -9,7 +10,10 @@ import ( "strings" "time" + "cloud.google.com/go/civil" "github.com/lib/pq" + log "github.com/sirupsen/logrus" + "gorm.io/gorm" "k8s.io/apimachinery/pkg/util/sets" "github.com/openshift/sippy/pkg/api" @@ -21,6 +25,8 @@ import ( "github.com/openshift/sippy/pkg/apis/cache" 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/query" ) var _ dataprovider.DataProvider = &PostgresProvider{} @@ -64,21 +70,11 @@ func parseVariants(variants pq.StringArray) map[string]string { return result } -// variantMapToSlice converts a map to sorted "Key:Value" strings. -func variantMapToSlice(m map[string]string) []string { - result := make([]string, 0, len(m)) - for k, v := range m { - result = append(result, k+":"+v) - } - sort.Strings(result) - return result -} - // filterByDBGroupBy returns a copy of the variant map keeping only keys in dbGroupBy. -func filterByDBGroupBy(variants map[string]string, dbGroupBy map[string]bool) map[string]string { - filtered := make(map[string]string, len(dbGroupBy)) +func filterByDBGroupBy(variants map[string]string, dbGroupBy sets.Set[string]) map[string]string { + filtered := make(map[string]string, dbGroupBy.Len()) for k, v := range variants { - if dbGroupBy[k] { + if dbGroupBy.Has(k) { filtered[k] = v } } @@ -101,7 +97,7 @@ func matchesIncludeVariants(variants map[string]string, includeVariants map[stri // --- MetadataQuerier --- -func (p *PostgresProvider) QueryJobVariants(ctx context.Context) (crtest.JobVariants, []error) { +func (p *PostgresProvider) QueryJobVariants(ctx context.Context, _ reqopts.RequestOptions) (crtest.JobVariants, []error) { variants := crtest.JobVariants{Variants: map[string][]string{}} var pairs []string @@ -146,7 +142,7 @@ func (p *PostgresProvider) QueryReleaseDates(ctx context.Context, reqOptions req return timeRanges, nil } -func (p *PostgresProvider) QueryUniqueVariantValues(ctx context.Context, field string, nested bool) ([]string, error) { +func (p *PostgresProvider) QueryUniqueVariantValues(ctx context.Context, _ reqopts.RequestOptions, field string, nested bool) ([]string, error) { if nested { // Return all variant key names var pairs []string @@ -206,134 +202,6 @@ func (p *PostgresProvider) QueryUniqueVariantValues(ctx context.Context, field s return result, nil } -// --- TestStatusQuerier --- - -// testStatusRow is the result of the aggregation query. -type testStatusRow struct { - TestID string `gorm:"column:test_id"` - TestName string `gorm:"column:test_name"` - TestSuite string `gorm:"column:test_suite"` - Component string `gorm:"column:component"` - Capabilities pq.StringArray `gorm:"column:capabilities;type:text[]"` - ProwJobID uint `gorm:"column:prow_job_id"` - TotalCount int `gorm:"column:total_count"` - SuccessCount int `gorm:"column:success_count"` - FlakeCount int `gorm:"column:flake_count"` - LastFailure *time.Time `gorm:"column:last_failure"` -} - -const testStatusQuery = ` -WITH deduped AS ( - SELECT DISTINCT ON (pjrt.prow_job_run_id, pjrt.test_id, pjrt.suite_id) - pjrt.test_id, pjrt.suite_id, pjrt.status, - pjr.timestamp, pj.id AS prow_job_id - FROM prow_job_run_tests pjrt - JOIN prow_job_runs pjr ON pjr.id = pjrt.prow_job_run_id - JOIN prow_jobs pj ON pj.id = pjr.prow_job_id - WHERE pj.release = ? - AND pjr.timestamp >= ? AND pjr.timestamp < ? - AND pjr.prow_job_release = ? - AND pjrt.prow_job_run_release = ? - AND pjrt.prow_job_run_timestamp >= ? AND pjrt.prow_job_run_timestamp < ? - AND pjrt.deleted_at IS NULL AND pjr.deleted_at IS NULL AND pj.deleted_at IS NULL - AND (pjr.labels IS NULL OR NOT pjr.labels @> ARRAY['InfraFailure']) - ORDER BY pjrt.prow_job_run_id, pjrt.test_id, pjrt.suite_id, - CASE WHEN pjrt.status = 13 THEN 0 WHEN pjrt.status = 1 THEN 1 ELSE 2 END -) -SELECT - tow.unique_id AS test_id, - t.name AS test_name, - COALESCE(s.name, '') AS test_suite, - tow.component, - tow.capabilities, - d.prow_job_id, - COUNT(*) AS total_count, - SUM(CASE WHEN d.status IN (1, 13) THEN 1 ELSE 0 END) AS success_count, - SUM(CASE WHEN d.status = 13 THEN 1 ELSE 0 END) AS flake_count, - MAX(CASE WHEN d.status NOT IN (1, 13) THEN d.timestamp ELSE NULL END) AS last_failure -FROM deduped d -JOIN tests t ON t.id = d.test_id -JOIN test_ownerships tow ON tow.test_id = d.test_id - AND (tow.suite_id = d.suite_id OR (tow.suite_id IS NULL AND d.suite_id IS NULL)) -LEFT JOIN suites s ON s.id = d.suite_id -WHERE tow.staff_approved_obsolete = false -GROUP BY tow.unique_id, t.name, s.name, tow.component, tow.capabilities, d.prow_job_id -` - -func (p *PostgresProvider) queryTestStatus(ctx context.Context, release string, start, end time.Time, - includeVariants map[string][]string, - dbGroupBy map[string]bool) (map[string]crstatus.TestStatus, []error) { - - var rows []testStatusRow - if err := p.dbc.DB.WithContext(ctx).Raw(testStatusQuery, release, start, end, release, release, start, end).Scan(&rows).Error; err != nil { - return nil, []error{fmt.Errorf("querying test status: %w", err)} - } - - // Batch-fetch all ProwJob variants we need - jobIDs := make(map[uint]bool, len(rows)) - for _, r := range rows { - jobIDs[r.ProwJobID] = true - } - ids := make([]uint, 0, len(jobIDs)) - for id := range jobIDs { - ids = append(ids, id) - } - jobVariantMap, err := p.fetchJobVariantsByIDs(ids) - if err != nil { - return nil, []error{err} - } - - result := map[string]crstatus.TestStatus{} - for _, row := range rows { - variants, ok := jobVariantMap[row.ProwJobID] - if !ok { - continue - } - - if !matchesIncludeVariants(variants, includeVariants) { - continue - } - - filtered := filterByDBGroupBy(variants, dbGroupBy) - key := crtest.KeyWithVariants{ - TestID: row.TestID, - Variants: filtered, - } - keyStr := key.KeyOrDie() - - existing, exists := result[keyStr] - if exists { - // Merge counts for same test+variant combo from different job runs - existing.TotalCount += row.TotalCount - existing.SuccessCount += row.SuccessCount - existing.FlakeCount += row.FlakeCount - if row.LastFailure != nil && (existing.LastFailure.IsZero() || row.LastFailure.After(existing.LastFailure)) { - existing.LastFailure = *row.LastFailure - } - result[keyStr] = existing - } else { - ts := crstatus.TestStatus{ - TestName: row.TestName, - TestSuite: row.TestSuite, - Component: row.Component, - Capabilities: row.Capabilities, - Variants: variantMapToSlice(filtered), - Count: crtest.Count{ - TotalCount: row.TotalCount, - SuccessCount: row.SuccessCount, - FlakeCount: row.FlakeCount, - }, - } - if row.LastFailure != nil { - ts.LastFailure = *row.LastFailure - } - result[keyStr] = ts - } - } - - return result, nil -} - // fetchJobVariantsByIDs loads ProwJob variant maps for the given job IDs. func (p *PostgresProvider) fetchJobVariantsByIDs(ids []uint) (map[uint]map[string]string, error) { if len(ids) == 0 { @@ -357,48 +225,80 @@ func (p *PostgresProvider) fetchJobVariantsByIDs(ids []uint) (map[uint]map[strin return result, nil } -func (p *PostgresProvider) QueryBaseTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions) (map[string]crstatus.TestStatus, []error) { - - dbGroupBy := make(map[string]bool, reqOptions.VariantOption.DBGroupBy.Len()) - for _, k := range sets.List(reqOptions.VariantOption.DBGroupBy) { - dbGroupBy[k] = true +// baseMatchesGAWindow returns true when the base release dates align with a +// pre-computed GA window in prow_ga_raw_test_data. +func (p *PostgresProvider) baseMatchesGAWindow(ctx context.Context, release string, baseRange query.DateRange) bool { + var rd models.ReleaseDefinition + err := p.dbc.DB.WithContext(ctx). + Select("ga_date"). + Where("release = ?", release). + First(&rd).Error + if err != nil { + if !errors.Is(err, gorm.ErrRecordNotFound) { + log.WithError(err).WithField("release", release). + Warn("failed to query GA date, falling back to prefix-sum query") + } + return false + } + if rd.GADate == nil { + return false } - includeVariants := reqOptions.VariantOption.IncludeVariants - if includeVariants == nil { - includeVariants = map[string][]string{} + gaDate := civil.DateOf(*rd.GADate) + if gaDate.After(civil.DateOf(time.Now().UTC())) { + return false + } + if baseRange.End != utils.GAWindowEnd(gaDate) { + return false } - return p.queryTestStatus( - ctx, - reqOptions.BaseRelease.Name, - reqOptions.BaseRelease.Start, - reqOptions.BaseRelease.End, - includeVariants, - dbGroupBy, - ) + windowDays := gaDate.DaysSince(baseRange.Start) + return slices.Contains(utils.GAWindows, windowDays) } -func (p *PostgresProvider) QuerySampleTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions, - includeVariants map[string][]string, - start, end time.Time) (map[string]crstatus.TestStatus, []error) { - - dbGroupBy := make(map[string]bool, reqOptions.VariantOption.DBGroupBy.Len()) - for _, k := range sets.List(reqOptions.VariantOption.DBGroupBy) { - dbGroupBy[k] = true +func (p *PostgresProvider) QueryBaseTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions) (map[string]crstatus.TestStatus, []error) { + baseRange := query.DateRange{ + Start: civil.DateOf(reqOptions.BaseRelease.Start), + End: civil.DateOf(reqOptions.BaseRelease.End).AddDays(1), + } + if p.baseMatchesGAWindow(ctx, reqOptions.BaseRelease.Name, baseRange) { + return p.queryBaseTestStatusGA(ctx, reqOptions, baseRange) } + return p.queryTestStatusPrefixSum(ctx, reqOptions, + reqOptions.BaseRelease.Name, + reqOptions.VariantOption.IncludeVariants, + baseRange) +} - if includeVariants == nil { - includeVariants = map[string][]string{} +// mergeCompareVariants returns a copy of includeVariants with CompareVariants +// merged in for cross-compare views. For cross-compare, IncludeVariants holds +// base-side values (e.g. Topology:[ha]) while CompareVariants holds sample-side +// values (e.g. Topology:[single]). Sample queries need the merged set. +func mergeCompareVariants(reqOptions reqopts.RequestOptions, includeVariants map[string][]string) map[string][]string { + if len(reqOptions.VariantOption.VariantCrossCompare) == 0 { + return includeVariants + } + merged := make(map[string][]string, len(includeVariants)) + for k, v := range includeVariants { + merged[k] = v } + for _, k := range reqOptions.VariantOption.VariantCrossCompare { + if v, ok := reqOptions.VariantOption.CompareVariants[k]; ok { + merged[k] = v + } + } + return merged +} - return p.queryTestStatus( - ctx, - reqOptions.SampleRelease.Name, - start, end, - includeVariants, - dbGroupBy, - ) +func (p *PostgresProvider) QuerySampleTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions, + includeVariants map[string][]string, + start, end time.Time) (map[string]crstatus.TestStatus, []error) { + includeVariants = mergeCompareVariants(reqOptions, includeVariants) + return p.queryTestStatusPrefixSum(ctx, reqOptions, reqOptions.SampleRelease.Name, includeVariants, + query.DateRange{ + Start: civil.DateOf(start), + End: civil.DateOf(end).AddDays(1), + }) } // --- TestDetailsQuerier --- @@ -416,9 +316,37 @@ type testDetailRow struct { JiraComponentID *uint `gorm:"column:jira_component_id"` } -const testDetailQuery = ` +func (p *PostgresProvider) queryTestDetails(ctx context.Context, release string, start, end time.Time, + reqOptions reqopts.RequestOptions, + includeVariants map[string][]string) (map[string][]crstatus.TestJobRunRows, []error) { + + if includeVariants == nil { + includeVariants = map[string][]string{} + } + + // MATERIALIZED CTE forces the planner to resolve test_ids first, then + // drive prow_job_run_tests via the test_id index. Without it, the global + // work_mem=128MB setting causes the planner to choose a prow_jobs-first + // plan that scans ~20K runs × 30 partitions and never completes. + testIDs := make([]string, 0, len(reqOptions.TestIDOptions)) + for _, tid := range reqOptions.TestIDOptions { + testIDs = append(testIDs, tid.TestID) + } + + sqlQuery := `WITH target_tests AS MATERIALIZED ( + SELECT test_id, suite_id, unique_id, jira_component, jira_component_id + FROM test_ownerships + WHERE staff_approved_obsolete = false` + + var args []any + if len(testIDs) > 0 { + sqlQuery += ` AND unique_id IN (?)` + args = append(args, testIDs) + } + + sqlQuery += `) SELECT - tow.unique_id AS test_id, + tt.unique_id AS test_id, t.name AS test_name, pj.name AS prowjob_name, CAST(pjr.id AS TEXT) AS prowjob_run_id, @@ -426,44 +354,42 @@ SELECT pjr.timestamp AS prowjob_start, pj.id AS prow_job_id, pjrt.status, - COALESCE(tow.jira_component, '') AS jira_component, - tow.jira_component_id -FROM prow_job_run_tests pjrt + COALESCE(tt.jira_component, '') AS jira_component, + tt.jira_component_id +FROM target_tests tt +JOIN prow_job_run_tests pjrt ON pjrt.test_id = tt.test_id + AND (tt.suite_id = pjrt.suite_id OR (tt.suite_id IS NULL AND pjrt.suite_id IS NULL)) JOIN prow_job_runs pjr ON pjr.id = pjrt.prow_job_run_id JOIN prow_jobs pj ON pj.id = pjr.prow_job_id JOIN tests t ON t.id = pjrt.test_id -JOIN test_ownerships tow ON tow.test_id = pjrt.test_id - AND (tow.suite_id = pjrt.suite_id OR (tow.suite_id IS NULL AND pjrt.suite_id IS NULL)) WHERE pj.release = ? AND pjr.timestamp >= ? AND pjr.timestamp < ? AND pjr.prow_job_release = ? AND pjrt.prow_job_run_release = ? AND pjrt.prow_job_run_timestamp >= ? AND pjrt.prow_job_run_timestamp < ? AND pjrt.deleted_at IS NULL AND pjr.deleted_at IS NULL AND pj.deleted_at IS NULL - AND tow.staff_approved_obsolete = false - AND (pjr.labels IS NULL OR NOT pjr.labels @> ARRAY['InfraFailure']) -ORDER BY pjr.timestamp -` + AND (pjr.labels IS NULL OR NOT pjr.labels @> ARRAY['InfraFailure'])` -func (p *PostgresProvider) queryTestDetails(ctx context.Context, release string, start, end time.Time, - reqOptions reqopts.RequestOptions, - includeVariants map[string][]string) (map[string][]crstatus.TestJobRunRows, []error) { + args = append(args, release, start, end, release, release, start, end) - var rows []testDetailRow - if err := p.dbc.DB.WithContext(ctx).Raw(testDetailQuery, release, start, end, release, release, start, end).Scan(&rows).Error; err != nil { - return nil, []error{fmt.Errorf("querying test details: %w", err)} + if len(includeVariants) > 0 { + filterClause, filterArgs := buildVariantFilterClause(includeVariants) + if filterClause != "" { + sqlQuery += " AND pj.variant_combination_id IN (SELECT vc.id FROM variant_combinations vc WHERE " + filterClause + ")" + args = append(args, filterArgs...) + } } - dbGroupBy := make(map[string]bool, reqOptions.VariantOption.DBGroupBy.Len()) - for _, k := range sets.List(reqOptions.VariantOption.DBGroupBy) { - dbGroupBy[k] = true - } + sqlQuery += " ORDER BY pjr.timestamp" - if includeVariants == nil { - includeVariants = map[string][]string{} + var rows []testDetailRow + if err := p.dbc.DB.WithContext(ctx).Raw(sqlQuery, args...).Scan(&rows).Error; err != nil { + return nil, []error{fmt.Errorf("querying test details: %w", err)} } - // Batch-fetch job variants + dbGroupBy := reqOptions.VariantOption.DBGroupBy + + // Batch-fetch job variants for per-test requested variant filtering jobIDs := map[uint]bool{} for _, r := range rows { jobIDs[r.ProwJobID] = true @@ -477,12 +403,8 @@ func (p *PostgresProvider) queryTestDetails(ctx context.Context, release string, return nil, []error{err} } - // Filter test IDs if specified - // Build test ID filter and per-test requested variant filters - testIDFilter := map[string]bool{} requestedVariantsByTestID := map[string]map[string]string{} for _, tid := range reqOptions.TestIDOptions { - testIDFilter[tid.TestID] = true if len(tid.RequestedVariants) > 0 { requestedVariantsByTestID[tid.TestID] = tid.RequestedVariants } @@ -490,19 +412,11 @@ func (p *PostgresProvider) queryTestDetails(ctx context.Context, release string, result := map[string][]crstatus.TestJobRunRows{} for _, row := range rows { - if len(testIDFilter) > 0 && !testIDFilter[row.TestID] { - continue - } - variants, ok := jobVariantMap[row.ProwJobID] if !ok { continue } - if !matchesIncludeVariants(variants, includeVariants) { - continue - } - // Filter by requested variants (exact match for specific test+variant combo) if rv, ok := requestedVariantsByTestID[row.TestID]; ok { match := true for k, v := range rv { @@ -539,7 +453,7 @@ func (p *PostgresProvider) queryTestDetails(ctx context.Context, release string, normalizedName := utils.NormalizeProwJobName(row.ProwJobName) entry := crstatus.TestJobRunRows{ TestKey: key, - TestKeyStr: key.KeyOrDie(), + TestKeyStr: key.Encode(), TestName: row.TestName, ProwJob: normalizedName, ProwJobRunID: row.ProwJobRunID, @@ -569,12 +483,11 @@ func (p *PostgresProvider) QueryBaseJobRunTestStatus(ctx context.Context, reqOpt func (p *PostgresProvider) QuerySampleJobRunTestStatus(ctx context.Context, reqOptions reqopts.RequestOptions, includeVariants map[string][]string, start, end time.Time) (map[string][]crstatus.TestJobRunRows, []error) { - return p.queryTestDetails( ctx, reqOptions.SampleRelease.Name, start, end, - reqOptions, includeVariants, + reqOptions, mergeCompareVariants(reqOptions, includeVariants), ) } @@ -657,7 +570,7 @@ func (p *PostgresProvider) QueryJobRuns(ctx context.Context, reqOptions reqopts. return results, nil } -func (p *PostgresProvider) QueryJobVariantValues(ctx context.Context, jobNames []string, +func (p *PostgresProvider) QueryJobVariantValues(ctx context.Context, _ reqopts.RequestOptions, jobNames []string, variantKeys []string) (map[string]map[string]string, error) { if len(jobNames) == 0 { @@ -697,7 +610,7 @@ func (p *PostgresProvider) QueryJobVariantValues(ctx context.Context, jobNames [ return results, nil } -func (p *PostgresProvider) LookupJobVariants(ctx context.Context, jobName string) (map[string]string, error) { +func (p *PostgresProvider) LookupJobVariants(ctx context.Context, _ reqopts.RequestOptions, jobName string) (map[string]string, error) { type jvRow struct { Variants pq.StringArray `gorm:"column:variants;type:text[]"` } diff --git a/pkg/api/componentreadiness/dataprovider/postgres/provider_test.go b/pkg/api/componentreadiness/dataprovider/postgres/provider_test.go new file mode 100644 index 0000000000..e9c40ea72b --- /dev/null +++ b/pkg/api/componentreadiness/dataprovider/postgres/provider_test.go @@ -0,0 +1,149 @@ +package postgres + +import ( + "testing" + + "github.com/lib/pq" + "k8s.io/apimachinery/pkg/util/sets" +) + +func TestParseVariants(t *testing.T) { + tests := []struct { + name string + variants pq.StringArray + want map[string]string + }{ + { + name: "empty array", + variants: pq.StringArray{}, + want: map[string]string{}, + }, + { + name: "single variant", + variants: pq.StringArray{"Platform:aws"}, + want: map[string]string{"Platform": "aws"}, + }, + { + name: "multiple variants", + variants: pq.StringArray{"Platform:aws", "Network:ovn", "Architecture:amd64"}, + want: map[string]string{"Platform": "aws", "Network": "ovn", "Architecture": "amd64"}, + }, + { + name: "value containing colon uses first colon only", + variants: pq.StringArray{"Suite:openshift-tests:sig-auth"}, + want: map[string]string{"Suite": "openshift-tests:sig-auth"}, + }, + { + name: "entry without colon is skipped", + variants: pq.StringArray{"malformed", "Platform:aws"}, + want: map[string]string{"Platform": "aws"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := parseVariants(tc.variants) + if len(got) != len(tc.want) { + t.Fatalf("len = %d, want %d", len(got), len(tc.want)) + } + for k, wantV := range tc.want { + if gotV, ok := got[k]; !ok { + t.Errorf("missing key %q", k) + } else if gotV != wantV { + t.Errorf("got[%q] = %q, want %q", k, gotV, wantV) + } + } + }) + } +} + +func TestFilterByDBGroupBy(t *testing.T) { + tests := []struct { + name string + variants map[string]string + dbGroupBy sets.Set[string] + want map[string]string + }{ + { + name: "empty variants", + variants: map[string]string{}, + dbGroupBy: sets.New[string]("Platform"), + want: map[string]string{}, + }, + { + name: "keeps matching keys", + variants: map[string]string{"Platform": "aws", "Network": "ovn", "Architecture": "amd64"}, + dbGroupBy: sets.New[string]("Platform", "Network"), + want: map[string]string{"Platform": "aws", "Network": "ovn"}, + }, + { + name: "no matching keys", + variants: map[string]string{"Platform": "aws"}, + dbGroupBy: sets.New[string]("Network"), + want: map[string]string{}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := filterByDBGroupBy(tc.variants, tc.dbGroupBy) + if len(got) != len(tc.want) { + t.Fatalf("len = %d, want %d", len(got), len(tc.want)) + } + for k, wantV := range tc.want { + if gotV := got[k]; gotV != wantV { + t.Errorf("got[%q] = %q, want %q", k, gotV, wantV) + } + } + }) + } +} + +func TestMatchesIncludeVariants(t *testing.T) { + tests := []struct { + name string + variants map[string]string + includeVariants map[string][]string + want bool + }{ + { + name: "empty filter matches everything", + variants: map[string]string{"Platform": "aws"}, + includeVariants: map[string][]string{}, + want: true, + }, + { + name: "matching single key", + variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + includeVariants: map[string][]string{"Platform": {"aws", "gcp"}}, + want: true, + }, + { + name: "value not in allowed list", + variants: map[string]string{"Platform": "azure"}, + includeVariants: map[string][]string{"Platform": {"aws", "gcp"}}, + want: false, + }, + { + name: "required key missing from variants", + variants: map[string]string{"Platform": "aws"}, + includeVariants: map[string][]string{"Network": {"ovn"}}, + want: false, + }, + { + name: "all keys must match", + variants: map[string]string{"Platform": "aws", "Network": "sdn"}, + includeVariants: map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + want: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := matchesIncludeVariants(tc.variants, tc.includeVariants) + if got != tc.want { + t.Errorf("got %v, want %v", got, tc.want) + } + }) + } +} diff --git a/pkg/api/componentreadiness/dataprovider/postgres/variants.go b/pkg/api/componentreadiness/dataprovider/postgres/variants.go new file mode 100644 index 0000000000..038211a089 --- /dev/null +++ b/pkg/api/componentreadiness/dataprovider/postgres/variants.go @@ -0,0 +1,226 @@ +package postgres + +import ( + "context" + "fmt" + "maps" + "sort" + "strings" + + "github.com/lib/pq" + "k8s.io/apimachinery/pkg/util/sets" + + "github.com/openshift/sippy/pkg/apis/api/componentreport/crtest" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + "github.com/openshift/sippy/pkg/db" +) + +// buildVariantFilterClause generates the WHERE clause fragment and bind args +// for filtering variant_combinations by includeVariants. Each key produces an +// array-overlap condition: variants && ARRAY['Key:v1','Key:v2']::text[]. +func buildVariantFilterClause(includeVariants map[string][]string) (string, []any) { + var clauses []string + var args []any + + keys := make([]string, 0, len(includeVariants)) + for k := range includeVariants { + keys = append(keys, k) + } + sort.Strings(keys) + + for _, key := range keys { + values := includeVariants[key] + if len(values) == 0 { + continue + } + placeholders := make([]string, len(values)) + for i, v := range values { + placeholders[i] = "?" + args = append(args, key+":"+v) + } + clauses = append(clauses, fmt.Sprintf( + "variants && ARRAY[%s]::text[]", strings.Join(placeholders, ", "))) + } + return strings.Join(clauses, " AND "), args +} + +// lookupVariantValues queries variant_combinations matching the filter and +// returns a map from variant_combination_id to the extracted variant values +// for each dbGroupBy key. This runs as a small, fast query (~6ms for ~400 +// rows) and the result is used to enrich aggregated rows in Go. +func lookupVariantValues( + ctx context.Context, + dbc *db.DB, + includeVariants map[string][]string, + dbGroupBy sets.Set[string], +) (map[uint]map[string]string, error) { + filterClause, args := buildVariantFilterClause(includeVariants) + + query := "SELECT id, variants FROM variant_combinations" + if filterClause != "" { + query += " WHERE " + filterClause + } + + type vcRow struct { + ID uint `gorm:"column:id"` + Variants pq.StringArray `gorm:"column:variants;type:text[]"` + } + + var rows []vcRow + if err := dbc.DB.WithContext(ctx).Raw(query, args...).Scan(&rows).Error; err != nil { + return nil, fmt.Errorf("looking up variant values: %w", err) + } + + result := make(map[uint]map[string]string, len(rows)) + for _, row := range rows { + parsed := parseVariants(row.Variants) + filtered := make(map[string]string, dbGroupBy.Len()) + for k, v := range parsed { + if dbGroupBy.Has(k) { + filtered[k] = v + } + } + result[row.ID] = filtered + } + return result, nil +} + +// variantGroupMapping holds the result of grouping variant_combination_ids by +// their dbGroupBy dimension values. Multiple VCIDs that share the same +// (Platform, Architecture, Network, ...) combo get the same group ID. +type variantGroupMapping struct { + // valuesClause is a SQL fragment like "VALUES (1,0),(2,0),(3,1),..." + // for joining as (vcid, group_id) in the query. + valuesClause string + + // groupToVariants maps each group ID to its dimension key-value pairs. + groupToVariants map[int]map[string]string +} + +// buildVariantGroupMapping assigns a sequential group ID to each unique +// combination of dbGroupBy dimension values across all VCIDs. The resulting +// VALUES clause can be joined in SQL to push dimension-level GROUP BY into the +// database, reducing ~500K rows to ~81K (matching BQ's output granularity). +func buildVariantGroupMapping(variantLookup map[uint]map[string]string) variantGroupMapping { + type groupEntry struct { + groupID int + variants map[string]string + } + + seen := make(map[string]groupEntry) + groupToVariants := make(map[int]map[string]string) + nextGroupID := 0 + + var valuePairs []string + + vcids := make([]uint, 0, len(variantLookup)) + for vcid := range variantLookup { + vcids = append(vcids, vcid) + } + sort.Slice(vcids, func(i, j int) bool { return vcids[i] < vcids[j] }) + + for _, vcid := range vcids { + dims := variantLookup[vcid] + keyStr := crtest.EncodeVariants(dims) + + entry, exists := seen[keyStr] + if !exists { + dimsCopy := make(map[string]string, len(dims)) + for k, v := range dims { + dimsCopy[k] = v + } + entry = groupEntry{ + groupID: nextGroupID, + variants: dimsCopy, + } + seen[keyStr] = entry + groupToVariants[nextGroupID] = dimsCopy + nextGroupID++ + } + valuePairs = append(valuePairs, fmt.Sprintf("(%d,%d)", vcid, entry.groupID)) + } + + valuesClause := "" + if len(valuePairs) > 0 { + valuesClause = "VALUES " + strings.Join(valuePairs, ",") + } + + return variantGroupMapping{ + valuesClause: valuesClause, + groupToVariants: groupToVariants, + } +} + +// mergeRequestedVariants returns a copy of includeVariants with +// RequestedVariants from TestIDOptions merged in. Each RequestedVariant +// key replaces the corresponding includeVariants entry with a single-value +// slice, narrowing the filter to an exact match. This mirrors what the +// BQ provider does with per-variant WHERE clauses in BuildComponentReportQuery. +func mergeRequestedVariants(includeVariants map[string][]string, reqOptions reqopts.RequestOptions) map[string][]string { + if len(reqOptions.TestIDOptions) != 1 || len(reqOptions.TestIDOptions[0].RequestedVariants) == 0 { + return includeVariants + } + merged := make(map[string][]string, len(includeVariants)) + maps.Copy(merged, includeVariants) + for k, v := range reqOptions.TestIDOptions[0].RequestedVariants { + merged[k] = []string{v} + } + return merged +} + +// columnGroupMapping maps each real group_id to a synthetic col_group_id that +// has only columnGroupBy dimensions. Multiple group_ids that share the same +// column-level projection get the same col_group_id. The synthetic IDs start +// above the real group IDs to avoid collision, and their variant maps are +// registered in groupToVariants for scanGroupedResults to resolve. +type columnGroupMapping struct { + valuesClause string +} + +// buildColumnGroupMapping mutates groupToVariants by inserting synthetic +// col_group_id entries so that scanGroupedResults can resolve them. +// Must only be called once per groupToVariants map. +func buildColumnGroupMapping( + groupToVariants map[int]map[string]string, + columnGroupBy sets.Set[string], +) columnGroupMapping { + nextColGroupID := len(groupToVariants) + seen := make(map[string]int) // serialized column variants -> col_group_id + + var valuePairs []string + + groupIDs := make([]int, 0, len(groupToVariants)) + for gid := range groupToVariants { + groupIDs = append(groupIDs, gid) + } + sort.Ints(groupIDs) + + for _, gid := range groupIDs { + variants := groupToVariants[gid] + colVariants := make(map[string]string, columnGroupBy.Len()) + for k, v := range variants { + if columnGroupBy.Has(k) { + colVariants[k] = v + } + } + keyStr := crtest.EncodeVariants(colVariants) + + colGID, exists := seen[keyStr] + if !exists { + colGID = nextColGroupID + seen[keyStr] = colGID + groupToVariants[colGID] = colVariants + nextColGroupID++ + } + valuePairs = append(valuePairs, fmt.Sprintf("(%d,%d)", gid, colGID)) + } + + valuesClause := "" + if len(valuePairs) > 0 { + valuesClause = "VALUES " + strings.Join(valuePairs, ",") + } + + return columnGroupMapping{ + valuesClause: valuesClause, + } +} diff --git a/pkg/api/componentreadiness/dataprovider/postgres/variants_test.go b/pkg/api/componentreadiness/dataprovider/postgres/variants_test.go new file mode 100644 index 0000000000..88a4b92488 --- /dev/null +++ b/pkg/api/componentreadiness/dataprovider/postgres/variants_test.go @@ -0,0 +1,307 @@ +package postgres + +import ( + "testing" + + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" + "k8s.io/apimachinery/pkg/util/sets" +) + +func TestBuildVariantFilterClause(t *testing.T) { + tests := []struct { + name string + includeVariants map[string][]string + wantClause string + wantArgCount int + wantArgs []any + }{ + { + name: "empty filter", + includeVariants: map[string][]string{}, + wantClause: "", + wantArgCount: 0, + }, + { + name: "single key single value", + includeVariants: map[string][]string{"Platform": {"aws"}}, + wantClause: "variants && ARRAY[?]::text[]", + wantArgCount: 1, + wantArgs: []any{"Platform:aws"}, + }, + { + name: "single key multiple values", + includeVariants: map[string][]string{"Platform": {"aws", "gcp"}}, + wantClause: "variants && ARRAY[?, ?]::text[]", + wantArgCount: 2, + wantArgs: []any{"Platform:aws", "Platform:gcp"}, + }, + { + name: "multiple keys sorted alphabetically", + includeVariants: map[string][]string{ + "Platform": {"aws"}, + "Network": {"ovn"}, + }, + wantClause: "variants && ARRAY[?]::text[] AND variants && ARRAY[?]::text[]", + wantArgCount: 2, + wantArgs: []any{"Network:ovn", "Platform:aws"}, + }, + { + name: "key with empty values skipped", + includeVariants: map[string][]string{"Platform": {}}, + wantClause: "", + wantArgCount: 0, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + clause, args := buildVariantFilterClause(tc.includeVariants) + if clause != tc.wantClause { + t.Errorf("clause = %q, want %q", clause, tc.wantClause) + } + if len(args) != tc.wantArgCount { + t.Errorf("len(args) = %d, want %d", len(args), tc.wantArgCount) + } + if tc.wantArgs != nil { + for i, want := range tc.wantArgs { + if i >= len(args) { + break + } + if args[i] != want { + t.Errorf("args[%d] = %v, want %v", i, args[i], want) + } + } + } + }) + } +} + +func TestMergeRequestedVariants(t *testing.T) { + tests := []struct { + name string + includeVariants map[string][]string + reqOptions reqopts.RequestOptions + want map[string][]string + }{ + { + name: "no TestIDOptions returns input unchanged", + includeVariants: map[string][]string{"Platform": {"aws", "gcp"}}, + reqOptions: reqopts.RequestOptions{}, + want: map[string][]string{"Platform": {"aws", "gcp"}}, + }, + { + name: "multiple TestIDOptions returns input unchanged", + includeVariants: map[string][]string{"Platform": {"aws"}}, + reqOptions: reqopts.RequestOptions{ + TestIDOptions: []reqopts.TestIdentification{{}, {}}, + }, + want: map[string][]string{"Platform": {"aws"}}, + }, + { + name: "empty RequestedVariants returns input unchanged", + includeVariants: map[string][]string{"Platform": {"aws", "gcp"}}, + reqOptions: reqopts.RequestOptions{ + TestIDOptions: []reqopts.TestIdentification{{}}, + }, + want: map[string][]string{"Platform": {"aws", "gcp"}}, + }, + { + name: "overrides existing key with single value", + includeVariants: map[string][]string{"Platform": {"aws", "gcp"}, "Network": {"ovn", "sdn"}}, + reqOptions: reqopts.RequestOptions{ + TestIDOptions: []reqopts.TestIdentification{{ + RequestedVariants: map[string]string{"Platform": "aws"}, + }}, + }, + want: map[string][]string{"Platform": {"aws"}, "Network": {"ovn", "sdn"}}, + }, + { + name: "adds new key", + includeVariants: map[string][]string{"Platform": {"aws"}}, + reqOptions: reqopts.RequestOptions{ + TestIDOptions: []reqopts.TestIdentification{{ + RequestedVariants: map[string]string{"Topology": "ha"}, + }}, + }, + want: map[string][]string{"Platform": {"aws"}, "Topology": {"ha"}}, + }, + { + name: "does not modify original map", + includeVariants: map[string][]string{"Platform": {"aws", "gcp"}}, + reqOptions: reqopts.RequestOptions{ + TestIDOptions: []reqopts.TestIdentification{{ + RequestedVariants: map[string]string{"Platform": "aws"}, + }}, + }, + want: map[string][]string{"Platform": {"aws"}}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + origLen := len(tc.includeVariants["Platform"]) + result := mergeRequestedVariants(tc.includeVariants, tc.reqOptions) + + for k, wantVals := range tc.want { + gotVals, ok := result[k] + if !ok { + t.Errorf("missing key %q", k) + continue + } + if len(gotVals) != len(wantVals) { + t.Errorf("key %q: got %v, want %v", k, gotVals, wantVals) + continue + } + for i, want := range wantVals { + if gotVals[i] != want { + t.Errorf("key %q[%d] = %q, want %q", k, i, gotVals[i], want) + } + } + } + + if tc.name == "does not modify original map" { + if len(tc.includeVariants["Platform"]) != origLen { + t.Error("original includeVariants was mutated") + } + } + }) + } +} + +func TestBuildVariantGroupMapping(t *testing.T) { + tests := []struct { + name string + variantLookup map[uint]map[string]string + wantValuesClause string + wantGroupVariants map[int]map[string]string + }{ + { + name: "empty input", + variantLookup: map[uint]map[string]string{}, + wantValuesClause: "", + wantGroupVariants: map[int]map[string]string{}, + }, + { + name: "single vcid", + variantLookup: map[uint]map[string]string{ + 1: {"Platform": "aws"}, + }, + wantValuesClause: "VALUES (1,0)", + wantGroupVariants: map[int]map[string]string{0: {"Platform": "aws"}}, + }, + { + name: "two vcids same dimensions get same group", + variantLookup: map[uint]map[string]string{ + 1: {"Platform": "aws"}, + 2: {"Platform": "aws"}, + }, + wantValuesClause: "VALUES (1,0),(2,0)", + wantGroupVariants: map[int]map[string]string{0: {"Platform": "aws"}}, + }, + { + name: "two vcids different dimensions get different groups", + variantLookup: map[uint]map[string]string{ + 1: {"Platform": "aws"}, + 2: {"Platform": "gcp"}, + }, + wantValuesClause: "VALUES (1,0),(2,1)", + wantGroupVariants: map[int]map[string]string{ + 0: {"Platform": "aws"}, + 1: {"Platform": "gcp"}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := buildVariantGroupMapping(tc.variantLookup) + if result.valuesClause != tc.wantValuesClause { + t.Errorf("valuesClause = %q, want %q", result.valuesClause, tc.wantValuesClause) + } + if len(result.groupToVariants) != len(tc.wantGroupVariants) { + t.Fatalf("group count = %d, want %d", len(result.groupToVariants), len(tc.wantGroupVariants)) + } + for gid, wantVars := range tc.wantGroupVariants { + gotVars, ok := result.groupToVariants[gid] + if !ok { + t.Errorf("missing group %d", gid) + continue + } + for k, wantV := range wantVars { + if gotV := gotVars[k]; gotV != wantV { + t.Errorf("group %d: got[%q] = %q, want %q", gid, k, gotV, wantV) + } + } + } + }) + } +} + +func TestBuildColumnGroupMapping(t *testing.T) { + tests := []struct { + name string + groupToVariants map[int]map[string]string + columnGroupBy sets.Set[string] + wantValuesClause string + wantNewEntries map[int]map[string]string + }{ + { + name: "empty input", + groupToVariants: map[int]map[string]string{}, + columnGroupBy: sets.New[string]("Platform"), + wantValuesClause: "", + wantNewEntries: map[int]map[string]string{}, + }, + { + name: "groups with same column projection collapse", + groupToVariants: map[int]map[string]string{ + 0: {"Platform": "aws", "Network": "ovn"}, + 1: {"Platform": "aws", "Network": "sdn"}, + }, + columnGroupBy: sets.New[string]("Platform"), + wantValuesClause: "VALUES (0,2),(1,2)", + wantNewEntries: map[int]map[string]string{2: {"Platform": "aws"}}, + }, + { + name: "groups with different column projection stay separate", + groupToVariants: map[int]map[string]string{ + 0: {"Platform": "aws", "Network": "ovn"}, + 1: {"Platform": "gcp", "Network": "ovn"}, + }, + columnGroupBy: sets.New[string]("Platform"), + wantValuesClause: "VALUES (0,2),(1,3)", + wantNewEntries: map[int]map[string]string{ + 2: {"Platform": "aws"}, + 3: {"Platform": "gcp"}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + initialSize := len(tc.groupToVariants) + result := buildColumnGroupMapping(tc.groupToVariants, tc.columnGroupBy) + + if result.valuesClause != tc.wantValuesClause { + t.Errorf("valuesClause = %q, want %q", result.valuesClause, tc.wantValuesClause) + } + + newEntryCount := len(tc.groupToVariants) - initialSize + if newEntryCount != len(tc.wantNewEntries) { + t.Fatalf("new synthetic entries = %d, want %d", newEntryCount, len(tc.wantNewEntries)) + } + for gid, wantVars := range tc.wantNewEntries { + gotVars, ok := tc.groupToVariants[gid] + if !ok { + t.Errorf("missing synthetic group %d", gid) + continue + } + for k, wantV := range wantVars { + if gotV := gotVars[k]; gotV != wantV { + t.Errorf("group %d: got[%q] = %q, want %q", gid, k, gotV, wantV) + } + } + } + }) + } +} diff --git a/pkg/api/componentreadiness/middleware/linkinjector/linkinjector.go b/pkg/api/componentreadiness/middleware/linkinjector/linkinjector.go index 06e6f16c3c..7d7fca6f48 100644 --- a/pkg/api/componentreadiness/middleware/linkinjector/linkinjector.go +++ b/pkg/api/componentreadiness/middleware/linkinjector/linkinjector.go @@ -75,6 +75,7 @@ func (l *LinkInjector) PostAnalysis(testKey crtest.Identification, testStats *te testKey.Capability, variants, baseReleaseOverride, + l.reqOptions.DataSource, ) if err != nil { l.log.WithError(err).Warnf("failed to generate test details URL for test %s", testKey.TestID) diff --git a/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go b/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go index 0e3b1fdb7f..72c85e50ba 100644 --- a/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go +++ b/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker.go @@ -3,7 +3,6 @@ package regressiontracker import ( "context" "fmt" - "strings" "sync" "time" @@ -57,11 +56,12 @@ func NewRegressionTrackerMiddleware(dbc *db.DB, reqOptions reqopts.RequestOption // inject details onto regressed test stats if they match known regressions. // It also handles adjustments if those regressions are triaged to bugs. type RegressionTracker struct { - log log.FieldLogger - reqOptions reqopts.RequestOptions - dbc *db.DB - failureCounter failureCounterFunc - openRegressions []*models.TestRegression + log log.FieldLogger + reqOptions reqopts.RequestOptions + dbc *db.DB + failureCounter failureCounterFunc + openRegressions []*models.TestRegression + regressionsByTestID map[string][]*models.TestRegression // hasLoadedRegressions will be true once we've loaded regression data hasLoadedRegressions bool } @@ -91,14 +91,24 @@ func (r *RegressionTracker) ensureRegressionsLoaded() error { if err != nil { return err } + r.regressionsByTestID = BuildRegressionIndex(r.openRegressions) r.log.Infof("Found %d open regressions", len(r.openRegressions)) r.hasLoadedRegressions = true return nil } +// BuildRegressionIndex groups regressions by TestID for O(1) lookup. +func BuildRegressionIndex(regressions []*models.TestRegression) map[string][]*models.TestRegression { + idx := make(map[string][]*models.TestRegression, len(regressions)) + for _, reg := range regressions { + idx[reg.TestID] = append(idx[reg.TestID], reg) + } + return idx +} + func (r *RegressionTracker) PreAnalysis(testKey crtest.Identification, testStats *testdetails.TestComparison) error { if len(r.openRegressions) > 0 { - or := FindOpenRegression(r.reqOptions.SampleRelease.Name, testKey.TestID, len(r.reqOptions.VariantOption.VariantCrossCompare) > 0, testKey.Variants, r.openRegressions) + or := FindOpenRegression(r.reqOptions.SampleRelease.Name, testKey.TestID, len(r.reqOptions.VariantOption.VariantCrossCompare) > 0, testKey.Variants, r.regressionsByTestID) if or != nil { testStats.Regression = or @@ -130,7 +140,7 @@ func (r *RegressionTracker) PostAnalysis(testKey crtest.Identification, testStat return err } if len(r.openRegressions) > 0 { - or := FindOpenRegression(r.reqOptions.SampleRelease.Name, testKey.TestID, len(r.reqOptions.VariantOption.VariantCrossCompare) > 0, testKey.Variants, r.openRegressions) + or := FindOpenRegression(r.reqOptions.SampleRelease.Name, testKey.TestID, len(r.reqOptions.VariantOption.VariantCrossCompare) > 0, testKey.Variants, r.regressionsByTestID) r.log.Debugf("checking regressions for %+v", testKey) if or == nil { return nil @@ -203,56 +213,41 @@ func (r *RegressionTracker) PostAnalysis(testKey crtest.Identification, testStat return nil } -// FindOpenRegression scans the list of open regressions for any that match the given test summary. -// The regressions list is expected to be pre-filtered by sample release (e.g. from ListOpenRegressions); -// the sampleRelease check is redundant but kept for safety. +// FindOpenRegression looks up open regressions matching the given test by +// testID, then checks release, crossCompare, and variant subset matching. +// The index is keyed by testID for O(1) lookup instead of a linear scan. func FindOpenRegression(sampleRelease, testID string, crossCompare bool, variants map[string]string, - regressions []*models.TestRegression) *models.TestRegression { + regressionsByTestID map[string][]*models.TestRegression) *models.TestRegression { - var matches []*models.TestRegression - for _, tr := range regressions { + candidates := regressionsByTestID[testID] + for _, tr := range candidates { if sampleRelease != tr.Release { continue } if tr.CrossCompare != crossCompare { continue } - // We compare test ID not name, as names can change. - if tr.TestID != testID { + if !variantsMatch(tr.Variants, variants) { continue } - // Subset matching: check if ALL regression variants are present in the input variants. - // This allows the input to have additional variants beyond what the regression has, - // which supports db_column_groupby modifications that add new variants. - found := true - for _, variant := range tr.Variants { - keyVal := strings.Split(variant, ":") - if len(keyVal) != 2 { - // Malformed variant, skip this regression - found = false - break - } - variantKey := keyVal[0] - variantValue := keyVal[1] + return tr + } + return nil +} - inputValue, exists := variants[variantKey] - if !exists || inputValue != variantValue { - found = false - break - } - } - if !found { - continue +// variantsMatch checks if ALL regression variants are present in the input +// variants (subset matching). This allows the input to have additional variants +// beyond what the regression has, supporting db_column_groupby modifications. +func variantsMatch(regressionVariants []string, inputVariants map[string]string) bool { + for _, variant := range regressionVariants { + key, value := crtest.VariantStringToKeyValue(variant) + if key == "" || inputVariants[key] != value { + return false } - // If we made it this far, this appears to be a match: - matches = append(matches, tr) - } - if len(matches) > 0 { - return matches[0] } - return nil + return true } func (r *RegressionTracker) PreTestDetailsAnalysis(testKey crtest.KeyWithVariants, status *crstatus.TestJobRunStatuses) error { diff --git a/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker_test.go b/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker_test.go index 059c233067..632dd686eb 100644 --- a/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker_test.go +++ b/pkg/api/componentreadiness/middleware/regressiontracker/regressiontracker_test.go @@ -289,6 +289,7 @@ func TestRegressionTracker_PostAnalysis(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { mw.openRegressions = []*models.TestRegression{&tt.openRegression} + mw.regressionsByTestID = BuildRegressionIndex(mw.openRegressions) mw.hasLoadedRegressions = true mw.log = logrus.New() err := mw.PostAnalysis(testKey, &tt.testStats) @@ -380,11 +381,11 @@ func TestRegressionTracker_PreAnalysis_Adjustments(t *testing.T) { Closed: sql.NullTime{Valid: false}, } mw.openRegressions = []*models.TestRegression{openRegression} - mw.hasLoadedRegressions = true } else { mw.openRegressions = []*models.TestRegression{} - mw.hasLoadedRegressions = true } + mw.regressionsByTestID = BuildRegressionIndex(mw.openRegressions) + mw.hasLoadedRegressions = true // Run PreAnalysis err := mw.PreAnalysis(testKey, testStats) @@ -566,6 +567,7 @@ func TestRegressionTracker_PreAnalysis_RegressionMatching(t *testing.T) { }, }, openRegressions: tt.openRegressions, + regressionsByTestID: BuildRegressionIndex(tt.openRegressions), hasLoadedRegressions: true, log: logrus.New(), } @@ -702,7 +704,8 @@ func TestFindOpenRegression(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := FindOpenRegression(sampleRelease, testID, false, variants, tt.regressions) + idx := BuildRegressionIndex(tt.regressions) + got := FindOpenRegression(sampleRelease, testID, false, variants, idx) if !tt.wantMatch { assert.Nil(t, got, "expected no match") return @@ -740,15 +743,17 @@ func TestFindOpenRegression_CrossCompareIsolation(t *testing.T) { }, } + idx := BuildRegressionIndex(regressions) + t.Run("standard view selects standard regression", func(t *testing.T) { - got := FindOpenRegression(sampleRelease, testID, false, variants, regressions) + got := FindOpenRegression(sampleRelease, testID, false, variants, idx) require.NotNil(t, got) assert.Equal(t, uint(1), got.ID) assert.False(t, got.CrossCompare) }) t.Run("cross-compare view selects cross-compare regression", func(t *testing.T) { - got := FindOpenRegression(sampleRelease, testID, true, variants, regressions) + got := FindOpenRegression(sampleRelease, testID, true, variants, idx) require.NotNil(t, got) assert.Equal(t, uint(2), got.ID) assert.True(t, got.CrossCompare) @@ -867,7 +872,8 @@ func TestFindOpenRegression_SubsetMatching(t *testing.T) { }, } - got := FindOpenRegression(sampleRelease, testID, false, tt.inputVariants, regressions) + idx := BuildRegressionIndex(regressions) + got := FindOpenRegression(sampleRelease, testID, false, tt.inputVariants, idx) if !tt.wantMatch { assert.Nil(t, got, "expected no match but got regression ID %v", got) @@ -991,6 +997,7 @@ func TestRegressionTracker_PostAnalysis_KeyTestThreshold(t *testing.T) { Triages: []models.Triage{resolvedTriage}, } + openRegs := []*models.TestRegression{openRegression} mw := RegressionTracker{ reqOptions: reqopts.RequestOptions{ BaseRelease: reqopts.Release{Name: baseRelease}, @@ -1000,7 +1007,8 @@ func TestRegressionTracker_PostAnalysis_KeyTestThreshold(t *testing.T) { KeyTestNames: []string{keyTestName}, }, }, - openRegressions: []*models.TestRegression{openRegression}, + openRegressions: openRegs, + regressionsByTestID: BuildRegressionIndex(openRegs), hasLoadedRegressions: true, failureCounter: func(_ uint, _ time.Time) (int, error) { return tt.failureCount, tt.failureCountErr diff --git a/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go b/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go index c219007d50..dfd7b78376 100644 --- a/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go +++ b/pkg/api/componentreadiness/middleware/releasefallback/releasefallback.go @@ -2,7 +2,6 @@ package releasefallback import ( "context" - "encoding/json" "fmt" "sync" "time" @@ -102,8 +101,7 @@ func (r *ReleaseFallback) PreAnalysis(testKey crtest.Identification, testStats * TestID: testKey.TestID, Variants: testKey.Variants, } - testIDBytes, _ := json.Marshal(testIDVariantsKey) - testKeyStr := string(testIDBytes) + testKeyStr := testIDVariantsKey.Encode() if r.cachedFallbackTestStatuses == nil { // In the test details path, this map is not initialized and we have no work to do for pre analysis. @@ -280,7 +278,7 @@ func (r *ReleaseFallback) QueryTestDetails(ctx context.Context, wg *sync.WaitGro func (r *ReleaseFallback) PreTestDetailsAnalysis(testKey crtest.KeyWithVariants, status *crstatus.TestJobRunStatuses) error { // Add our baseOverrideStatus to the report, unfortunate hack we have to live with for now. - testKeyStr := testKey.KeyOrDie() + testKeyStr := testKey.Encode() if _, ok := r.baseOverrideStatus[testKeyStr]; ok { status.BaseOverrideStatus = r.baseOverrideStatus[testKeyStr] } @@ -336,6 +334,7 @@ type fallbackTestQueryReleasesGeneratorCacheKey struct { CRTimeRoundingOffset time.Duration // KeyTestNames affects the BuildComponentReportQuery results via filtering logic KeyTestNames []string + DataSource string `json:",omitempty"` } // getCacheKey creates a cache key using the generator properties that we want included for uniqueness in what @@ -350,6 +349,7 @@ func (f *fallbackTestQueryReleasesGenerator) getCacheKey() fallbackTestQueryRele CRTimeRoundingFactor: f.ReqOptions.CacheOption.CRTimeRoundingFactor, CRTimeRoundingOffset: f.ReqOptions.CacheOption.CRTimeRoundingOffset, KeyTestNames: f.ReqOptions.AdvancedOption.KeyTestNames, + DataSource: f.ReqOptions.DataSource, } } diff --git a/pkg/api/componentreadiness/middleware/releasefallback/releasefallback_test.go b/pkg/api/componentreadiness/middleware/releasefallback/releasefallback_test.go index 3625ebbd57..9604ab1ba5 100644 --- a/pkg/api/componentreadiness/middleware/releasefallback/releasefallback_test.go +++ b/pkg/api/componentreadiness/middleware/releasefallback/releasefallback_test.go @@ -1,7 +1,6 @@ package releasefallback import ( - "encoding/json" "testing" "time" @@ -25,14 +24,11 @@ func Test_PreAnalysis(t *testing.T) { "Arch": "amd64", "Platform": "aws", } - test1VariantsFlattened := []string{"Arch:amd64", "Platform:aws"} test1MapKey := crtest.KeyWithVariants{ TestID: test1ID, Variants: test1Variants, } - test1KeyBytes, err := json.Marshal(test1MapKey) - test1KeyStr := string(test1KeyBytes) - assert.NoError(t, err) + test1KeyStr := test1MapKey.Encode() test1RTI := crtest.Identification{ RowIdentification: crtest.RowIdentification{ Component: "", @@ -65,7 +61,7 @@ func Test_PreAnalysis(t *testing.T) { fallbackMap418 := ReleaseTestMap{ ReleaseTimeRange: release418, Tests: map[string]crstatus.TestStatus{ - test1KeyStr: buildTestStatus("test1", test1VariantsFlattened, 100, 95, 0), + test1KeyStr: buildTestStatus("test1", test1Variants, 100, 95, 0), }, } @@ -79,7 +75,7 @@ func Test_PreAnalysis(t *testing.T) { fallbackMap417 := ReleaseTestMap{ ReleaseTimeRange: release417, Tests: map[string]crstatus.TestStatus{ - test1KeyStr: buildTestStatus("test1", test1VariantsFlattened, 100, 98, 0), + test1KeyStr: buildTestStatus("test1", test1Variants, 100, 98, 0), }, } @@ -224,7 +220,7 @@ func TestCalculateFallbackReleases(t *testing.T) { } //nolint:unparam -func buildTestStatus(testName string, variants []string, total, success, flake int) crstatus.TestStatus { +func buildTestStatus(testName string, variants map[string]string, total, success, flake int) crstatus.TestStatus { return crstatus.TestStatus{ TestName: testName, TestSuite: "conformance", diff --git a/pkg/api/componentreadiness/queryparamparser_test.go b/pkg/api/componentreadiness/queryparamparser_test.go index 21677dd52f..7843ada39b 100644 --- a/pkg/api/componentreadiness/queryparamparser_test.go +++ b/pkg/api/componentreadiness/queryparamparser_test.go @@ -584,6 +584,7 @@ func TestHATEOASLinkCacheConsistency(t *testing.T) { "TestCapability", []string{"Architecture:amd64", "Platform:aws"}, "", + "", ) require.NoError(t, err) diff --git a/pkg/api/componentreadiness/regressiontracker.go b/pkg/api/componentreadiness/regressiontracker.go index 81a4982ea9..f11ebc39d0 100644 --- a/pkg/api/componentreadiness/regressiontracker.go +++ b/pkg/api/componentreadiness/regressiontracker.go @@ -290,10 +290,11 @@ func SyncRegressionsForReport( var openedRegs, reopenedRegs, ongoingRegs, statsUpdatedRegs int var activeRegressions []*models.TestRegression // all the matches we found, and new regressions opened, used to determine what had no match + regressionIndex := regressiontracker.BuildRegressionIndex(regressions) rLog.Infof("syncing %d open regressions", len(allRegressedTests)) + crossCompare := len(view.VariantOptions.VariantCrossCompare) > 0 for _, regTest := range allRegressedTests { - crossCompare := len(view.VariantOptions.VariantCrossCompare) > 0 - if openReg := regressiontracker.FindOpenRegression(view.SampleRelease.Name, regTest.TestID, crossCompare, regTest.Variants, regressions); openReg != nil { + if openReg := regressiontracker.FindOpenRegression(view.SampleRelease.Name, regTest.TestID, crossCompare, regTest.Variants, regressionIndex); openReg != nil { // Check if we need to add new variants to the regression found via subset matching. // This allows regressions to be split by new variant dimensions when db_column_groupby is modified. diff --git a/pkg/api/componentreadiness/test_details.go b/pkg/api/componentreadiness/test_details.go index 433fb4a50d..aee887f93c 100644 --- a/pkg/api/componentreadiness/test_details.go +++ b/pkg/api/componentreadiness/test_details.go @@ -160,7 +160,7 @@ func (c *ComponentReportGenerator) GenerateTestDetailsReportMultiTest(ctx contex TestID: tOpt.TestID, Variants: tOpt.RequestedVariants, } - testKeyStr := testKey.KeyOrDie() + testKeyStr := testKey.Encode() if statuses, ok := testKeyTestJobRunStatuses[testKeyStr]; ok { report, generateReportErrs := c.GenerateDetailsReportForTest(ctx, tOpt, statuses, false) if len(generateReportErrs) > 0 { @@ -186,16 +186,14 @@ func (c *ComponentReportGenerator) GenerateDetailsReportForTest( ) (testdetails.Report, []error) { if testIDOption.TestID == "" { - return testdetails.Report{}, []error{&api.ValidationError{ - Message: "test_id has to be defined for test details", - }} + return testdetails.Report{}, []error{&api.ValidationError{Message: "test_id has to be defined for test details"}} } for _, v := range sets.List(c.ReqOptions.VariantOption.DBGroupBy) { if _, ok := testIDOption.RequestedVariants[v]; !ok { - return testdetails.Report{}, []error{&api.ValidationError{ - Message: fmt.Sprintf("all dbGroupBy variants have to be defined for test details: %s is missing in %v", - v, testIDOption.RequestedVariants), - }} + return testdetails.Report{}, []error{ + &api.ValidationError{Message: fmt.Sprintf("all dbGroupBy variants have to be defined for test details: %s is missing in %v", + v, testIDOption.RequestedVariants)}, + } } } @@ -324,6 +322,7 @@ func (c *ComponentReportGenerator) GenerateDetailsReportForTest( testIDOption.Capability, variants, baseReleaseOverride, + c.ReqOptions.DataSource, ) if err != nil { logrus.WithError(err).Warnf("failed to generate latest test details URL for test %s", testIDOption.TestID) @@ -437,8 +436,14 @@ func (c *ComponentReportGenerator) internalGenerateTestDetailsReport( totalBase, totalSample, report, result, lastFailure := c.summarizeRecordedTestStats(baseStatus, sampleStatus, testKey) + explanations := []string{} + if len(baseStatus) == 0 && c.ReqOptions.DataSource == reqopts.DataSourcePostgres { + explanations = append(explanations, + "Base test details are not available: individual test run data has not been backfilled for this release in the PostgreSQL data source. Aggregated base statistics from the grid view remain accurate.") + } + testStats := testdetails.TestComparison{ - Explanations: []string{}, + Explanations: explanations, RequiredConfidence: c.ReqOptions.AdvancedOption.Confidence, SampleStats: testdetails.ReleaseStats{ Release: c.ReqOptions.SampleRelease.Name, diff --git a/pkg/api/componentreadiness/triage.go b/pkg/api/componentreadiness/triage.go index c64d0013a3..f2ea2ae9a4 100644 --- a/pkg/api/componentreadiness/triage.go +++ b/pkg/api/componentreadiness/triage.go @@ -873,6 +873,7 @@ func generateTestDetailsURLFromRegression(regression *models.TestRegression, vie regression.Capability, regression.Variants, regression.BaseRelease, + "", ) } diff --git a/pkg/api/componentreadiness/utils/queryparamparser.go b/pkg/api/componentreadiness/utils/queryparamparser.go index 9dec0f4413..b634940928 100644 --- a/pkg/api/componentreadiness/utils/queryparamparser.go +++ b/pkg/api/componentreadiness/utils/queryparamparser.go @@ -90,6 +90,10 @@ func ParseComponentReportRequest( return } + if dataSource := param.SafeRead(req, "dataSource"); dataSource != "" { + opts.DataSource = dataSource + } + opts.CacheOption = cache.NewStandardCROptions(crTimeRoundingFactor, crTimeRoundingOffset) opts.CacheOption.ForceRefresh, err = ParseBoolArg(req, "forceRefresh", false) if err != nil { diff --git a/pkg/api/componentreadiness/utils/utils.go b/pkg/api/componentreadiness/utils/utils.go index 1744c5eb98..13bfdde648 100644 --- a/pkg/api/componentreadiness/utils/utils.go +++ b/pkg/api/componentreadiness/utils/utils.go @@ -1,7 +1,6 @@ package utils import ( - "encoding/json" "fmt" "net/url" "regexp" @@ -61,51 +60,42 @@ func NormalizeProwJobName(prowName string) string { return prowName } -// DeserializeTestKey helps us workaround the limitations of a struct as a map key, where -// we instead serialize a very small struct to json for a unit test key that includes test -// ID and a specific set of variants. This function deserializes back to a struct. -func DeserializeTestKey(stats crstatus.TestStatus, testKeyStr string) (crtest.Identification, error) { - var testKey crtest.KeyWithVariants - err := json.Unmarshal([]byte(testKeyStr), &testKey) - if err != nil { - logrus.WithError(err).Errorf("trying to unmarshel %s", testKeyStr) - return crtest.Identification{}, err - } +// IdentificationFromStatus reconstructs an Identification from the TestStatus fields. +func IdentificationFromStatus(stats crstatus.TestStatus) crtest.Identification { testID := crtest.Identification{ RowIdentification: crtest.RowIdentification{ Component: stats.Component, TestName: stats.TestName, TestSuite: stats.TestSuite, - TestID: testKey.TestID, + TestID: stats.TestID, }, ColumnIdentification: crtest.ColumnIdentification{ - Variants: testKey.Variants, + Variants: stats.Variants, }, } // Take the first cap for now. When we reach to a cell with specific capability, we will override the value. if len(stats.Capabilities) > 0 { testID.Capability = stats.Capabilities[0] } - return testID, nil + return testID } // VariantsMapToStringSlice converts the map form of variants to a string slice // where each variant is formatted key:value. func VariantsMapToStringSlice(variants map[string]string) []string { - vs := []string{} + vs := make([]string, 0, len(variants)) for k, v := range variants { - vs = append(vs, fmt.Sprintf("%s:%s", k, v)) + vs = append(vs, crtest.VariantKeyValueToString(k, v)) } return vs } -// VariantsStringSliceToMap converts a slice of "key:value" strings to a map +// VariantsStringSliceToMap converts a slice of "key:value" strings to a map. func VariantsStringSliceToMap(variants []string) map[string]string { - variantMap := make(map[string]string) + variantMap := make(map[string]string, len(variants)) for _, variant := range variants { - parts := strings.SplitN(variant, ":", 2) - if len(parts) == 2 { - variantMap[parts[0]] = parts[1] + if k, v := crtest.VariantStringToKeyValue(variant); k != "" { + variantMap[k] = v } } return variantMap @@ -275,6 +265,7 @@ func GenerateTestDetailsURL( capability string, variants []string, baseReleaseOverride string, + dataSource string, ) (string, error) { if testID == "" { @@ -307,6 +298,10 @@ func GenerateTestDetailsURL( params.Add("view", viewName) } + if dataSource != "" { + params.Add("dataSource", dataSource) + } + // Always generate full URL with all parameters params.Add("testId", testID) diff --git a/pkg/api/componentreadiness/utils/utils_test.go b/pkg/api/componentreadiness/utils/utils_test.go index f3318a27f4..4aff7c59b7 100644 --- a/pkg/api/componentreadiness/utils/utils_test.go +++ b/pkg/api/componentreadiness/utils/utils_test.go @@ -79,6 +79,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "", []string{"Platform:aws"}, "", + "", ) require.NoError(t, err) assert.True(t, strings.HasPrefix(url, "/api/component_readiness/test_details")) @@ -98,6 +99,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "", []string{}, "", + "", ) assert.Error(t, err) assert.Contains(t, err.Error(), "testID cannot be empty") @@ -117,6 +119,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "", []string{"Architecture:amd64", "InvalidVariant", "Platform:aws"}, "", + "", ) require.NoError(t, err) @@ -141,6 +144,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { // Use variants in non-alphabetical order to test sorting []string{"Topology:ha", "Architecture:amd64", "Platform:aws", "Network:ovn"}, "", + "", ) require.NoError(t, err) @@ -162,6 +166,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "capability-example", []string{"Architecture:amd64", "Platform:aws"}, "", + "", ) require.NoError(t, err) assert.NotEmpty(t, url) @@ -196,6 +201,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "", []string{"Architecture:amd64", "Platform:aws"}, "4.17", // Different from view's base release + "", ) require.NoError(t, err) assert.NotEmpty(t, url) @@ -272,6 +278,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "FeatureSet:default", }, "4.18", + "", ) require.NoError(t, err) assert.NotEmpty(t, url) @@ -361,6 +368,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "", []string{}, "", + "", ) require.NoError(t, err) @@ -434,6 +442,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "", []string{"Platform:aws"}, "", + "", ) require.NoError(t, err) @@ -482,6 +491,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "", []string{"Platform:aws"}, "", + "", ) require.NoError(t, err) assert.NotEmpty(t, url) @@ -516,6 +526,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "", []string{"Platform:aws"}, "", + "", ) require.NoError(t, err) assert.NotEmpty(t, url) @@ -545,6 +556,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "", []string{"Platform:aws"}, "", + "", ) require.NoError(t, err) assert.NotEmpty(t, url) @@ -576,6 +588,7 @@ func TestGenerateTestDetailsURL(t *testing.T) { "capability-example", []string{"Platform:aws", "Architecture:amd64"}, "", + "", ) require.NoError(t, err) assert.NotEmpty(t, url) @@ -605,4 +618,44 @@ func TestGenerateTestDetailsURL(t *testing.T) { assert.Contains(t, url, "sampleEndTime=") }) + t.Run("dataSource parameter is included when set", func(t *testing.T) { + url, err := GenerateTestDetailsURL( + "test-id", + "https://sippy.example.com", + "", + getBaseReleaseOpts(), + getSampleReleaseOpts(), + testView.AdvancedOptions, + testView.VariantOptions, + reqopts.TestFilters{}, + "", + "", + []string{"Platform:aws"}, + "", + "postgres", + ) + require.NoError(t, err) + assert.Contains(t, url, "dataSource=postgres") + }) + + t.Run("dataSource parameter is omitted when empty", func(t *testing.T) { + url, err := GenerateTestDetailsURL( + "test-id", + "https://sippy.example.com", + "", + getBaseReleaseOpts(), + getSampleReleaseOpts(), + testView.AdvancedOptions, + testView.VariantOptions, + reqopts.TestFilters{}, + "", + "", + []string{"Platform:aws"}, + "", + "", + ) + require.NoError(t, err) + assert.NotContains(t, url, "dataSource") + }) + } diff --git a/pkg/api/utils.go b/pkg/api/utils.go index 75367eb51a..9ac9aaca18 100644 --- a/pkg/api/utils.go +++ b/pkg/api/utils.go @@ -20,7 +20,7 @@ func VariantsStringToSet(allJobVariants crtest.JobVariants, variantsString strin for _, v := range variants { // ensure the variant is one we've recorded in BQ, not just some random string if _, ok := allJobVariants.Variants[v]; !ok { - return variantSet, fmt.Errorf("invalid variant %s in variants string %s", v, variantsString) + return variantSet, &ValidationError{Message: fmt.Sprintf("invalid variant %s in variants string %s", v, variantsString)} } variantSet.Insert(v) } @@ -33,27 +33,27 @@ func VariantListToMap(allJobVariants crtest.JobVariants, variants []string) (map variantsMap := map[string][]string{} var err error for _, variant := range variants { - kv := strings.Split(variant, ":") - if len(kv) != 2 { - err = fmt.Errorf("invalid variant %s in list", variant) + key, value := crtest.VariantStringToKeyValue(variant) + if key == "" { + err = &ValidationError{Message: fmt.Sprintf("invalid variant %s in list", variant)} return variantsMap, err } // ensure the variant name/value is one we've recorded in BQ, not just some random string - values, ok := allJobVariants.Variants[kv[0]] + values, ok := allJobVariants.Variants[key] if !ok { - err = fmt.Errorf("invalid name from list variant %s", variant) + err = &ValidationError{Message: fmt.Sprintf("invalid name from list variant %s", variant)} return variantsMap, err } found := false for _, v := range values { - if v == kv[1] { - variantsMap[kv[0]] = append(variantsMap[kv[0]], kv[1]) + if v == value { + variantsMap[key] = append(variantsMap[key], value) found = true break } } if !found { - err = fmt.Errorf("invalid value from list variant %s", variant) + err = &ValidationError{Message: fmt.Sprintf("invalid value from list variant %s", variant)} return variantsMap, err } } @@ -91,12 +91,11 @@ func ValidateVariants(allJobVariants crtest.JobVariants, variantsMap map[string] func VariantListToMapWithWarnings(allJobVariants crtest.JobVariants, variants []string) (map[string][]string, []string, error) { variantsMap := map[string][]string{} for _, variant := range variants { - kv := strings.Split(variant, ":") - if len(kv) != 2 { - // This is a fatal error as the format is completely wrong - return variantsMap, nil, fmt.Errorf("invalid variant %s in list", variant) + key, value := crtest.VariantStringToKeyValue(variant) + if key == "" { + return variantsMap, nil, &ValidationError{Message: fmt.Sprintf("invalid variant %s in list", variant)} } - variantsMap[kv[0]] = append(variantsMap[kv[0]], kv[1]) + variantsMap[key] = append(variantsMap[key], value) } // Validate all variants and collect warnings diff --git a/pkg/apis/api/componentreport/crstatus/types.go b/pkg/apis/api/componentreport/crstatus/types.go index 8831cd5900..2dc2e831f6 100644 --- a/pkg/apis/api/componentreport/crstatus/types.go +++ b/pkg/apis/api/componentreport/crstatus/types.go @@ -27,11 +27,12 @@ type ReportTestStatus struct { // TestStatus is an internal type used to pass data from the data provider to the // actual report generation. It is not serialized over the API. type TestStatus struct { - TestName string `json:"test_name"` - TestSuite string `json:"test_suite"` - Component string `json:"component"` - Capabilities []string `json:"capabilities"` - Variants []string `json:"variants"` + TestID string `json:"test_id"` + TestName string `json:"test_name"` + TestSuite string `json:"test_suite"` + Component string `json:"component"` + Capabilities []string `json:"capabilities"` + Variants map[string]string `json:"variants"` crtest.Count LastFailure time.Time `json:"last_failure"` } diff --git a/pkg/apis/api/componentreport/crtest/types.go b/pkg/apis/api/componentreport/crtest/types.go index 95900a8526..95ea3f211f 100644 --- a/pkg/apis/api/componentreport/crtest/types.go +++ b/pkg/apis/api/componentreport/crtest/types.go @@ -1,7 +1,8 @@ package crtest import ( - "encoding/json" + "sort" + "strings" "time" ) @@ -96,14 +97,59 @@ type KeyWithVariants struct { Variants map[string]string `json:"variants"` } -// KeyOrDie serializes this test key into a json string suitable for use in maps. -// JSON serialization uses sorted map keys, so the output is stable. -func (t KeyWithVariants) KeyOrDie() string { - testIDBytes, err := json.Marshal(t) - if err != nil { - panic(err) +// VariantKeyValueToString formats a variant key and value as "key:value". +func VariantKeyValueToString(key, value string) string { + return key + ":" + value +} + +// VariantStringToKeyValue splits a "key:value" string into its key and value. +// Returns empty strings if the format is invalid. +func VariantStringToKeyValue(variant string) (string, string) { + k, v, ok := strings.Cut(variant, ":") + if !ok { + return "", "" + } + return k, v +} + +// EncodeVariants returns a deterministic null-byte-separated encoding of variant pairs. +func EncodeVariants(variants map[string]string) string { + pairs := make([]string, 0, len(variants)) + for k, v := range variants { + pairs = append(pairs, VariantKeyValueToString(k, v)) + } + sort.Strings(pairs) + return strings.Join(pairs, "\x00") +} + +// Encode returns a deterministic string encoding suitable for use as a map key. +// Format: testID\x00key1:val1\x00key2:val2 (sorted variant pairs, null-separated). +func (t KeyWithVariants) Encode() string { + encoded := EncodeVariants(t.Variants) + if encoded == "" { + return t.TestID + } + return t.TestID + "\x00" + encoded +} + +// Encode returns a deterministic string encoding for column identification. +// Format: key1:val1\x00key2:val2 (sorted variant pairs, null-separated). +func (c ColumnIdentification) Encode() ColumnID { + return ColumnID(EncodeVariants(c.Variants)) +} + +// DecodeColumnID reverses ColumnIdentification.Encode(). +func DecodeColumnID(key ColumnID) ColumnIdentification { + variants := make(map[string]string) + if key == "" { + return ColumnIdentification{Variants: variants} + } + for p := range strings.SplitSeq(string(key), "\x00") { + if k, v := VariantStringToKeyValue(p); k != "" { + variants[k] = v + } } - return string(testIDBytes) + return ColumnIdentification{Variants: variants} } type ReleaseTimeRange struct { diff --git a/pkg/apis/api/componentreport/crtest/types_test.go b/pkg/apis/api/componentreport/crtest/types_test.go new file mode 100644 index 0000000000..c9398d6189 --- /dev/null +++ b/pkg/apis/api/componentreport/crtest/types_test.go @@ -0,0 +1,117 @@ +package crtest + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestEncodeStability(t *testing.T) { + key := KeyWithVariants{ + TestID: "test-1", + Variants: map[string]string{ + "Zebra": "z", + "Alpha": "a", + "Middle": "m", + }, + } + first := key.Encode() + for range 100 { + assert.Equal(t, first, key.Encode(), "Encode() must be deterministic") + } + assert.Equal(t, "test-1\x00Alpha:a\x00Middle:m\x00Zebra:z", first) +} + +func TestEncodeNilVariants(t *testing.T) { + key := KeyWithVariants{TestID: "test-nil"} + encoded := key.Encode() + assert.Equal(t, "test-nil", encoded) +} + +func TestColumnEncodeDecodeRoundTrip(t *testing.T) { + tests := []struct { + name string + col ColumnIdentification + }{ + { + name: "typical columns", + col: ColumnIdentification{ + Variants: map[string]string{ + "Network": "ovn", + "Platform": "aws", + "Topology": "ha", + }, + }, + }, + { + name: "single column", + col: ColumnIdentification{ + Variants: map[string]string{"Platform": "gcp"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + encoded := tt.col.Encode() + decoded := DecodeColumnID(encoded) + assert.Equal(t, tt.col.Variants, decoded.Variants) + }) + } +} + +func TestVariantKeyValueToString(t *testing.T) { + tests := []struct { + name string + key string + value string + expected string + }{ + {name: "typical variant", key: "Platform", value: "aws", expected: "Platform:aws"}, + {name: "empty value", key: "Platform", value: "", expected: "Platform:"}, + {name: "empty key", key: "", value: "aws", expected: ":aws"}, + {name: "value with colon", key: "Label", value: "a:b", expected: "Label:a:b"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, VariantKeyValueToString(tt.key, tt.value)) + }) + } +} + +func TestVariantStringToKeyValue(t *testing.T) { + tests := []struct { + name string + input string + wantKey string + wantValue string + }{ + {name: "typical variant", input: "Platform:aws", wantKey: "Platform", wantValue: "aws"}, + {name: "empty value", input: "Platform:", wantKey: "Platform", wantValue: ""}, + {name: "value with colon", input: "Label:a:b", wantKey: "Label", wantValue: "a:b"}, + {name: "no colon", input: "invalid", wantKey: "", wantValue: ""}, + {name: "empty string", input: "", wantKey: "", wantValue: ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key, value := VariantStringToKeyValue(tt.input) + assert.Equal(t, tt.wantKey, key) + assert.Equal(t, tt.wantValue, value) + }) + } +} + +func TestVariantRoundTrip(t *testing.T) { + key, value := "Architecture", "amd64" + s := VariantKeyValueToString(key, value) + gotKey, gotValue := VariantStringToKeyValue(s) + assert.Equal(t, key, gotKey) + assert.Equal(t, value, gotValue) +} + +func TestColumnEncodeEmpty(t *testing.T) { + col := ColumnIdentification{Variants: map[string]string{}} + encoded := col.Encode() + assert.Equal(t, ColumnID(""), encoded) + decoded := DecodeColumnID(encoded) + assert.Empty(t, decoded.Variants) +} diff --git a/pkg/apis/api/componentreport/reqopts/types.go b/pkg/apis/api/componentreport/reqopts/types.go index 66912ee783..f8b7972ba9 100644 --- a/pkg/apis/api/componentreport/reqopts/types.go +++ b/pkg/apis/api/componentreport/reqopts/types.go @@ -10,6 +10,11 @@ import ( // These types represent report options requested by the user, // which need to be serialized as part of caching the report results. +const ( + DataSourceBigQuery = "bigquery" + DataSourcePostgres = "postgres" +) + // RequestOptions is a struct packaging all the options for a CR request. type RequestOptions struct { BaseRelease Release @@ -24,6 +29,9 @@ type RequestOptions struct { // When generating test details URLs, if a view is present, we include just the view parameter // plus test-specific overrides, rather than expanding all view parameters into the URL. ViewName string `json:"view_name,omitempty" yaml:"view_name,omitempty"` + // DataSource controls which backend is used for CR test queries. + // Valid values: "" (default, uses BigQuery), DataSourceBigQuery, DataSourcePostgres. + DataSource string `json:"data_source,omitempty" yaml:"data_source,omitempty"` } // PullRequest specifies a specific pull request to use as the diff --git a/pkg/dataloader/gateststatus/loader.go b/pkg/dataloader/gateststatus/loader.go index e0eb5ab45d..42ebde95d9 100644 --- a/pkg/dataloader/gateststatus/loader.go +++ b/pkg/dataloader/gateststatus/loader.go @@ -21,12 +21,10 @@ import ( // GATestStatusLoader populates prow_ga_raw_test_data for releases that have reached GA. // -// The loader has two conceptual phases: -// 1. Fetch: for each GA release, query BigQuery once for all configured windows -// and persist the raw results in prow_ga_raw_test_data. This runs only when -// the raw data is missing or the GA date changed, or when forced. -// 2. Aggregate: handled by the prow_ga_test_statuses_matview, which joins -// raw data with current dimension tables on each refresh cycle. +// For each GA release, it queries BigQuery once for all configured windows +// and persists the raw results in prow_ga_raw_test_data. This runs only when +// the raw data is missing or the GA date changed, or when forced. +// Aggregation happens at query time in the Component Readiness provider. type GATestStatusLoader struct { ctx context.Context dbc *db.DB diff --git a/pkg/db/models/prow.go b/pkg/db/models/prow.go index 53381ebb70..5a38f8f2aa 100644 --- a/pkg/db/models/prow.go +++ b/pkg/db/models/prow.go @@ -205,8 +205,7 @@ type TestCumulativeSummary struct { } // ProwGARawTestDatum stores raw BigQuery test results for GA release windows. -// Fetched once per GA date and persisted so that the aggregation into -// prow_ga_test_statuses_matview can be re-run cheaply when dimension tables change. +// Fetched once per GA date and persisted for query-time aggregation. // Each (release, window_days) pair holds results aggregated over a different lookback // period (e.g. 1, 30, or 90 days before GA). type ProwGARawTestDatum struct { diff --git a/pkg/db/query/cumulative_query.go b/pkg/db/query/cumulative_query.go index b7c685ef0b..33da66edf1 100644 --- a/pkg/db/query/cumulative_query.go +++ b/pkg/db/query/cumulative_query.go @@ -139,12 +139,12 @@ func TestReportQuery(dbc *db.DB, release string, sample, base DateRange, nameMat return testReportPreAgg(dbc, release, sample, base, nameMatches) } -// resolveDateRanges clamps the Start and End of each DateRange to the latest +// ResolveDateRanges clamps the Start and End of each DateRange to the latest // available date (+1, since DateRange uses half-open intervals) for the release // in test_cumulative_summaries. This ensures the planner sees literal dates it // can use for partition pruning, and handles cases where data hasn't been // backfilled up to the requested dates. -func resolveDateRanges(dbc *db.DB, release string, ranges ...*DateRange) error { +func ResolveDateRanges(dbc *db.DB, release string, ranges ...*DateRange) error { var maxDate *civil.Date row := dbc.DB.Table("test_cumulative_summaries"). Select("MAX(date)"). @@ -267,7 +267,7 @@ func processedFilterConditions(f *filter.Filter) (conditions []string, args []an // dates to the three prefix sum lookup dates used by the 3-way self-join // (each shifted by -1 day): end (e), boundary (m), and start (s). func resolvePrefixSumDates(dbc *db.DB, release string, sample, base *DateRange) (end, boundary, start civil.Date, err error) { - if err = resolveDateRanges(dbc, release, sample, base); err != nil { + if err = ResolveDateRanges(dbc, release, sample, base); err != nil { return } if sample.Start != base.End { diff --git a/pkg/db/query/feature_gates.go b/pkg/db/query/feature_gates.go index 6d0004b15e..154635fb42 100644 --- a/pkg/db/query/feature_gates.go +++ b/pkg/db/query/feature_gates.go @@ -17,7 +17,7 @@ func GetFeatureGatesFromDB(dbc *db.DB, release string, filterOpts *filter.Filter // at least once, filtering out tests that merely have carried-forward rows. tomorrow := civil.DateOf(time.Now().UTC()).AddDays(1) dr := DateRange{Start: tomorrow.AddDays(-8), End: tomorrow} - if err := resolveDateRanges(dbc, release, &dr); err != nil { + if err := ResolveDateRanges(dbc, release, &dr); err != nil { return nil, err } lookupEnd := dr.End.AddDays(-1) diff --git a/pkg/sippyserver/server.go b/pkg/sippyserver/server.go index 2ea35676ba..fd7c0e811f 100644 --- a/pkg/sippyserver/server.go +++ b/pkg/sippyserver/server.go @@ -28,6 +28,7 @@ import ( "github.com/openshift/sippy/pkg/api/jobartifacts" "github.com/openshift/sippy/pkg/apis/api/componentreport" "github.com/openshift/sippy/pkg/apis/api/componentreport/crview" + "github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts" "github.com/openshift/sippy/pkg/bigquery/bqlabel" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" @@ -1012,12 +1013,13 @@ func (s *Server) jsonComponentTestVariantsFromBigQuery(w http.ResponseWriter, re api.RespondWithJSON(http.StatusOK, w, outputs) } -func (s *Server) jsonJobVariantsFromBigQuery(w http.ResponseWriter, req *http.Request) { +func (s *Server) jsonJobVariants(w http.ResponseWriter, req *http.Request) { if s.crDataProvider == nil { failureResponse(w, http.StatusBadRequest, "job variants API is only available when a data provider is configured") return } - outputs, errs := componentreadiness.GetJobVariants(req.Context(), s.crDataProvider) + reqOptions := reqopts.RequestOptions{DataSource: param.SafeRead(req, "dataSource")} + outputs, errs := componentreadiness.GetJobVariants(req.Context(), s.crDataProvider, reqOptions) if len(errs) > 0 { log.Warningf("%d errors were encountered while querying job variants:", len(errs)) for _, err := range errs { @@ -1081,31 +1083,41 @@ func (s *Server) getRegressedTestsForRegressions(req *http.Request, regressions return result, nil } -// getComponentReportFromRequest creates a component report based on the HTTP request parameters -func (s *Server) getComponentReportFromRequest(req *http.Request) (componentreport.ComponentReport, error) { +// parseCRRequest validates the data provider, resolves variants and releases, +// and parses query parameters into RequestOptions. Shared by all CR handlers. +func (s *Server) parseCRRequest(req *http.Request) (reqopts.RequestOptions, []sippyv1.Release, []string, error) { if s.crDataProvider == nil { - return componentreport.ComponentReport{}, &api.ValidationError{ + return reqopts.RequestOptions{}, nil, nil, &api.ValidationError{ Message: "component report API is only available when a data provider is configured", } } - allJobVariants, errs := componentreadiness.GetJobVariants(req.Context(), s.crDataProvider) + variantReqOptions := reqopts.RequestOptions{DataSource: param.SafeRead(req, "dataSource")} + allJobVariants, errs := componentreadiness.GetJobVariants(req.Context(), s.crDataProvider, variantReqOptions) if len(errs) > 0 { - return componentreport.ComponentReport{}, fmt.Errorf("failed to get job variants") + return reqopts.RequestOptions{}, nil, nil, fmt.Errorf("failed to get job variants: %v", errs) } allReleases, err := s.getReleases(req.Context()) if err != nil { - return componentreport.ComponentReport{}, err + return reqopts.RequestOptions{}, nil, nil, err } options, warnings, err := utils.ParseComponentReportRequest(s.views.ComponentReadiness, allReleases, req, allJobVariants, s.crTimeRoundingFactor, s.crTimeRoundingOffset) + if err != nil { + return reqopts.RequestOptions{}, nil, nil, err + } + return options, allReleases, warnings, nil +} + +// getComponentReportFromRequest creates a component report based on the HTTP request parameters +func (s *Server) getComponentReportFromRequest(req *http.Request) (componentreport.ComponentReport, error) { + options, _, warnings, err := s.parseCRRequest(req) if err != nil { return componentreport.ComponentReport{}, err } - // This baseURL is used to generate links to test_details reports, which are frontend links baseURL := api.GetBaseFrontendURL(req) outputs, errs := componentreadiness.GetComponentReport( @@ -1119,13 +1131,12 @@ func (s *Server) getComponentReportFromRequest(req *http.Request) (componentrepo return componentreport.ComponentReport{}, fmt.Errorf("error querying component: %v", errs) } - // Add any warnings from parsing to the report outputs.Warnings = warnings return outputs, nil } -func (s *Server) jsonComponentReportFromBigQuery(w http.ResponseWriter, req *http.Request) { +func (s *Server) jsonComponentReport(w http.ResponseWriter, req *http.Request) { outputs, err := s.getComponentReportFromRequest(req) if err != nil { failureResponseWithError(w, "error generating component report", err) @@ -1135,29 +1146,13 @@ func (s *Server) jsonComponentReportFromBigQuery(w http.ResponseWriter, req *htt api.RespondWithJSON(http.StatusOK, w, outputs) } -func (s *Server) jsonComponentReportTestDetailsFromBigQuery(w http.ResponseWriter, req *http.Request) { - if s.crDataProvider == nil { - failureResponseWithError(w, "error querying component test details", - &api.ValidationError{Message: "component report API is only available when a data provider is configured"}) - return - } - allJobVariants, errs := componentreadiness.GetJobVariants(req.Context(), s.crDataProvider) - if len(errs) > 0 { - failureResponseWithError(w, "error querying component test details", fmt.Errorf("failed to get job variants")) - return - } - allReleases, err := s.getReleases(req.Context()) +func (s *Server) jsonComponentReportTestDetails(w http.ResponseWriter, req *http.Request) { + reqOptions, allReleases, _, err := s.parseCRRequest(req) if err != nil { failureResponseWithError(w, "error querying component test details", err) return } - reqOptions, _, err := utils.ParseComponentReportRequest(s.views.ComponentReadiness, allReleases, req, allJobVariants, s.crTimeRoundingFactor, s.crTimeRoundingOffset) - - if err != nil { - failureResponseWithError(w, "error querying component test details", err) - return - } baseURL := api.GetBaseFrontendURL(req) outputs, errs := componentreadiness.GetTestDetails(req.Context(), s.crDataProvider, s.db, reqOptions, allReleases, baseURL) if len(errs) > 0 { @@ -2535,9 +2530,9 @@ func (s *Server) Serve() { }, { EndpointPath: "/api/job_variants", - Description: "Reports all job variants defined in BigQuery", + Description: "Reports all job variants", Capabilities: []string{ComponentReadinessCapability}, - HandlerFunc: s.jsonJobVariantsFromBigQuery, + HandlerFunc: s.jsonJobVariants, }, { EndpointPath: "/api/pull_requests", @@ -2702,15 +2697,15 @@ func (s *Server) Serve() { }, { EndpointPath: "/api/component_readiness", - Description: "Reports component readiness from BigQuery", + Description: "Reports component readiness", Capabilities: []string{ComponentReadinessCapability}, - HandlerFunc: s.jsonComponentReportFromBigQuery, + HandlerFunc: s.jsonComponentReport, }, { EndpointPath: "/api/component_readiness/test_details", - Description: "Reports test details for component readiness from BigQuery", + Description: "Reports test details for component readiness", Capabilities: []string{ComponentReadinessCapability}, - HandlerFunc: s.jsonComponentReportTestDetailsFromBigQuery, + HandlerFunc: s.jsonComponentReportTestDetails, }, { EndpointPath: "/api/component_readiness/variants", diff --git a/pkg/util/param/param.go b/pkg/util/param/param.go index aea0d0aae2..e1e8d7e0b5 100644 --- a/pkg/util/param/param.go +++ b/pkg/util/param/param.go @@ -63,7 +63,8 @@ var paramRegexp = map[string]*regexp.Regexp{ "samplePRRepo": nameRegexp, "samplePRNumber": uintRegexp, "samplePayloadTag": nameRegexp, - "view": nameRegexp, // component readiness view name + "view": nameRegexp, // component readiness view name + "dataSource": regexp.MustCompile(`^(bigquery|postgres)$`), // data source for CR queries // jobartifacts params "prowJobRuns": regexp.MustCompile(`^\d+(,\d+)*$`), // comma-separated integers "pathGlob": nonEmptyRegex, // a glob can be anything diff --git a/sippy-ng/src/component_readiness/CompReadyUtils.jsx b/sippy-ng/src/component_readiness/CompReadyUtils.jsx index 01948c0230..0ab85458d8 100644 --- a/sippy-ng/src/component_readiness/CompReadyUtils.jsx +++ b/sippy-ng/src/component_readiness/CompReadyUtils.jsx @@ -400,6 +400,9 @@ export function makeRFC3339Time(aUrlStr) { // The api thinks that the null component is real and will filter accordingly // so omit it. retVal = retVal.replace(/&component=null/g, '') + + // The browser URL uses crDataSource, but the API expects dataSource. + retVal = retVal.replace(/crDataSource=/g, 'dataSource=') return retVal } @@ -461,6 +464,10 @@ export function getUpdatedUrlParts(vars) { //component: vars.component, } + if (vars.dataSource) { + valuesMap.crDataSource = vars.dataSource + } + if (vars.samplePROrg && vars.samplePRRepo && vars.samplePRNumber) { valuesMap.samplePROrg = vars.samplePROrg valuesMap.samplePRRepo = vars.samplePRRepo @@ -766,14 +773,18 @@ export function convertApiUrlToUiUrl(apiUrl) { if (apiIndex === -1) { return apiUrl } - const pathAndQuery = apiUrl.substring(apiIndex) + let pathAndQuery = apiUrl.substring(apiIndex) if (pathAndQuery.startsWith('/api/component_readiness/')) { - return pathAndQuery.replace( + pathAndQuery = pathAndQuery.replace( '/api/component_readiness/', '/sippy-ng/component_readiness/' ) + } else { + pathAndQuery = pathAndQuery.replace('/api/', '/sippy-ng/') } - return pathAndQuery.replace('/api/', '/sippy-ng/') + // The API uses dataSource, but the browser URL uses crDataSource. + pathAndQuery = pathAndQuery.replace(/dataSource=/g, 'crDataSource=') + return pathAndQuery } // Extracts the test_details link from HATEOAS links. Prefers the plain diff --git a/sippy-ng/src/component_readiness/CompReadyVars.jsx b/sippy-ng/src/component_readiness/CompReadyVars.jsx index 3bc2e9643f..b9627ca2da 100644 --- a/sippy-ng/src/component_readiness/CompReadyVars.jsx +++ b/sippy-ng/src/component_readiness/CompReadyVars.jsx @@ -128,11 +128,15 @@ export const CompReadyVarsProvider = ({ children }) => { includeVariant: ArrayParam, // variants selected for inclusion in the basis and sample (unless cross-compared) variantCrossCompare: ArrayParam, // variant groups (e.g. "Architecture") selected for cross-variant comparison compareVariant: ArrayParam, // individual variants (e.g. "Architecture:arm64") checked for cross-variant comparison + crDataSource: StringParam, // "postgres" to route CR queries to PG instead of BigQuery }) // Find the most recent GA releases const { defaultBaseRelease, defaultSampleRelease, getReleaseDate } = gaReleaseInfo(useContext(ReleasesContext)) + + const dataSource = params.crDataSource || '' + const days = 24 * 60 * 60 * 1000 const seconds = 1000 const now = new Date() @@ -141,10 +145,10 @@ export const CompReadyVarsProvider = ({ children }) => { const initialSampleStartTime = new Date(now.getTime() - 7 * days) const initialSampleEndTime = new Date(now.getTime()) - // Base is 28 days from the default base release's GA date + // Base is 30 days from the default base release's GA date // Match what the metrics uses in the api. const initialBaseStartTime = - getReleaseDate(defaultBaseRelease).getTime() - 27 * days + getReleaseDate(defaultBaseRelease).getTime() - 30 * days const initialBaseEndTime = getReleaseDate(defaultBaseRelease).getTime() + 1 * days - 1 * seconds @@ -225,6 +229,7 @@ export const CompReadyVarsProvider = ({ children }) => { const [flakeAsFailure, setFlakeAsFailure] = React.useState(false) const [includeMultiReleaseAnalysis, setIncludeMultiReleaseAnalysis] = React.useState(false) + /****************************************************************************** * Parameters that are used to refine the query as the user drills down into CR ****************************************************************************** */ @@ -388,6 +393,7 @@ export const CompReadyVarsProvider = ({ children }) => { includeVariant: convertVariantItemsToParam(includeVariantsCheckedItems), variantCrossCompare: variantCrossCompare, compareVariant: convertVariantItemsToParam(compareVariantsCheckedItems), + crDataSource: dataSource || undefined, }) } @@ -398,7 +404,11 @@ export const CompReadyVarsProvider = ({ children }) => { for (const key in params) { nonView[key] = undefined } - setParams({ ...nonView, view: view.name }) + setParams({ + ...nonView, + view: view.name, + crDataSource: dataSource || undefined, + }) updateVarsFromView(view.name, views) } @@ -450,7 +460,10 @@ export const CompReadyVarsProvider = ({ children }) => { } useEffect(() => { - const jobVariantsAPIURL = getJobVariantsAPIUrl() + const dsParam = dataSource + ? `?dataSource=${safeEncodeURIComponent(dataSource)}` + : '' + const jobVariantsAPIURL = getJobVariantsAPIUrl() + dsParam const viewsAPIURL = getComponentReadinessViewsAPIUrl() Promise.all([fetch(jobVariantsAPIURL), fetch(viewsAPIURL)]) .then(([variantsResp, viewsResp]) => { @@ -501,7 +514,7 @@ export const CompReadyVarsProvider = ({ children }) => { // Mark the attempt as finished whether successful or not. setIsLoaded(true) }) - }, []) + }, [dataSource]) const shouldLoadDefaultView = () => { // Attempt to decide if we should pre-select the default view, or if we were given params: @@ -620,6 +633,7 @@ export const CompReadyVarsProvider = ({ children }) => { setFlakeAsFailure, includeMultiReleaseAnalysis, setIncludeMultiReleaseAnalysis, + dataSource, component, capability, environment, diff --git a/sippy-ng/src/component_readiness/ComponentReadiness.jsx b/sippy-ng/src/component_readiness/ComponentReadiness.jsx index 05762de5d9..e56afdc4f5 100644 --- a/sippy-ng/src/component_readiness/ComponentReadiness.jsx +++ b/sippy-ng/src/component_readiness/ComponentReadiness.jsx @@ -24,7 +24,7 @@ import { StatusLegend, } from './CompReadyUtils' import { CompReadyVarsContext } from './CompReadyVars' -import { escapeRegex } from '../helpers' +import { escapeRegex, safeEncodeURIComponent } from '../helpers' import { grey } from '@mui/material/colors' import { makeStyles, useTheme } from '@mui/styles' import { @@ -282,6 +282,10 @@ export default function ComponentReadiness(_props) { if (varsContext.view != null && varsContext.view !== '') { apiCallStr += '?view=' + varsContext.view + if (varsContext.dataSource) { + apiCallStr += + '&dataSource=' + safeEncodeURIComponent(varsContext.dataSource) + } } else { apiCallStr += getUpdatedUrlParts(varsContext) } diff --git a/sippy-ng/src/component_readiness/ComponentReadinessIndicator.jsx b/sippy-ng/src/component_readiness/ComponentReadinessIndicator.jsx index 2abac6df2c..c0575f4ae3 100644 --- a/sippy-ng/src/component_readiness/ComponentReadinessIndicator.jsx +++ b/sippy-ng/src/component_readiness/ComponentReadinessIndicator.jsx @@ -11,7 +11,7 @@ import { useTheme, } from '@mui/material' import { COMPONENT_READINESS_THRESHOLDS } from '../constants' -import { getTestDetailsLink } from './CompReadyUtils' +import { convertApiUrlToUiUrl, getTestDetailsLink } from './CompReadyUtils' import { Link } from 'react-router-dom' import { makeStyles } from '@mui/styles' import { relativeTime } from '../helpers' @@ -256,18 +256,13 @@ export default function ComponentReadinessIndicator({ release }) { {regressions.recent.map((regression, index) => { // Get the test details URL from the regression links const viewName = `${release}-main` - let testDetailsUrl = null const rawUrl = getTestDetailsLink( regression.links, viewName ) - if (rawUrl) { - const apiIndex = rawUrl.indexOf('/api/') - if (apiIndex !== -1) { - const pathAfterApi = rawUrl.substring(apiIndex + 5) - testDetailsUrl = '/' + pathAfterApi - } - } + const testDetailsUrl = rawUrl + ? convertApiUrlToUiUrl(rawUrl) + : null return ( diff --git a/sippy-ng/src/component_readiness/RegressionRedirect.jsx b/sippy-ng/src/component_readiness/RegressionRedirect.jsx index b36fdaa029..2f10f82eff 100644 --- a/sippy-ng/src/component_readiness/RegressionRedirect.jsx +++ b/sippy-ng/src/component_readiness/RegressionRedirect.jsx @@ -1,4 +1,8 @@ -import { getRegressionAPIUrl, getTestDetailsLink } from './CompReadyUtils' +import { + convertApiUrlToUiUrl, + getRegressionAPIUrl, + getTestDetailsLink, +} from './CompReadyUtils' import { useNavigate, useParams } from 'react-router-dom' import Alert from '@mui/material/Alert' import React from 'react' @@ -37,24 +41,21 @@ export default function RegressionRedirect() { setError('No test details link available for this regression.') return } - const apiIndex = testDetailsUrl.indexOf('/api/') - if (apiIndex === -1) { - setError('Could not parse test details link.') - return - } - const pathAfterApi = testDetailsUrl.substring(apiIndex + 5) + const uiUrl = convertApiUrlToUiUrl(testDetailsUrl) let parsed try { - parsed = new URL(pathAfterApi, window.location.origin) + parsed = new URL(uiUrl, window.location.origin) } catch { setError('Could not parse test details link.') return } - if (!parsed.pathname.startsWith('/component_readiness/')) { + if (!parsed.pathname.startsWith('/sippy-ng/component_readiness/')) { setError('Unexpected redirect path.') return } - navigate(parsed.pathname + parsed.search, { replace: true }) + const navigatePath = + parsed.pathname.replace('/sippy-ng', '') + parsed.search + navigate(navigatePath, { replace: true }) }) .catch((err) => { if (err.name === 'AbortError') { diff --git a/sippy-ng/src/component_readiness/ReleaseSelector.jsx b/sippy-ng/src/component_readiness/ReleaseSelector.jsx index eafa9e7134..20bf7ad2d8 100644 --- a/sippy-ng/src/component_readiness/ReleaseSelector.jsx +++ b/sippy-ng/src/component_readiness/ReleaseSelector.jsx @@ -73,7 +73,7 @@ function ReleaseSelector(props) { const setGADate = () => { let start = new Date(versions[version]) setStartTime( - formatLongDate(start.setDate(start.getDate() - 27), dateFormat) + formatLongDate(start.setDate(start.getDate() - 30), dateFormat) ) setEndTime(formatLongDate(versions[version], dateEndFormat)) } @@ -337,7 +337,7 @@ function ReleaseSelector(props) { - + failures=1 + createCumulativeSummary(t, dbc, startMinus1, release, test1.ID, jobAWS.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, release, test1.ID, jobAWS.ID, suite.ID, 110, 98, 6) + + // test2 on jobGCP: runs=20, successes=15, flakes=2 -> failures=3 + createCumulativeSummary(t, dbc, startMinus1, release, test2.ID, jobGCP.ID, suite.ID, 50, 40, 3) + createCumulativeSummary(t, dbc, endMinus1, release, test2.ID, jobGCP.ID, suite.ID, 70, 55, 5) + + // test1 on jobAWS2: runs=5, successes=4, flakes=0 -> failures=1 + createCumulativeSummary(t, dbc, startMinus1, release, test1.ID, jobAWS2.ID, suite.ID, 30, 25, 2) + createCumulativeSummary(t, dbc, endMinus1, release, test1.ID, jobAWS2.ID, suite.ID, 35, 29, 2) + + // test3 on jobAWS: runs=8, successes=8, flakes=0 -> failures=0 (used for placeholder test) + createCumulativeSummary(t, dbc, startMinus1, release, test3.ID, jobAWS.ID, suite.ID, 40, 40, 0) + createCumulativeSummary(t, dbc, endMinus1, release, test3.ID, jobAWS.ID, suite.ID, 48, 48, 0) + + return crSeedData{ + vcAWS: vcAWS, vcGCP: vcGCP, vcAWS2: vcAWS2, + jobAWS: jobAWS, jobGCP: jobGCP, jobAWS2: jobAWS2, + test1: test1, test2: test2, test3: test3, + suite: suite, + tow1: tow1, tow2: tow2, tow3: tow3, + } +} + +func TestQuerySampleTestStatus(t *testing.T) { + t.Run("basic aggregation", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + seed := seedCRData(t, dbc) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + includeVariants := map[string][]string{ + "Platform": {"aws", "gcp"}, + "Network": {"ovn", "sdn"}, + } + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, includeVariants, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + require.NotEmpty(t, result) + + // test1 on jobAWS (Platform:aws, Network:ovn): runs=10, successes=8, flakes=1 + awsOvnKey := crtest.KeyWithVariants{ + TestID: seed.tow1.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[awsOvnKey.Encode()] + require.True(t, ok, "expected key %s in results", awsOvnKey.Encode()) + assert.Equal(t, 10, ts.TotalCount) + assert.Equal(t, 8, ts.SuccessCount) + assert.Equal(t, 1, ts.FlakeCount) + assert.Equal(t, "Storage", ts.Component) + + // test2 on jobGCP (Platform:gcp, Network:sdn): runs=20, successes=15, flakes=2 + gcpSdnKey := crtest.KeyWithVariants{ + TestID: seed.tow2.UniqueID, + Variants: map[string]string{"Platform": "gcp", "Network": "sdn"}, + } + ts2, ok := result[gcpSdnKey.Encode()] + require.True(t, ok, "expected key %s in results", gcpSdnKey.Encode()) + assert.Equal(t, 20, ts2.TotalCount) + assert.Equal(t, 15, ts2.SuccessCount) + assert.Equal(t, 2, ts2.FlakeCount) + assert.Equal(t, "Networking", ts2.Component) + + // test1 on jobAWS2 (Platform:aws, Network:sdn): runs=5, successes=4, flakes=0 + awsSdnKey := crtest.KeyWithVariants{ + TestID: seed.tow1.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "sdn"}, + } + ts3, ok := result[awsSdnKey.Encode()] + require.True(t, ok, "expected key %s in results", awsSdnKey.Encode()) + assert.Equal(t, 5, ts3.TotalCount) + assert.Equal(t, 4, ts3.SuccessCount) + assert.Equal(t, 0, ts3.FlakeCount) + }) + + t.Run("variant filter", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + seedCRData(t, dbc) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + includeVariants := map[string][]string{ + "Platform": {"aws"}, + } + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, includeVariants, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + for _, ts := range result { + if ts.Variants != nil { + platform, ok := ts.Variants["Platform"] + if ok { + assert.Equal(t, "aws", platform, "only aws variants should be returned") + } + } + } + + for key := range result { + assert.NotContains(t, key, "gcp", "gcp variant should be filtered out") + } + }) + + t.Run("variant grouping collapses same DBGroupBy dims", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + // Two VCs that share Platform:aws but differ on a non-grouped variant (Topology) + vc1 := createVariantCombination(t, dbc, []string{"Platform:aws", "Topology:ha"}) + vc2 := createVariantCombination(t, dbc, []string{"Platform:aws", "Topology:single"}) + + job1 := createProwJobWithVC(t, dbc, "periodic-e2e-aws-ha", release, vc1) + job2 := createProwJobWithVC(t, dbc, "periodic-e2e-aws-single", release, vc2) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] PVC test") + suite := createSuite(t, dbc, "openshift-tests") + createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:pvc", "Storage", []string{"PVC"}) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + + // job1: runs=10, successes=8, flakes=1 + createCumulativeSummary(t, dbc, startMinus1, release, test.ID, job1.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, release, test.ID, job1.ID, suite.ID, 110, 98, 6) + + // job2: runs=6, successes=5, flakes=0 + createCumulativeSummary(t, dbc, startMinus1, release, test.ID, job2.ID, suite.ID, 50, 45, 2) + createCumulativeSummary(t, dbc, endMinus1, release, test.ID, job2.ID, suite.ID, 56, 50, 2) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.VariantOption.DBGroupBy = sets.New[string]("Platform") + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + // Both jobs collapse to the same group (Platform:aws), aggregating counts + key := crtest.KeyWithVariants{ + TestID: "openshift-tests:pvc", + Variants: map[string]string{"Platform": "aws"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "expected collapsed group key") + assert.Equal(t, 16, ts.TotalCount, "runs should aggregate: 10 + 6") + assert.Equal(t, 13, ts.SuccessCount, "successes should aggregate: 8 + 5") + assert.Equal(t, 1, ts.FlakeCount, "flakes should aggregate: 1 + 0") + }) + + t.Run("minimum failure threshold", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + seed := seedCRData(t, dbc) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.AdvancedOption.MinimumFailure = 2 + includeVariants := map[string][]string{ + "Platform": {"aws", "gcp"}, + "Network": {"ovn", "sdn"}, + } + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, includeVariants, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + // test2/jobGCP has 3 failures (>= 2), should appear via failure query + gcpKey := crtest.KeyWithVariants{ + TestID: seed.tow2.UniqueID, + Variants: map[string]string{"Platform": "gcp", "Network": "sdn"}, + } + ts, ok := result[gcpKey.Encode()] + require.True(t, ok, "test2/gcp with 3 failures should pass MinimumFailure=2") + assert.Equal(t, 20, ts.TotalCount) + + // test1/jobAWS has 1 failure (< 2), should NOT appear in failure results + // but a placeholder for its component should exist + awsOvnKey := crtest.KeyWithVariants{ + TestID: seed.tow1.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + _, hasDirect := result[awsOvnKey.Encode()] + assert.False(t, hasDirect, "test1/aws-ovn with 1 failure should not pass MinimumFailure=2 filter") + }) + + t.Run("no data for release", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.99" + + // Create at least one VC so the query doesn't short-circuit on empty variantLookup + vc := createVariantCombination(t, dbc, []string{"Platform:aws"}) + createProwJobWithVC(t, dbc, "periodic-e2e-aws-empty", release, vc) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + assert.Empty(t, result) + }) + + t.Run("RequestedVariants drill-down", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + seed := seedCRData(t, dbc) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.TestIDOptions = []reqopts.TestIdentification{{ + RequestedVariants: map[string]string{"Platform": "aws", "Network": "ovn"}, + }} + includeVariants := map[string][]string{ + "Platform": {"aws", "gcp"}, + "Network": {"ovn", "sdn"}, + } + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, includeVariants, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + // Only aws+ovn VC should match + for _, ts := range result { + if ts.Variants != nil { + if p, ok := ts.Variants["Platform"]; ok { + assert.Equal(t, "aws", p) + } + if n, ok := ts.Variants["Network"]; ok { + assert.Equal(t, "ovn", n) + } + } + } + + // test1 on aws+ovn should be present + awsOvnKey := crtest.KeyWithVariants{ + TestID: seed.tow1.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + _, ok := result[awsOvnKey.Encode()] + assert.True(t, ok, "test1 on aws+ovn should be present with RequestedVariants drill-down") + }) + + t.Run("TestID drill-down", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + seed := seedCRData(t, dbc) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.TestIDOptions = []reqopts.TestIdentification{{ + TestID: seed.tow1.UniqueID, // "openshift-tests:aaa" + }} + includeVariants := map[string][]string{ + "Platform": {"aws", "gcp"}, + "Network": {"ovn", "sdn"}, + } + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, includeVariants, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + // Only test1 entries should appear (excluding grid placeholders) + for _, ts := range result { + if isPlaceholderKey(ts.TestID) { + continue + } + assert.Equal(t, seed.tow1.UniqueID, ts.TestID, + "only test1 entries should appear with TestID drill-down") + } + }) + + t.Run("Capability drill-down", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + seedCRData(t, dbc) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.TestIDOptions = []reqopts.TestIdentification{{ + Capability: "RBAC", + }} + includeVariants := map[string][]string{ + "Platform": {"aws", "gcp"}, + "Network": {"ovn", "sdn"}, + } + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, includeVariants, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + // test3 has RBAC capability but 0 failures, so with MinimumFailure=1 + // it only appears as a placeholder. Check that no non-RBAC tests are returned. + for _, ts := range result { + if ts.TestID != "" && !isPlaceholderKey(ts.TestID) { + assert.Contains(t, ts.Capabilities, "RBAC", + "only tests with RBAC capability should appear") + } + } + }) + + t.Run("release isolation", func(t *testing.T) { + dbc := crTestDB(t) + sampleRelease := "4.17" + distractorRelease := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + sampleJob := createProwJobWithVC(t, dbc, "periodic-e2e-aws-sample", sampleRelease, vc) + distractorJob := createProwJobWithVC(t, dbc, "periodic-e2e-aws-distractor", distractorRelease, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] release isolation") + suite := createSuite(t, dbc, "openshift-tests-ri") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:ri-test", "Storage", []string{"PVC"}) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + + // Sample release: runs=10, successes=8, flakes=1 + createCumulativeSummary(t, dbc, startMinus1, sampleRelease, test.ID, sampleJob.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, sampleRelease, test.ID, sampleJob.ID, suite.ID, 110, 98, 6) + + // Distractor release: much larger counts that would be obvious if leaked + createCumulativeSummary(t, dbc, startMinus1, distractorRelease, test.ID, distractorJob.ID, suite.ID, 1000, 900, 50) + createCumulativeSummary(t, dbc, endMinus1, distractorRelease, test.ID, distractorJob.ID, suite.ID, 2000, 1800, 100) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(sampleRelease) + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: tow.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "expected result for sample release") + assert.Equal(t, 10, ts.TotalCount, "should only include data from the queried release, not the distractor") + }) + + t.Run("obsolete ownership excluded", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-e2e-aws-obs", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] obsolete test") + suite := createSuite(t, dbc, "openshift-tests-obs") + + obsOwnership := models.TestOwnership{ + TestID: test.ID, + SuiteID: &suite.ID, + UniqueID: "openshift-tests:obsolete", + Name: "openshift-tests:obsolete", + Component: "Storage", + Capabilities: pq.StringArray{"PVC"}, + StaffApprovedObsolete: true, + } + require.NoError(t, dbc.DB.Create(&obsOwnership).Error) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + createCumulativeSummary(t, dbc, startMinus1, release, test.ID, job.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, release, test.ID, job.ID, suite.ID, 110, 98, 6) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + for _, ts := range result { + assert.NotEqual(t, "openshift-tests:obsolete", ts.TestID, + "obsolete test ownership should be excluded from results") + } + }) + + t.Run("suite isolation", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-e2e-aws-suite", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] multi-suite test") + suiteA := createSuite(t, dbc, "suite-a") + suiteB := createSuite(t, dbc, "suite-b") + + towA := createTestOwnership(t, dbc, test.ID, &suiteA.ID, "suite-a:multi", "Storage", []string{"PVC"}) + towB := createTestOwnership(t, dbc, test.ID, &suiteB.ID, "suite-b:multi", "Storage", []string{"PVC"}) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + + // suite-a: runs=10, successes=8, flakes=1 + createCumulativeSummary(t, dbc, startMinus1, release, test.ID, job.ID, suiteA.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, release, test.ID, job.ID, suiteA.ID, 110, 98, 6) + + // suite-b: runs=20, successes=15, flakes=2 + createCumulativeSummary(t, dbc, startMinus1, release, test.ID, job.ID, suiteB.ID, 50, 40, 3) + createCumulativeSummary(t, dbc, endMinus1, release, test.ID, job.ID, suiteB.ID, 70, 55, 5) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + keyA := crtest.KeyWithVariants{ + TestID: towA.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + keyB := crtest.KeyWithVariants{ + TestID: towB.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + + tsA, ok := result[keyA.Encode()] + require.True(t, ok, "suite-a result should be present") + assert.Equal(t, 10, tsA.TotalCount) + assert.Equal(t, "suite-a", tsA.TestSuite) + + tsB, ok := result[keyB.Encode()] + require.True(t, ok, "suite-b result should be present") + assert.Equal(t, 20, tsB.TotalCount) + assert.Equal(t, "suite-b", tsB.TestSuite) + }) + + t.Run("sample query uses compare-side variants in cross-compare view", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vcHA := createVariantCombination(t, dbc, []string{"Platform:aws", "Topology:ha"}) + vcSingle := createVariantCombination(t, dbc, []string{"Platform:aws", "Topology:single"}) + + jobHA := createProwJobWithVC(t, dbc, "periodic-e2e-aws-ha", release, vcHA) + jobSingle := createProwJobWithVC(t, dbc, "periodic-e2e-aws-single", release, vcSingle) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] cross-compare test") + suite := createSuite(t, dbc, "openshift-tests-cc") + createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:cc-test", "Storage", []string{"PVC"}) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + + // HA: runs=10, successes=8, flakes=1 + createCumulativeSummary(t, dbc, startMinus1, release, test.ID, jobHA.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, release, test.ID, jobHA.ID, suite.ID, 110, 98, 6) + + // Single: runs=20, successes=15, flakes=2 + createCumulativeSummary(t, dbc, startMinus1, release, test.ID, jobSingle.ID, suite.ID, 50, 40, 3) + createCumulativeSummary(t, dbc, endMinus1, release, test.ID, jobSingle.ID, suite.ID, 70, 55, 5) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.VariantOption.DBGroupBy = sets.New[string]("Platform", "Topology") + opts.VariantOption.ColumnGroupBy = sets.New[string]("Platform") + // Base-side: Topology:ha, but cross-compare swaps to Topology:single for sample + opts.VariantOption.VariantCrossCompare = []string{"Topology"} + opts.VariantOption.CompareVariants = map[string][]string{"Topology": {"single"}} + + // includeVariants has the base-side value + includeVariants := map[string][]string{ + "Platform": {"aws"}, + "Topology": {"ha"}, + } + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, includeVariants, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + nonPlaceholders := filterPlaceholders(result) + require.NotEmpty(t, nonPlaceholders, "should return results for cross-compare") + for _, ts := range nonPlaceholders { + assert.Equal(t, "single", ts.Variants["Topology"], + "should return sample-side (single) data, not base-side (ha)") + assert.Equal(t, 20, ts.TotalCount) + } + }) + + t.Run("most recent failure date is included in results", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-e2e-aws-lf", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] last failure test") + suite := createSuite(t, dbc, "openshift-tests-lf") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:lf-test", "Storage", []string{"PVC"}) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + + // runs=10, successes=8, flakes=1 -> failures=1 + createCumulativeSummary(t, dbc, startMinus1, release, test.ID, job.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, release, test.ID, job.ID, suite.ID, 110, 98, 6) + + failTime1 := time.Date(2024, 6, 5, 12, 0, 0, 0, time.UTC) + failTime2 := time.Date(2024, 6, 10, 18, 0, 0, 0, time.UTC) + run1 := createProwJobRunForCR(t, dbc, job.ID, release, failTime1) + run2 := createProwJobRunForCR(t, dbc, job.ID, release, failTime2) + createProwJobRunTest(t, dbc, run1.ID, job.ID, test.ID, &suite.ID, 12, release, failTime1) // failure + createProwJobRunTest(t, dbc, run2.ID, job.ID, test.ID, &suite.ID, 12, release, failTime2) // later failure + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: tow.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "expected result for test with failures") + assert.False(t, ts.LastFailure.IsZero(), "LastFailure should be populated when failures exist") + assert.True(t, ts.LastFailure.Equal(failTime2), + "LastFailure should be the most recent failure: got %v, want %v", ts.LastFailure, failTime2) + }) + + t.Run("results limited to available data when requested range exceeds it", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-e2e-aws-clamp", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] clamping test") + suite := createSuite(t, dbc, "openshift-tests-clamp") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:clamp-test", "Storage", []string{"PVC"}) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + // Data only goes up to June 10, not June 14 (which the default range expects) + clampedEnd := civil.Date{Year: 2024, Month: 6, Day: 10} + + createCumulativeSummary(t, dbc, startMinus1, release, test.ID, job.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, clampedEnd, release, test.ID, job.ID, suite.ID, 107, 95, 6) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: tow.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "expected result even with clamped date range") + assert.Equal(t, 7, ts.TotalCount, "counts should reflect only the available data period") + }) + + t.Run("tests without a suite appear with empty suite name", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-e2e-aws-nosuite", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] no suite test") + tow := createTestOwnership(t, dbc, test.ID, nil, "openshift-tests:nosuite-test", "Storage", []string{"PVC"}) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + + createCumulativeSummary(t, dbc, startMinus1, release, test.ID, job.ID, 0, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, release, test.ID, job.ID, 0, 110, 98, 6) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: tow.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "test with NULL suite_id should appear in results") + assert.Equal(t, 10, ts.TotalCount) + assert.Equal(t, "", ts.TestSuite, "suite name should be empty when test has no suite") + }) + + t.Run("test with no data before reporting period returns full period counts", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-e2e-aws-coalesce", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] coalesce test") + suite := createSuite(t, dbc, "openshift-tests-co") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:coalesce-test", "Storage", []string{"PVC"}) + + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + createCumulativeSummary(t, dbc, endMinus1, release, test.ID, job.ID, suite.ID, 50, 40, 3) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: tow.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "test should appear even with no prior data") + assert.Equal(t, 50, ts.TotalCount) + assert.Equal(t, 40, ts.SuccessCount) + assert.Equal(t, 3, ts.FlakeCount) + }) + + t.Run("failure counts preserved when test also qualifies as placeholder", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-e2e-aws-merge", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] merge test") + suite := createSuite(t, dbc, "openshift-tests-merge") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:merge-test", "Storage", []string{"PVC"}) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + + // runs=10, successes=7, flakes=1 -> failures=2 + createCumulativeSummary(t, dbc, startMinus1, release, test.ID, job.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, release, test.ID, job.ID, suite.ID, 110, 97, 6) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.AdvancedOption.MinimumFailure = 1 + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: tow.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "test should appear via failure query") + assert.Equal(t, 10, ts.TotalCount, "failure counts should be preserved, not zeroed by placeholder") + assert.Equal(t, 7, ts.SuccessCount) + assert.Equal(t, 1, ts.FlakeCount) + }) + + t.Run("combined drill-down", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + seed := seedCRData(t, dbc) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.TestIDOptions = []reqopts.TestIdentification{{ + TestID: seed.tow1.UniqueID, + Capability: "PersistentVolumes", + RequestedVariants: map[string]string{"Platform": "aws", "Network": "ovn"}, + }} + includeVariants := map[string][]string{ + "Platform": {"aws", "gcp"}, + "Network": {"ovn", "sdn"}, + } + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, includeVariants, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + // Should get exactly test1 on aws+ovn + nonPlaceholder := filterPlaceholders(result) + require.Len(t, nonPlaceholder, 1, "combined drill-down should yield exactly one non-placeholder result") + + ts := nonPlaceholder[0] + assert.Equal(t, seed.tow1.UniqueID, ts.TestID) + assert.Equal(t, "aws", ts.Variants["Platform"]) + assert.Equal(t, "ovn", ts.Variants["Network"]) + assert.Equal(t, 10, ts.TotalCount) + }) +} + +func TestQueryBaseTestStatus_PrefixSum(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + seed := seedCRData(t, dbc) + + // Seed base period data: BaseRelease = [2024-05-15, 2024-06-01) + // QueryBaseTestStatus constructs: baseRange = {Start:2024-05-15, End:2024-06-02} + // lookupStart = Start.AddDays(-1) = 2024-05-14 + // lookupEnd = End.AddDays(-1) = 2024-06-01 + baseLookupStart := civil.Date{Year: 2024, Month: 5, Day: 14} + baseLookupEnd := civil.Date{Year: 2024, Month: 6, Day: 1} + + createCumulativeSummary(t, dbc, baseLookupStart, release, seed.test1.ID, seed.jobAWS.ID, seed.suite.ID, 80, 75, 3) + createCumulativeSummary(t, dbc, baseLookupEnd, release, seed.test1.ID, seed.jobAWS.ID, seed.suite.ID, 100, 90, 5) + // base_runs = 100-80=20, base_successes = 90-75=15, base_flakes = 5-3=2 + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + includeVariants := map[string][]string{ + "Platform": {"aws"}, + "Network": {"ovn"}, + } + opts.VariantOption.IncludeVariants = includeVariants + + result, errs := provider.QueryBaseTestStatus(context.Background(), opts) + require.Empty(t, errs) + + awsOvnKey := crtest.KeyWithVariants{ + TestID: seed.tow1.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[awsOvnKey.Encode()] + require.True(t, ok, "expected base test status for test1 on aws+ovn") + assert.Equal(t, 20, ts.TotalCount) + assert.Equal(t, 15, ts.SuccessCount) + assert.Equal(t, 2, ts.FlakeCount) +} + +func TestQueryTestStatus_DifferentBaseAndSampleReleases(t *testing.T) { + dbc := crTestDB(t) + baseRelease := "4.16" + sampleRelease := "4.17" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + baseJob := createProwJobWithVC(t, dbc, "periodic-e2e-aws-base", baseRelease, vc) + sampleJob := createProwJobWithVC(t, dbc, "periodic-e2e-aws-sample", sampleRelease, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] cross-release test") + suite := createSuite(t, dbc, "openshift-tests-xr") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:xr-test", "Storage", []string{"PVC"}) + + // Base period: [2024-05-15, 2024-06-01) -> lookupStart=2024-05-14, lookupEnd=2024-06-01 + // runs=20, successes=15, flakes=2 + baseLookupStart := civil.Date{Year: 2024, Month: 5, Day: 14} + baseLookupEnd := civil.Date{Year: 2024, Month: 6, Day: 1} + createCumulativeSummary(t, dbc, baseLookupStart, baseRelease, test.ID, baseJob.ID, suite.ID, 80, 75, 3) + createCumulativeSummary(t, dbc, baseLookupEnd, baseRelease, test.ID, baseJob.ID, suite.ID, 100, 90, 5) + + // Sample period: [2024-06-01, 2024-06-15) -> lookupStart=2024-05-31, lookupEnd=2024-06-14 + // runs=10, successes=8, flakes=1 + sampleLookupStart := civil.Date{Year: 2024, Month: 5, Day: 31} + sampleLookupEnd := civil.Date{Year: 2024, Month: 6, Day: 14} + createCumulativeSummary(t, dbc, sampleLookupStart, sampleRelease, test.ID, sampleJob.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, sampleLookupEnd, sampleRelease, test.ID, sampleJob.ID, suite.ID, 110, 98, 6) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := reqopts.RequestOptions{ + BaseRelease: reqopts.Release{ + Name: baseRelease, + Start: time.Date(2024, 5, 15, 0, 0, 0, 0, time.UTC), + End: time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC), + }, + SampleRelease: reqopts.Release{ + Name: sampleRelease, + Start: time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC), + End: time.Date(2024, 6, 15, 0, 0, 0, 0, time.UTC), + }, + VariantOption: reqopts.Variants{ + DBGroupBy: sets.New[string]("Platform", "Network"), + ColumnGroupBy: sets.New[string]("Platform"), + IncludeVariants: map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + }, + AdvancedOption: reqopts.Advanced{ + MinimumFailure: 1, + }, + } + + key := crtest.KeyWithVariants{ + TestID: tow.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + + baseResult, errs := provider.QueryBaseTestStatus(context.Background(), opts) + require.Empty(t, errs) + + baseTS, ok := baseResult[key.Encode()] + require.True(t, ok, "expected base result") + assert.Equal(t, 20, baseTS.TotalCount, "base should reflect baseRelease data only") + assert.Equal(t, 15, baseTS.SuccessCount) + + sampleResult, errs := provider.QuerySampleTestStatus(context.Background(), opts, + opts.VariantOption.IncludeVariants, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + sampleTS, ok := sampleResult[key.Encode()] + require.True(t, ok, "expected sample result") + assert.Equal(t, 10, sampleTS.TotalCount, "sample should reflect sampleRelease data only") + assert.Equal(t, 8, sampleTS.SuccessCount) +} + +func TestQueryBaseTestStatus_GA(t *testing.T) { + dbc := crTestDB(t) + release := "4.15" + + // GA date in the past + gaDate := time.Date(2024, 3, 1, 0, 0, 0, 0, time.UTC) + createReleaseDefinition(t, dbc, release, &gaDate) + + gaCivil := civil.DateOf(gaDate) + gaEnd := utils.GAWindowEnd(gaCivil) + windowDays := 30 + gaStart := gaCivil.AddDays(-windowDays) + + vcAWS := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + jobAWS := createProwJobWithVC(t, dbc, "periodic-ga-aws-ovn", release, vcAWS) + test := createTest(t, dbc, "openshift-tests:[sig-storage] GA PVC test") + suite := createSuite(t, dbc, "openshift-tests-ga") + createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:ga-pvc", "Storage", []string{"PVC"}) + + // GA raw data: 50 runs, 45 passes, 2 flakes + createGARawData(t, dbc, release, windowDays, test.ID, jobAWS.ID, suite.ID, 50, 45, 2) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.BaseRelease = reqopts.Release{ + Name: release, + Start: gaStart.In(time.UTC), + End: gaEnd.AddDays(-1).In(time.UTC), + } + opts.VariantOption.IncludeVariants = map[string][]string{ + "Platform": {"aws"}, + "Network": {"ovn"}, + } + + result, errs := provider.QueryBaseTestStatus(context.Background(), opts) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: "openshift-tests:ga-pvc", + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "expected GA base test status") + assert.Equal(t, 50, ts.TotalCount) + assert.Equal(t, 45, ts.SuccessCount) + assert.Equal(t, 2, ts.FlakeCount) + + t.Run("GA drill-down with TestID", func(t *testing.T) { + opts2 := opts + opts2.TestIDOptions = []reqopts.TestIdentification{{ + TestID: "openshift-tests:ga-pvc", + }} + + result2, errs := provider.QueryBaseTestStatus(context.Background(), opts2) + require.Empty(t, errs) + + ts2, ok := result2[key.Encode()] + require.True(t, ok, "TestID drill-down should return GA data for matching test") + assert.Equal(t, 50, ts2.TotalCount) + }) + + t.Run("GA drill-down with non-matching TestID", func(t *testing.T) { + opts3 := opts + opts3.TestIDOptions = []reqopts.TestIdentification{{ + TestID: "openshift-tests:nonexistent", + }} + + result3, errs := provider.QueryBaseTestStatus(context.Background(), opts3) + require.Empty(t, errs) + + nonPlaceholders := filterPlaceholders(result3) + assert.Empty(t, nonPlaceholders, "non-matching TestID should yield no results") + }) + + t.Run("release without GA date uses cumulative data", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.14" + + createReleaseDefinition(t, dbc, release, nil) + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-ga-nil-aws", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] GA nil date test") + suite := createSuite(t, dbc, "openshift-tests-ga-nil") + createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:ga-nil", "Storage", []string{"PVC"}) + + // Seed prefix-sum data for base period + baseLookupStart := civil.Date{Year: 2024, Month: 5, Day: 14} + baseLookupEnd := civil.Date{Year: 2024, Month: 6, Day: 1} + createCumulativeSummary(t, dbc, baseLookupStart, release, test.ID, job.ID, suite.ID, 80, 75, 3) + createCumulativeSummary(t, dbc, baseLookupEnd, release, test.ID, job.ID, suite.ID, 100, 90, 5) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.VariantOption.IncludeVariants = map[string][]string{ + "Platform": {"aws"}, + "Network": {"ovn"}, + } + + result, errs := provider.QueryBaseTestStatus(context.Background(), opts) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: "openshift-tests:ga-nil", + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "release without GA date should still return data") + assert.Equal(t, 20, ts.TotalCount) + }) + + t.Run("release with future GA date uses cumulative data", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.20" + + futureGA := time.Date(2030, 1, 1, 0, 0, 0, 0, time.UTC) + createReleaseDefinition(t, dbc, release, &futureGA) + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-ga-future-aws", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] GA future date test") + suite := createSuite(t, dbc, "openshift-tests-ga-future") + createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:ga-future", "Storage", []string{"PVC"}) + + baseLookupStart := civil.Date{Year: 2024, Month: 5, Day: 14} + baseLookupEnd := civil.Date{Year: 2024, Month: 6, Day: 1} + createCumulativeSummary(t, dbc, baseLookupStart, release, test.ID, job.ID, suite.ID, 80, 75, 3) + createCumulativeSummary(t, dbc, baseLookupEnd, release, test.ID, job.ID, suite.ID, 100, 90, 5) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.VariantOption.IncludeVariants = map[string][]string{ + "Platform": {"aws"}, + "Network": {"ovn"}, + } + + result, errs := provider.QueryBaseTestStatus(context.Background(), opts) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: "openshift-tests:ga-future", + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "release with future GA date should still return data") + assert.Equal(t, 20, ts.TotalCount) + }) + + t.Run("release with non-standard GA window duration uses cumulative data", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.13" + + // Set GADate such that the window is 15 days (not in GAWindows: [1, 30, 90]) + gaDate := time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC) + createReleaseDefinition(t, dbc, release, &gaDate) + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-ga-nonstandard-aws", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] GA nonstandard window test") + suite := createSuite(t, dbc, "openshift-tests-ga-ns") + createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:ga-nonstandard", "Storage", []string{"PVC"}) + + // Base period: May 17 to June 2 (= GAWindowEnd for June 1) + // windowDays = gaCivil.DaysSince(Start) = June1 - May17 = 15 (not in GAWindows) + baseLookupStart := civil.Date{Year: 2024, Month: 5, Day: 16} + baseLookupEnd := civil.Date{Year: 2024, Month: 6, Day: 1} + createCumulativeSummary(t, dbc, baseLookupStart, release, test.ID, job.ID, suite.ID, 80, 75, 3) + createCumulativeSummary(t, dbc, baseLookupEnd, release, test.ID, job.ID, suite.ID, 95, 88, 4) + + provider := postgres.NewPostgresProvider(dbc, nil) + gaCivil := civil.DateOf(gaDate) + gaEnd := utils.GAWindowEnd(gaCivil) + opts := defaultReqOptions(release) + opts.BaseRelease = reqopts.Release{ + Name: release, + Start: time.Date(2024, 5, 17, 0, 0, 0, 0, time.UTC), + End: gaEnd.AddDays(-1).In(time.UTC), + } + opts.VariantOption.IncludeVariants = map[string][]string{ + "Platform": {"aws"}, + "Network": {"ovn"}, + } + + result, errs := provider.QueryBaseTestStatus(context.Background(), opts) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: "openshift-tests:ga-nonstandard", + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "release with non-standard window should still return data") + assert.Equal(t, 15, ts.TotalCount) + }) +} + +func TestQuerySampleJobRunTestStatus(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vcAWS := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + vcGCP := createVariantCombination(t, dbc, []string{"Platform:gcp", "Network:sdn"}) + jobAWS := createProwJobWithVC(t, dbc, "periodic-jr-aws", release, vcAWS) + jobGCP := createProwJobWithVC(t, dbc, "periodic-jr-gcp", release, vcGCP) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] JR PVC test") + suite := createSuite(t, dbc, "openshift-tests-jr") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:jr-pvc", "Storage", []string{"PVC"}) + + ts1 := time.Date(2024, 6, 5, 12, 0, 0, 0, time.UTC) + ts2 := time.Date(2024, 6, 6, 12, 0, 0, 0, time.UTC) + ts3 := time.Date(2024, 6, 7, 12, 0, 0, 0, time.UTC) + + run1 := createProwJobRunForCR(t, dbc, jobAWS.ID, release, ts1) + run2 := createProwJobRunForCR(t, dbc, jobAWS.ID, release, ts2) + run3 := createProwJobRunForCR(t, dbc, jobGCP.ID, release, ts3) + + // status 1 = pass, 12 = fail, 13 = flake + createProwJobRunTest(t, dbc, run1.ID, jobAWS.ID, test.ID, &suite.ID, 1, release, ts1) + createProwJobRunTest(t, dbc, run2.ID, jobAWS.ID, test.ID, &suite.ID, 12, release, ts2) + createProwJobRunTest(t, dbc, run3.ID, jobGCP.ID, test.ID, &suite.ID, 13, release, ts3) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.TestIDOptions = []reqopts.TestIdentification{{ + TestID: tow.UniqueID, + }} + + t.Run("returns per-job-run test results", func(t *testing.T) { + result, errs := provider.QuerySampleJobRunTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws", "gcp"}, "Network": {"ovn", "sdn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + totalRows := 0 + for _, rows := range result { + totalRows += len(rows) + } + assert.Equal(t, 3, totalRows, "should have 3 job run test rows") + + // Verify that all rows reference the correct test + for _, rows := range result { + for _, row := range rows { + assert.Equal(t, tow.UniqueID, row.TestKey.TestID) + } + } + }) + + t.Run("RequestedVariants filtering", func(t *testing.T) { + opts2 := opts + opts2.TestIDOptions = []reqopts.TestIdentification{{ + TestID: tow.UniqueID, + RequestedVariants: map[string]string{"Platform": "aws"}, + }} + + result, errs := provider.QuerySampleJobRunTestStatus(context.Background(), opts2, + map[string][]string{"Platform": {"aws", "gcp"}, "Network": {"ovn", "sdn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + totalRows := 0 + for _, rows := range result { + totalRows += len(rows) + } + assert.Equal(t, 2, totalRows, "RequestedVariants=aws should exclude gcp run") + }) + + t.Run("IncludeVariants filtering", func(t *testing.T) { + result, errs := provider.QuerySampleJobRunTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"gcp"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + totalRows := 0 + for _, rows := range result { + totalRows += len(rows) + } + assert.Equal(t, 1, totalRows, "IncludeVariants=gcp should only return gcp run") + }) + + t.Run("infrastructure failure runs excluded from results", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-jr-infra", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] infra exclusion test") + suite := createSuite(t, dbc, "openshift-tests-infra") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:infra-test", "Storage", []string{"PVC"}) + + normalTS := time.Date(2024, 6, 5, 12, 0, 0, 0, time.UTC) + infraTS := time.Date(2024, 6, 6, 12, 0, 0, 0, time.UTC) + + normalRun := createProwJobRunForCR(t, dbc, job.ID, release, normalTS) + infraRun := createProwJobRunForCR(t, dbc, job.ID, release, infraTS) + setJobRunLabels(t, dbc, infraRun.ID, []string{"InfraFailure"}) + + createProwJobRunTest(t, dbc, normalRun.ID, job.ID, test.ID, &suite.ID, 1, release, normalTS) + createProwJobRunTest(t, dbc, infraRun.ID, job.ID, test.ID, &suite.ID, 12, release, infraTS) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.TestIDOptions = []reqopts.TestIdentification{{TestID: tow.UniqueID}} + + result, errs := provider.QuerySampleJobRunTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + totalRows := 0 + for _, rows := range result { + totalRows += len(rows) + } + assert.Equal(t, 1, totalRows, "infrastructure failure run should be excluded") + }) + + t.Run("Jira component ID included in test details", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-jr-jira", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] jira component test") + suite := createSuite(t, dbc, "openshift-tests-jira") + tow := createTestOwnershipFull(t, dbc, test.ID, &suite.ID, "openshift-tests:jira-test", "Storage", []string{"PVC"}, uintPtr(42)) + + ts := time.Date(2024, 6, 5, 12, 0, 0, 0, time.UTC) + run := createProwJobRunForCR(t, dbc, job.ID, release, ts) + createProwJobRunTest(t, dbc, run.ID, job.ID, test.ID, &suite.ID, 1, release, ts) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.TestIDOptions = []reqopts.TestIdentification{{TestID: tow.UniqueID}} + + result, errs := provider.QuerySampleJobRunTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + for _, rows := range result { + for _, row := range rows { + require.NotNil(t, row.JiraComponentID, "JiraComponentID should be populated") + expected := new(big.Rat).SetUint64(42) + assert.Equal(t, 0, row.JiraComponentID.Cmp(expected), + "JiraComponentID should be big.Rat(42), got %s", row.JiraComponentID.RatString()) + } + } + }) + + t.Run("release isolation", func(t *testing.T) { + dbc := crTestDB(t) + sampleRelease := "4.17" + distractorRelease := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + sampleJob := createProwJobWithVC(t, dbc, "periodic-jr-sample", sampleRelease, vc) + distractorJob := createProwJobWithVC(t, dbc, "periodic-jr-distractor", distractorRelease, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] JR release isolation") + suite := createSuite(t, dbc, "openshift-tests-jr-ri") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:jr-ri", "Storage", []string{"PVC"}) + + ts1 := time.Date(2024, 6, 5, 12, 0, 0, 0, time.UTC) + ts2 := time.Date(2024, 6, 6, 12, 0, 0, 0, time.UTC) + + sampleRun := createProwJobRunForCR(t, dbc, sampleJob.ID, sampleRelease, ts1) + distractorRun := createProwJobRunForCR(t, dbc, distractorJob.ID, distractorRelease, ts2) + + createProwJobRunTest(t, dbc, sampleRun.ID, sampleJob.ID, test.ID, &suite.ID, 1, sampleRelease, ts1) + createProwJobRunTest(t, dbc, distractorRun.ID, distractorJob.ID, test.ID, &suite.ID, 12, distractorRelease, ts2) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(sampleRelease) + opts.TestIDOptions = []reqopts.TestIdentification{{TestID: tow.UniqueID}} + + result, errs := provider.QuerySampleJobRunTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + totalRows := 0 + for _, rows := range result { + totalRows += len(rows) + } + assert.Equal(t, 1, totalRows, "should only include job runs from the queried release") + }) +} + +func TestQueryBaseJobRunTestStatus(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vcAWS := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + jobAWS := createProwJobWithVC(t, dbc, "periodic-base-jr-aws", release, vcAWS) + + test := createTest(t, dbc, "openshift-tests:[sig-network] Base JR test") + suite := createSuite(t, dbc, "openshift-tests-base-jr") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:base-jr", "Networking", []string{"Services"}) + + ts1 := time.Date(2024, 5, 20, 12, 0, 0, 0, time.UTC) + ts2 := time.Date(2024, 5, 25, 12, 0, 0, 0, time.UTC) + + run1 := createProwJobRunForCR(t, dbc, jobAWS.ID, release, ts1) + run2 := createProwJobRunForCR(t, dbc, jobAWS.ID, release, ts2) + + createProwJobRunTest(t, dbc, run1.ID, jobAWS.ID, test.ID, &suite.ID, 1, release, ts1) + createProwJobRunTest(t, dbc, run2.ID, jobAWS.ID, test.ID, &suite.ID, 12, release, ts2) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.TestIDOptions = []reqopts.TestIdentification{{ + TestID: tow.UniqueID, + }} + opts.VariantOption.IncludeVariants = map[string][]string{ + "Platform": {"aws"}, + "Network": {"ovn"}, + } + + result, errs := provider.QueryBaseJobRunTestStatus(context.Background(), opts) + require.Empty(t, errs) + + totalRows := 0 + for _, rows := range result { + totalRows += len(rows) + } + assert.Equal(t, 2, totalRows, "should have 2 base job run test rows") + + for _, rows := range result { + for _, row := range rows { + assert.Equal(t, tow.UniqueID, row.TestKey.TestID) + } + } +} + +func TestQueryJobVariants(t *testing.T) { + dbc := crTestDB(t) + + jobs := []models.ProwJob{ + {Name: "periodic-e2e-aws-ovn-ha", Release: "4.16", Variants: pq.StringArray{"Platform:aws", "Network:ovn", "Topology:ha"}}, + {Name: "periodic-e2e-gcp-sdn-single", Release: "4.16", Variants: pq.StringArray{"Platform:gcp", "Network:sdn", "Topology:single"}}, + {Name: "periodic-e2e-aws-sdn-ha", Release: "4.16", Variants: pq.StringArray{"Platform:aws", "Network:sdn", "Topology:ha"}}, + } + for i := range jobs { + require.NoError(t, dbc.DB.Create(&jobs[i]).Error) + } + + provider := postgres.NewPostgresProvider(dbc, nil) + + result, errs := provider.QueryJobVariants(context.Background(), reqopts.RequestOptions{}) + require.Empty(t, errs) + + // Check Platform values + platforms, ok := result.Variants["Platform"] + require.True(t, ok, "should have Platform key") + assert.Equal(t, []string{"aws", "gcp"}, platforms, "Platform values should be sorted and deduplicated") + + // Check Network values + networks, ok := result.Variants["Network"] + require.True(t, ok, "should have Network key") + assert.Equal(t, []string{"ovn", "sdn"}, networks) + + // Check Topology values + topologies, ok := result.Variants["Topology"] + require.True(t, ok, "should have Topology key") + assert.Equal(t, []string{"ha", "single"}, topologies) +} + +func TestQueryJobRuns(t *testing.T) { + t.Run("only periodic, release, and aggregator jobs included", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vcAWS := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + + periodicJob := createProwJobWithVC(t, dbc, "periodic-ci-aws-test", release, vcAWS) + releaseJob := createProwJobWithVC(t, dbc, "release-ci-aws-test", release, vcAWS) + pullJob := createProwJobWithVC(t, dbc, "pull-ci-aws-test", release, vcAWS) + + ts1 := time.Date(2024, 6, 5, 12, 0, 0, 0, time.UTC) + ts2 := time.Date(2024, 6, 6, 12, 0, 0, 0, time.UTC) + ts3 := time.Date(2024, 6, 7, 12, 0, 0, 0, time.UTC) + + // periodic: 2 runs, 1 success + createProwJobRunForCR(t, dbc, periodicJob.ID, release, ts1) + r2 := createProwJobRunForCR(t, dbc, periodicJob.ID, release, ts2) + require.NoError(t, dbc.DB.Model(&r2).Updates(map[string]any{"succeeded": false, "failed": true}).Error) + + // release: 1 run, 1 success + createProwJobRunForCR(t, dbc, releaseJob.ID, release, ts1) + + // pull: 1 run (should be excluded by prefix filter) + createProwJobRunForCR(t, dbc, pullJob.ID, release, ts3) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + + result, err := provider.QueryJobRuns(context.Background(), opts, release, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.NoError(t, err) + + assert.Contains(t, result, "periodic-ci-aws-test") + assert.Contains(t, result, "release-ci-aws-test") + assert.NotContains(t, result, "pull-ci-aws-test", "pull request jobs should be excluded") + + periodic := result["periodic-ci-aws-test"] + assert.Equal(t, 2, periodic.TotalRuns) + assert.Equal(t, 1, periodic.SuccessfulRuns) + assert.InDelta(t, 50.0, periodic.PassRate, 0.01) + + releaseStats := result["release-ci-aws-test"] + assert.Equal(t, 1, releaseStats.TotalRuns) + assert.Equal(t, 1, releaseStats.SuccessfulRuns) + assert.InDelta(t, 100.0, releaseStats.PassRate, 0.01) + }) + + t.Run("variant filtering", func(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vcAWS := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + vcGCP := createVariantCombination(t, dbc, []string{"Platform:gcp", "Network:sdn"}) + + awsJob := createProwJobWithVC(t, dbc, "periodic-ci-aws-vf", release, vcAWS) + gcpJob := createProwJobWithVC(t, dbc, "periodic-ci-gcp-vf", release, vcGCP) + + ts := time.Date(2024, 6, 5, 12, 0, 0, 0, time.UTC) + createProwJobRunForCR(t, dbc, awsJob.ID, release, ts) + createProwJobRunForCR(t, dbc, gcpJob.ID, release, ts) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.VariantOption.IncludeVariants = map[string][]string{ + "Platform": {"aws"}, + } + + result, err := provider.QueryJobRuns(context.Background(), opts, release, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.NoError(t, err) + + assert.Contains(t, result, "periodic-ci-aws-vf") + assert.NotContains(t, result, "periodic-ci-gcp-vf", "gcp job should be filtered out by IncludeVariants") + }) +} + +func TestQueryUniqueVariantValues(t *testing.T) { + dbc := crTestDB(t) + + jobs := []models.ProwJob{ + {Name: "periodic-uv-aws", Variants: pq.StringArray{"Platform:aws", "Network:ovn", "Architecture:amd64"}}, + {Name: "periodic-uv-gcp", Variants: pq.StringArray{"Platform:gcp", "Network:sdn", "Architecture:arm64"}}, + } + for i := range jobs { + require.NoError(t, dbc.DB.Create(&jobs[i]).Error) + } + + provider := postgres.NewPostgresProvider(dbc, nil) + + t.Run("nested returns key names", func(t *testing.T) { + result, err := provider.QueryUniqueVariantValues(context.Background(), reqopts.RequestOptions{}, "", true) + require.NoError(t, err) + assert.Equal(t, []string{"Architecture", "Network", "Platform"}, result) + }) + + t.Run("field maps to variant key", func(t *testing.T) { + result, err := provider.QueryUniqueVariantValues(context.Background(), reqopts.RequestOptions{}, "platform", false) + require.NoError(t, err) + assert.Equal(t, []string{"aws", "gcp"}, result) + }) + + t.Run("arch field maps to Architecture", func(t *testing.T) { + result, err := provider.QueryUniqueVariantValues(context.Background(), reqopts.RequestOptions{}, "arch", false) + require.NoError(t, err) + assert.Equal(t, []string{"amd64", "arm64"}, result) + }) + + t.Run("unknown field returns empty", func(t *testing.T) { + result, err := provider.QueryUniqueVariantValues(context.Background(), reqopts.RequestOptions{}, "unknown", false) + require.NoError(t, err) + assert.Empty(t, result) + }) +} + +func TestQueryJobVariantValues(t *testing.T) { + dbc := crTestDB(t) + + job1 := models.ProwJob{Name: "periodic-jvv-aws", Variants: pq.StringArray{"Platform:aws", "Network:ovn", "Topology:ha"}} + job2 := models.ProwJob{Name: "periodic-jvv-gcp", Variants: pq.StringArray{"Platform:gcp", "Network:sdn", "Topology:single"}} + require.NoError(t, dbc.DB.Create(&job1).Error) + require.NoError(t, dbc.DB.Create(&job2).Error) + + provider := postgres.NewPostgresProvider(dbc, nil) + + t.Run("returns all variants for given jobs", func(t *testing.T) { + result, err := provider.QueryJobVariantValues(context.Background(), reqopts.RequestOptions{}, + []string{"periodic-jvv-aws", "periodic-jvv-gcp"}, nil) + require.NoError(t, err) + + assert.Equal(t, map[string]string{"Platform": "aws", "Network": "ovn", "Topology": "ha"}, result["periodic-jvv-aws"]) + assert.Equal(t, map[string]string{"Platform": "gcp", "Network": "sdn", "Topology": "single"}, result["periodic-jvv-gcp"]) + }) + + t.Run("variantKeys filter", func(t *testing.T) { + result, err := provider.QueryJobVariantValues(context.Background(), reqopts.RequestOptions{}, + []string{"periodic-jvv-aws"}, []string{"Platform"}) + require.NoError(t, err) + + assert.Equal(t, map[string]string{"Platform": "aws"}, result["periodic-jvv-aws"]) + }) + + t.Run("empty jobNames returns empty map", func(t *testing.T) { + result, err := provider.QueryJobVariantValues(context.Background(), reqopts.RequestOptions{}, nil, nil) + require.NoError(t, err) + assert.Empty(t, result) + }) +} + +func TestLookupJobVariants(t *testing.T) { + dbc := crTestDB(t) + + job := models.ProwJob{Name: "periodic-ljv-aws", Variants: pq.StringArray{"Platform:aws", "Network:ovn"}} + require.NoError(t, dbc.DB.Create(&job).Error) + + provider := postgres.NewPostgresProvider(dbc, nil) + result, err := provider.LookupJobVariants(context.Background(), reqopts.RequestOptions{}, "periodic-ljv-aws") + require.NoError(t, err) + assert.Equal(t, map[string]string{"Platform": "aws", "Network": "ovn"}, result) +} + +func TestQueryReleases(t *testing.T) { + dbc := crTestDB(t) + + earlier := time.Date(2023, 6, 1, 0, 0, 0, 0, time.UTC) + later := time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC) + gaDate := time.Date(2024, 3, 1, 0, 0, 0, 0, time.UTC) + + rd1 := models.ReleaseDefinition{ + Release: "4.15", + Status: "GA", + GADate: &gaDate, + PreviousRelease: "4.14", + Product: "OCP", + DevelopmentStartDate: &earlier, + } + rd2 := models.ReleaseDefinition{ + Release: "4.16", + Status: "Development", + PreviousRelease: "4.15", + Product: "OCP", + DevelopmentStartDate: &later, + } + require.NoError(t, dbc.DB.Create(&rd1).Error) + require.NoError(t, dbc.DB.Create(&rd2).Error) + + provider := postgres.NewPostgresProvider(dbc, nil) + releases, err := provider.QueryReleases(context.Background()) + require.NoError(t, err) + require.Len(t, releases, 2) + + // Ordered by DevelopmentStartDate DESC, so 4.16 first + assert.Equal(t, "4.16", releases[0].Release) + assert.Equal(t, "Development", releases[0].Status) + assert.Nil(t, releases[0].GADate) + assert.Equal(t, "4.15", releases[0].PreviousRelease) + assert.Equal(t, "OCP", releases[0].Product) + + assert.Equal(t, "4.15", releases[1].Release) + assert.Equal(t, "GA", releases[1].Status) + require.NotNil(t, releases[1].GADate) + assert.True(t, releases[1].GADate.Equal(gaDate)) +} + +func TestQueryReleaseDates(t *testing.T) { + dbc := crTestDB(t) + + gaDate := time.Date(2024, 3, 1, 0, 0, 0, 0, time.UTC) + devStart := time.Date(2023, 6, 1, 0, 0, 0, 0, time.UTC) + devStartNoGA := time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC) + + rdWithGA := models.ReleaseDefinition{ + Release: "4.15", + GADate: &gaDate, + DevelopmentStartDate: &devStart, + Product: "OCP", + } + rdWithoutGA := models.ReleaseDefinition{ + Release: "4.16", + DevelopmentStartDate: &devStartNoGA, + Product: "OCP", + } + require.NoError(t, dbc.DB.Create(&rdWithGA).Error) + require.NoError(t, dbc.DB.Create(&rdWithoutGA).Error) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := reqopts.RequestOptions{} + timeRanges, errs := provider.QueryReleaseDates(context.Background(), opts) + require.Empty(t, errs) + require.Len(t, timeRanges, 2) + + var withGA, withoutGA *crtest.ReleaseTimeRange + for i := range timeRanges { + switch timeRanges[i].Release { + case "4.15": + withGA = &timeRanges[i] + case "4.16": + withoutGA = &timeRanges[i] + } + } + + require.NotNil(t, withGA, "release 4.15 should be in results") + require.NotNil(t, withGA.Start, "release with GADate should have Start") + require.NotNil(t, withGA.End, "release with GADate should have End") + assert.True(t, withGA.End.Equal(gaDate), "End should be the GA date") + + require.NotNil(t, withoutGA, "release 4.16 should be in results") + assert.Nil(t, withoutGA.Start, "release without GADate should have nil Start") + assert.Nil(t, withoutGA.End, "release without GADate should have nil End") +} + +func TestGAPathAggregatesMultipleJobs(t *testing.T) { + dbc := crTestDB(t) + release := "4.15" + + gaDate := time.Date(2024, 3, 1, 0, 0, 0, 0, time.UTC) + createReleaseDefinition(t, dbc, release, &gaDate) + + gaCivil := civil.DateOf(gaDate) + gaEnd := utils.GAWindowEnd(gaCivil) + windowDays := 30 + gaStart := gaCivil.AddDays(-windowDays) + + vc1 := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + vc2 := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:sdn"}) + job1 := createProwJobWithVC(t, dbc, "periodic-ga-multi-1", release, vc1) + job2 := createProwJobWithVC(t, dbc, "periodic-ga-multi-2", release, vc2) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] GA multi-job test") + suite := createSuite(t, dbc, "openshift-tests-ga-mj") + createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:ga-multi", "Storage", []string{"PVC"}) + + // Two jobs contribute GA data for the same test on the same variant group (when grouped by Platform only) + createGARawData(t, dbc, release, windowDays, test.ID, job1.ID, suite.ID, 30, 25, 2) + createGARawData(t, dbc, release, windowDays, test.ID, job2.ID, suite.ID, 20, 18, 1) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.BaseRelease = reqopts.Release{ + Name: release, + Start: gaStart.In(time.UTC), + End: gaEnd.AddDays(-1).In(time.UTC), + } + opts.VariantOption.DBGroupBy = sets.New[string]("Platform") + opts.VariantOption.ColumnGroupBy = sets.New[string]("Platform") + opts.VariantOption.IncludeVariants = map[string][]string{ + "Platform": {"aws"}, + } + + result, errs := provider.QueryBaseTestStatus(context.Background(), opts) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: "openshift-tests:ga-multi", + Variants: map[string]string{"Platform": "aws"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "GA path should return aggregated data from multiple jobs") + assert.Equal(t, 50, ts.TotalCount, "runs should aggregate: 30 + 20") + assert.Equal(t, 43, ts.SuccessCount, "successes should aggregate: 25 + 18") + assert.Equal(t, 3, ts.FlakeCount, "flakes should aggregate: 2 + 1") +} + +func TestMultipleTestsInSameComponent(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-e2e-aws-multi", release, vc) + + testA := createTest(t, dbc, "openshift-tests:[sig-storage] PVC create") + testB := createTest(t, dbc, "openshift-tests:[sig-storage] PVC expand") + suite := createSuite(t, dbc, "openshift-tests-multi") + + towA := createTestOwnership(t, dbc, testA.ID, &suite.ID, "openshift-tests:pvc-create", "Storage", []string{"PVC"}) + towB := createTestOwnership(t, dbc, testB.ID, &suite.ID, "openshift-tests:pvc-expand", "Storage", []string{"PVC"}) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + + // testA: runs=10, successes=9, flakes=0 -> failures=1 (90% pass rate) + createCumulativeSummary(t, dbc, startMinus1, release, testA.ID, job.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, release, testA.ID, job.ID, suite.ID, 110, 99, 5) + + // testB: runs=10, successes=5, flakes=0 -> failures=5 (50% pass rate) + createCumulativeSummary(t, dbc, startMinus1, release, testB.ID, job.ID, suite.ID, 50, 40, 2) + createCumulativeSummary(t, dbc, endMinus1, release, testB.ID, job.ID, suite.ID, 60, 45, 2) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + keyA := crtest.KeyWithVariants{ + TestID: towA.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + keyB := crtest.KeyWithVariants{ + TestID: towB.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + + tsA, ok := result[keyA.Encode()] + require.True(t, ok, "testA should appear independently") + assert.Equal(t, 10, tsA.TotalCount) + assert.Equal(t, 9, tsA.SuccessCount) + assert.Equal(t, "Storage", tsA.Component) + + tsB, ok := result[keyB.Encode()] + require.True(t, ok, "testB should appear independently") + assert.Equal(t, 10, tsB.TotalCount) + assert.Equal(t, 5, tsB.SuccessCount) + assert.Equal(t, "Storage", tsB.Component) +} + +func TestBaseAggregatesMultipleJobsInSameVariantGroup(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc1 := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + vc2 := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:sdn"}) + job1 := createProwJobWithVC(t, dbc, "periodic-base-agg-1", release, vc1) + job2 := createProwJobWithVC(t, dbc, "periodic-base-agg-2", release, vc2) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] base agg test") + suite := createSuite(t, dbc, "openshift-tests-bagg") + createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:base-agg", "Storage", []string{"PVC"}) + + baseLookupStart := civil.Date{Year: 2024, Month: 5, Day: 14} + baseLookupEnd := civil.Date{Year: 2024, Month: 6, Day: 1} + + // job1: delta runs=20, successes=15, flakes=2 + createCumulativeSummary(t, dbc, baseLookupStart, release, test.ID, job1.ID, suite.ID, 80, 75, 3) + createCumulativeSummary(t, dbc, baseLookupEnd, release, test.ID, job1.ID, suite.ID, 100, 90, 5) + + // job2: delta runs=10, successes=8, flakes=1 + createCumulativeSummary(t, dbc, baseLookupStart, release, test.ID, job2.ID, suite.ID, 40, 35, 1) + createCumulativeSummary(t, dbc, baseLookupEnd, release, test.ID, job2.ID, suite.ID, 50, 43, 2) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.VariantOption.DBGroupBy = sets.New[string]("Platform") + opts.VariantOption.ColumnGroupBy = sets.New[string]("Platform") + opts.VariantOption.IncludeVariants = map[string][]string{ + "Platform": {"aws"}, + } + + result, errs := provider.QueryBaseTestStatus(context.Background(), opts) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: "openshift-tests:base-agg", + Variants: map[string]string{"Platform": "aws"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "base should aggregate data from multiple jobs in same variant group") + assert.Equal(t, 30, ts.TotalCount, "runs should aggregate: 20 + 10") + assert.Equal(t, 23, ts.SuccessCount, "successes should aggregate: 15 + 8") + assert.Equal(t, 3, ts.FlakeCount, "flakes should aggregate: 2 + 1") +} + +func TestTestDetailStatusMapping(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-status-map", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] status mapping test") + suite := createSuite(t, dbc, "openshift-tests-sm") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:status-map", "Storage", []string{"PVC"}) + + passTS := time.Date(2024, 6, 5, 12, 0, 0, 0, time.UTC) + failTS := time.Date(2024, 6, 6, 12, 0, 0, 0, time.UTC) + flakeTS := time.Date(2024, 6, 7, 12, 0, 0, 0, time.UTC) + + passRun := createProwJobRunForCR(t, dbc, job.ID, release, passTS) + failRun := createProwJobRunForCR(t, dbc, job.ID, release, failTS) + flakeRun := createProwJobRunForCR(t, dbc, job.ID, release, flakeTS) + + createProwJobRunTest(t, dbc, passRun.ID, job.ID, test.ID, &suite.ID, 1, release, passTS) + createProwJobRunTest(t, dbc, failRun.ID, job.ID, test.ID, &suite.ID, 12, release, failTS) + createProwJobRunTest(t, dbc, flakeRun.ID, job.ID, test.ID, &suite.ID, 13, release, flakeTS) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.TestIDOptions = []reqopts.TestIdentification{{TestID: tow.UniqueID}} + + result, errs := provider.QuerySampleJobRunTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + var rows []crstatus.TestJobRunRows + for _, jobRows := range result { + rows = append(rows, jobRows...) + } + require.Len(t, rows, 3) + + // Results are ordered by pjr.timestamp: pass (June 5), fail (June 6), flake (June 7) + passRow := rows[0] + assert.Equal(t, 1, passRow.TotalCount) + assert.Equal(t, 1, passRow.SuccessCount, "pass: SuccessCount should be 1") + assert.Equal(t, 0, passRow.FlakeCount, "pass: FlakeCount should be 0") + + failRow := rows[1] + assert.Equal(t, 1, failRow.TotalCount) + assert.Equal(t, 0, failRow.SuccessCount, "fail: SuccessCount should be 0") + assert.Equal(t, 0, failRow.FlakeCount, "fail: FlakeCount should be 0") + + flakeRow := rows[2] + assert.Equal(t, 1, flakeRow.TotalCount) + assert.Equal(t, 1, flakeRow.SuccessCount, "flake: SuccessCount should be 1") + assert.Equal(t, 1, flakeRow.FlakeCount, "flake: FlakeCount should be 1") +} + +func TestJobNameNormalizationMergesResults(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + // Two jobs whose names differ only by version number, which normalizes to the same key + job416 := createProwJobWithVC(t, dbc, "periodic-ci-4.16-e2e-aws", release, vc) + job417 := createProwJobWithVC(t, dbc, "periodic-ci-4.17-e2e-aws", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] normalization test") + suite := createSuite(t, dbc, "openshift-tests-norm") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:norm-test", "Storage", []string{"PVC"}) + + ts1 := time.Date(2024, 6, 5, 12, 0, 0, 0, time.UTC) + ts2 := time.Date(2024, 6, 6, 12, 0, 0, 0, time.UTC) + + run1 := createProwJobRunForCR(t, dbc, job416.ID, release, ts1) + run2 := createProwJobRunForCR(t, dbc, job417.ID, release, ts2) + + createProwJobRunTest(t, dbc, run1.ID, job416.ID, test.ID, &suite.ID, 1, release, ts1) + createProwJobRunTest(t, dbc, run2.ID, job417.ID, test.ID, &suite.ID, 12, release, ts2) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.TestIDOptions = []reqopts.TestIdentification{{TestID: tow.UniqueID}} + + result, errs := provider.QuerySampleJobRunTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + // Both "periodic-ci-4.16-e2e-aws" and "periodic-ci-4.17-e2e-aws" should normalize + // to "periodic-ci-X.X-e2e-aws" and merge under that single key + normalizedKey := utils.NormalizeProwJobName("periodic-ci-4.16-e2e-aws") + rows, ok := result[normalizedKey] + require.True(t, ok, "both job runs should merge under normalized name %q", normalizedKey) + assert.Len(t, rows, 2, "both runs should appear under the normalized key") +} + +func TestTestExistsInBaseButNotSample(t *testing.T) { + dbc := crTestDB(t) + baseRelease := "4.16" + sampleRelease := "4.17" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + baseJob := createProwJobWithVC(t, dbc, "periodic-e2e-aws-base-only", baseRelease, vc) + sampleJob := createProwJobWithVC(t, dbc, "periodic-e2e-aws-sample-only", sampleRelease, vc) + + baseOnlyTest := createTest(t, dbc, "openshift-tests:[sig-storage] base-only test") + sampleOnlyTest := createTest(t, dbc, "openshift-tests:[sig-storage] sample-only test") + sharedTest := createTest(t, dbc, "openshift-tests:[sig-storage] shared test") + suite := createSuite(t, dbc, "openshift-tests-missing") + + towBaseOnly := createTestOwnership(t, dbc, baseOnlyTest.ID, &suite.ID, "openshift-tests:base-only", "Storage", []string{"PVC"}) + towSampleOnly := createTestOwnership(t, dbc, sampleOnlyTest.ID, &suite.ID, "openshift-tests:sample-only", "Storage", []string{"PVC"}) + towShared := createTestOwnership(t, dbc, sharedTest.ID, &suite.ID, "openshift-tests:shared", "Storage", []string{"PVC"}) + + baseLookupStart := civil.Date{Year: 2024, Month: 5, Day: 14} + baseLookupEnd := civil.Date{Year: 2024, Month: 6, Day: 1} + sampleLookupStart := civil.Date{Year: 2024, Month: 5, Day: 31} + sampleLookupEnd := civil.Date{Year: 2024, Month: 6, Day: 14} + + // baseOnlyTest has base data but no sample data + createCumulativeSummary(t, dbc, baseLookupStart, baseRelease, baseOnlyTest.ID, baseJob.ID, suite.ID, 80, 75, 3) + createCumulativeSummary(t, dbc, baseLookupEnd, baseRelease, baseOnlyTest.ID, baseJob.ID, suite.ID, 100, 90, 5) + + // sampleOnlyTest has sample data but no base data + createCumulativeSummary(t, dbc, sampleLookupStart, sampleRelease, sampleOnlyTest.ID, sampleJob.ID, suite.ID, 50, 40, 2) + createCumulativeSummary(t, dbc, sampleLookupEnd, sampleRelease, sampleOnlyTest.ID, sampleJob.ID, suite.ID, 60, 48, 3) + + // sharedTest has data in both + createCumulativeSummary(t, dbc, baseLookupStart, baseRelease, sharedTest.ID, baseJob.ID, suite.ID, 80, 75, 3) + createCumulativeSummary(t, dbc, baseLookupEnd, baseRelease, sharedTest.ID, baseJob.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, sampleLookupStart, sampleRelease, sharedTest.ID, sampleJob.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, sampleLookupEnd, sampleRelease, sharedTest.ID, sampleJob.ID, suite.ID, 110, 98, 6) + + provider := postgres.NewPostgresProvider(dbc, nil) + includeVariants := map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}} + opts := reqopts.RequestOptions{ + BaseRelease: reqopts.Release{ + Name: baseRelease, + Start: time.Date(2024, 5, 15, 0, 0, 0, 0, time.UTC), + End: time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC), + }, + SampleRelease: reqopts.Release{ + Name: sampleRelease, + Start: time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC), + End: time.Date(2024, 6, 15, 0, 0, 0, 0, time.UTC), + }, + VariantOption: reqopts.Variants{ + DBGroupBy: sets.New[string]("Platform", "Network"), + ColumnGroupBy: sets.New[string]("Platform"), + IncludeVariants: includeVariants, + }, + AdvancedOption: reqopts.Advanced{MinimumFailure: 1}, + } + + variantMap := map[string]string{"Platform": "aws", "Network": "ovn"} + + baseResult, errs := provider.QueryBaseTestStatus(context.Background(), opts) + require.Empty(t, errs) + + sampleResult, errs := provider.QuerySampleTestStatus(context.Background(), opts, + includeVariants, opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + // baseOnlyTest: should be in base results, absent from sample + baseOnlyKey := crtest.KeyWithVariants{TestID: towBaseOnly.UniqueID, Variants: variantMap}.Encode() + _, inBase := baseResult[baseOnlyKey] + assert.True(t, inBase, "base-only test should appear in base results") + nonPlaceholderSample := filterPlaceholders(sampleResult) + for _, ts := range nonPlaceholderSample { + assert.NotEqual(t, towBaseOnly.UniqueID, ts.TestID, + "base-only test should not appear in sample results") + } + + // sampleOnlyTest: should be in sample results, absent from base + sampleOnlyKey := crtest.KeyWithVariants{TestID: towSampleOnly.UniqueID, Variants: variantMap}.Encode() + _, inSample := sampleResult[sampleOnlyKey] + assert.True(t, inSample, "sample-only test should appear in sample results") + nonPlaceholderBase := filterPlaceholders(baseResult) + for _, ts := range nonPlaceholderBase { + assert.NotEqual(t, towSampleOnly.UniqueID, ts.TestID, + "sample-only test should not appear in base results") + } + + // sharedTest: should be in both + sharedKey := crtest.KeyWithVariants{TestID: towShared.UniqueID, Variants: variantMap}.Encode() + _, inBase = baseResult[sharedKey] + _, inSample = sampleResult[sharedKey] + assert.True(t, inBase, "shared test should appear in base results") + assert.True(t, inSample, "shared test should appear in sample results") +} + +func TestSingleDayPeriod(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-e2e-aws-1day", release, vc) + + test := createTest(t, dbc, "openshift-tests:[sig-storage] single day test") + suite := createSuite(t, dbc, "openshift-tests-1day") + tow := createTestOwnership(t, dbc, test.ID, &suite.ID, "openshift-tests:1day-test", "Storage", []string{"PVC"}) + + // Single-day range: [2024-06-10, 2024-06-11) + // lookupStart = 2024-06-09, lookupEnd = 2024-06-10 + lookupStart := civil.Date{Year: 2024, Month: 6, Day: 9} + lookupEnd := civil.Date{Year: 2024, Month: 6, Day: 10} + + // delta: runs=5, successes=2, flakes=1 → failures=2 (passes MinimumFailure=1) + createCumulativeSummary(t, dbc, lookupStart, release, test.ID, job.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, lookupEnd, release, test.ID, job.ID, suite.ID, 105, 92, 6) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.SampleRelease.Start = time.Date(2024, 6, 10, 0, 0, 0, 0, time.UTC) + opts.SampleRelease.End = time.Date(2024, 6, 11, 0, 0, 0, 0, time.UTC) + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + key := crtest.KeyWithVariants{ + TestID: tow.UniqueID, + Variants: map[string]string{"Platform": "aws", "Network": "ovn"}, + } + ts, ok := result[key.Encode()] + require.True(t, ok, "single-day period should return results") + assert.Equal(t, 5, ts.TotalCount, "delta for single day: 105-100=5") + assert.Equal(t, 2, ts.SuccessCount, "delta for single day: 92-90=2") + assert.Equal(t, 1, ts.FlakeCount, "delta for single day: 6-5=1") +} + +func TestMinimumFailureWithCapabilityFilter(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-e2e-aws-mfcap", release, vc) + + testHigh := createTest(t, dbc, "openshift-tests:[sig-storage] high failure PVC test") + testLow := createTest(t, dbc, "openshift-tests:[sig-storage] low failure PVC test") + testOther := createTest(t, dbc, "openshift-tests:[sig-network] high failure network test") + suite := createSuite(t, dbc, "openshift-tests-mfcap") + + // testHigh and testLow share PVC capability; testOther has Services capability + createTestOwnership(t, dbc, testHigh.ID, &suite.ID, "openshift-tests:high-pvc", "Storage", []string{"PVC"}) + towLow := createTestOwnership(t, dbc, testLow.ID, &suite.ID, "openshift-tests:low-pvc", "Storage", []string{"PVC"}) + createTestOwnership(t, dbc, testOther.ID, &suite.ID, "openshift-tests:high-net", "Networking", []string{"Services"}) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + + // testHigh: runs=10, successes=5, flakes=0 -> failures=5 + createCumulativeSummary(t, dbc, startMinus1, release, testHigh.ID, job.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, release, testHigh.ID, job.ID, suite.ID, 110, 95, 5) + + // testLow: runs=10, successes=9, flakes=0 -> failures=1 + createCumulativeSummary(t, dbc, startMinus1, release, testLow.ID, job.ID, suite.ID, 50, 45, 2) + createCumulativeSummary(t, dbc, endMinus1, release, testLow.ID, job.ID, suite.ID, 60, 54, 2) + + // testOther: runs=10, successes=5, flakes=0 -> failures=5 (but different capability) + createCumulativeSummary(t, dbc, startMinus1, release, testOther.ID, job.ID, suite.ID, 200, 180, 10) + createCumulativeSummary(t, dbc, endMinus1, release, testOther.ID, job.ID, suite.ID, 210, 185, 10) + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.AdvancedOption.MinimumFailure = 3 + opts.TestIDOptions = []reqopts.TestIdentification{{ + Capability: "PVC", + }} + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + nonPlaceholders := filterPlaceholders(result) + // testHigh has PVC capability and 5 failures >= 3: should appear + // testLow has PVC capability but 1 failure < 3: should NOT appear + // testOther has 5 failures >= 3 but Services capability, not PVC: should NOT appear + for _, ts := range nonPlaceholders { + assert.NotEqual(t, towLow.UniqueID, ts.TestID, + "low-failure PVC test should not pass MinimumFailure=3") + assert.NotEqual(t, "openshift-tests:high-net", ts.TestID, + "non-PVC test should not appear with PVC capability filter") + } +} + +func TestDrillDownBySecondaryCapability(t *testing.T) { + dbc := crTestDB(t) + release := "4.16" + + vc := createVariantCombination(t, dbc, []string{"Platform:aws", "Network:ovn"}) + job := createProwJobWithVC(t, dbc, "periodic-e2e-aws-cap2", release, vc) + + // testShared has both PVC and IPv4 capabilities + testShared := createTest(t, dbc, "openshift-tests:[sig-storage] shared cap test") + // testIPv4Only has only IPv4 + testIPv4Only := createTest(t, dbc, "openshift-tests:[sig-network] ipv4 only test") + // testPVCOnly has only PVC + testPVCOnly := createTest(t, dbc, "openshift-tests:[sig-storage] pvc only test") + suite := createSuite(t, dbc, "openshift-tests-cap2") + + createTestOwnership(t, dbc, testShared.ID, &suite.ID, "openshift-tests:shared-cap", "Storage", []string{"PVC", "IPv4"}) + createTestOwnership(t, dbc, testIPv4Only.ID, &suite.ID, "openshift-tests:ipv4-only", "Networking", []string{"IPv4"}) + createTestOwnership(t, dbc, testPVCOnly.ID, &suite.ID, "openshift-tests:pvc-only", "Storage", []string{"PVC"}) + + startMinus1 := civil.Date{Year: 2024, Month: 5, Day: 31} + endMinus1 := civil.Date{Year: 2024, Month: 6, Day: 14} + + for _, testModel := range []models.Test{testShared, testIPv4Only, testPVCOnly} { + createCumulativeSummary(t, dbc, startMinus1, release, testModel.ID, job.ID, suite.ID, 100, 90, 5) + createCumulativeSummary(t, dbc, endMinus1, release, testModel.ID, job.ID, suite.ID, 110, 98, 6) + } + + provider := postgres.NewPostgresProvider(dbc, nil) + opts := defaultReqOptions(release) + opts.TestIDOptions = []reqopts.TestIdentification{{ + Capability: "IPv4", + }} + + result, errs := provider.QuerySampleTestStatus(context.Background(), opts, + map[string][]string{"Platform": {"aws"}, "Network": {"ovn"}}, + opts.SampleRelease.Start, opts.SampleRelease.End) + require.Empty(t, errs) + + nonPlaceholders := filterPlaceholders(result) + for _, ts := range nonPlaceholders { + assert.Contains(t, ts.Capabilities, "IPv4", + "only tests with IPv4 capability should appear, got %v for %s", ts.Capabilities, ts.TestID) + } + + // testPVCOnly should not appear (no IPv4 capability) + for _, ts := range nonPlaceholders { + assert.NotEqual(t, "openshift-tests:pvc-only", ts.TestID, + "test with only PVC capability should not appear in IPv4 drill-down") + } +} + +// --- Helpers --- + +func isPlaceholderKey(testID string) bool { + return len(testID) > 5 && testID[:5] == "grid:" +} + +func filterPlaceholders(result map[string]crstatus.TestStatus) []crstatus.TestStatus { + var nonPlaceholder []crstatus.TestStatus + for _, ts := range result { + if !isPlaceholderKey(ts.TestID) { + nonPlaceholder = append(nonPlaceholder, ts) + } + } + return nonPlaceholder +}