diff --git a/cmd/sippy/seed_data.go b/cmd/sippy/seed_data.go index 9ae185b4cb..62de31a3a6 100644 --- a/cmd/sippy/seed_data.go +++ b/cmd/sippy/seed_data.go @@ -128,7 +128,7 @@ type testCount struct { flake int } -var syntheticReleases = []string{"4.22", "4.21", "4.20", "4.19"} +var syntheticReleases = []string{"4.22", "4.21", "4.20", "4.19", models.ReleasePresubmits} var syntheticJobs = []syntheticJobDef{ { @@ -490,6 +490,11 @@ func seedSyntheticData(dbc *db.DB) error { return err } + if err := seedPresubmitData(dbc); err != nil { + return errors.WithMessage(err, "failed to seed presubmit data") + } + log.Info("Seeded presubmit/PR test data") + if err := createLabelsAndSymptoms(dbc); err != nil { return errors.WithMessage(err, "failed to create labels and symptoms") } @@ -543,28 +548,39 @@ func seedReleaseDefinitions(dbc *db.DB) error { } for _, release := range syntheticReleases { - m := meta[release] - parts := strings.Split(release, ".") - major, minor := 0, 0 - if len(parts) >= 2 { - _, _ = fmt.Sscanf(parts[0], "%d", &major) - _, _ = fmt.Sscanf(parts[1], "%d", &minor) - } + var def models.ReleaseDefinition + + if release == models.ReleasePresubmits { + def = models.ReleaseDefinition{ + Release: release, + Product: "OCP", + Status: "Development", + Capabilities: pq.StringArray{models.CapPullRequests, models.CapSippyClassic}, + } + } else { + m := meta[release] + parts := strings.Split(release, ".") + major, minor := 0, 0 + if len(parts) >= 2 { + _, _ = fmt.Sscanf(parts[0], "%d", &major) + _, _ = fmt.Sscanf(parts[1], "%d", &minor) + } - develStart := now.AddDate(0, 0, m.gaDays-180) - def := models.ReleaseDefinition{ - Release: release, - Major: major, - Minor: minor, - PreviousRelease: m.previous, - DevelopmentStartDate: &develStart, - Product: "OCP", - Status: "Full Support", - Capabilities: allCaps, - } - if m.gaDays != 0 { - ga := now.AddDate(0, 0, m.gaDays) - def.GADate = &ga + develStart := now.AddDate(0, 0, m.gaDays-180) + def = models.ReleaseDefinition{ + Release: release, + Major: major, + Minor: minor, + PreviousRelease: m.previous, + DevelopmentStartDate: &develStart, + Product: "OCP", + Status: "Full Support", + Capabilities: allCaps, + } + if m.gaDays != 0 { + ga := now.AddDate(0, 0, m.gaDays) + def.GADate = &ga + } } if err := dbc.DB.Where("release = ?", release).FirstOrCreate(&def).Error; err != nil { @@ -897,6 +913,234 @@ func syncRegressions(dbc *db.DB) error { return nil } +func seedPresubmitData(dbc *db.DB) error { + now := time.Now().UTC().Truncate(time.Hour) + + var suite models.Suite + if err := dbc.DB.Where("name = ?", "synthetic").First(&suite).Error; err != nil { + return fmt.Errorf("failed to find suite: %w", err) + } + + // Look up existing test records to reuse + testNames := []string{ + "install should succeed: overall", + "[sig-network] Services should serve endpoints on same port and different protocol", + } + testsByName := map[string]uint{} + for _, name := range testNames { + var t models.Test + if err := dbc.DB.Where("name = ?", name).First(&t).Error; err != nil { + return fmt.Errorf("failed to find test %q: %w", name, err) + } + testsByName[name] = t.ID + } + + // Create presubmit ProwJobs + presubmitJobs := []models.ProwJob{ + { + Kind: models.ProwKind("presubmit"), + Name: "openshift-origin-ci-5.0-e2e-aws-ovn-upgrade", + Release: models.ReleasePresubmits, + Variants: pq.StringArray{ + "Architecture:amd64", "FeatureSet:default", "Installer:ipi", + "LayeredProduct:none", "Network:ovn", "Platform:aws", + "Suite:unknown", "Topology:ha", "Upgrade:minor", + }, + }, + { + Kind: models.ProwKind("presubmit"), + Name: "openshift-origin-ci-5.0-e2e-gcp-ovn-amd64", + Release: models.ReleasePresubmits, + Variants: pq.StringArray{ + "Architecture:amd64", "FeatureSet:default", "Installer:ipi", + "LayeredProduct:none", "Network:ovn", "Platform:gcp", + "Suite:parallel", "Topology:ha", "Upgrade:none", + }, + }, + } + + for i, pj := range presubmitJobs { + if err := dbc.DB.Create(&pj).Error; err != nil { + return fmt.Errorf("failed to create presubmit ProwJob %s: %w", pj.Name, err) + } + presubmitJobs[i] = pj + } + + // Create ProwPullRequests + // PR 99001 has two SHAs to exercise the latest_sha_only filter: an older + // SHA linked to the earliest run, and a newer SHA linked to the rest. + prs := []models.ProwPullRequest{ + { + Org: "openshift", + Repo: "origin", + Number: 99001, + Author: "test-author-1", + Title: "Test PR 99001", + SHA: "abc123def456", + Link: "https://github.com/openshift/origin/pull/99001", + }, + { + Org: "openshift", + Repo: "origin", + Number: 99002, + Author: "test-author-2", + Title: "Test PR 99002", + SHA: "789abc012def", + Link: "https://github.com/openshift/origin/pull/99002", + }, + } + oldSHAPR := models.ProwPullRequest{ + Org: "openshift", + Repo: "origin", + Number: 99001, + Author: "test-author-1", + Title: "Test PR 99001", + SHA: "old111old222", + Link: "https://github.com/openshift/origin/pull/99001?old=1", + } + + for i, pr := range prs { + if err := dbc.DB.Create(&pr).Error; err != nil { + return fmt.Errorf("failed to create ProwPullRequest %d: %w", pr.Number, err) + } + prs[i] = pr + } + if err := dbc.DB.Create(&oldSHAPR).Error; err != nil { + return fmt.Errorf("failed to create old-SHA ProwPullRequest: %w", err) + } + + // Create runs: 3 runs per job, PR 99001 gets job[0] runs, PR 99002 gets job[1] runs + type runInfo struct { + run models.ProwJobRun + prIdx int + } + var runs []runInfo + + for jobIdx, pj := range presubmitJobs { + for i := 0; i < 3; i++ { + timestamp := now.Add(-time.Duration(3-i) * 20 * time.Hour) + run := models.ProwJobRun{ + ProwJobID: pj.ID, + ProwJobRelease: models.ReleasePresubmits, + Cluster: "build01", + Timestamp: timestamp, + Duration: 2 * time.Hour, + OverallResult: v1.JobTestFailure, + Failed: true, + } + if err := dbc.DB.Create(&run).Error; err != nil { + return fmt.Errorf("failed to create ProwJobRun: %w", err) + } + runs = append(runs, runInfo{run: run, prIdx: jobIdx}) + } + } + + // Link runs to PRs via join table. + // The first run of job[0] (oldest for PR 99001) links to oldSHAPR so that + // the latest_sha_only filter has something to exclude. + for runIdx, ri := range runs { + prID := prs[ri.prIdx].ID + if ri.prIdx == 0 && runIdx == 0 { + prID = oldSHAPR.ID + } + jrpr := models.ProwJobRunProwPullRequest{ + ProwJobRunID: ri.run.ID, + ProwPullRequestID: prID, + ProwJobRunRelease: models.ReleasePresubmits, + ProwJobRunTimestamp: ri.run.Timestamp, + } + if err := dbc.DB.Create(&jrpr).Error; err != nil { + return fmt.Errorf("failed to create ProwJobRunProwPullRequest: %w", err) + } + } + + // Create test results with mixed statuses + installTestID := testsByName["install should succeed: overall"] + networkTestID := testsByName["[sig-network] Services should serve endpoints on same port and different protocol"] + + for _, ri := range runs { + // Failure result for install test + failResult := models.ProwJobRunTest{ + ProwJobRunID: ri.run.ID, + ProwJobID: ri.run.ProwJobID, + ProwJobRunRelease: models.ReleasePresubmits, + ProwJobRunTimestamp: ri.run.Timestamp, + TestID: installTestID, + SuiteID: &suite.ID, + Status: int(v1.TestStatusFailure), + Duration: 5.0, + CreatedAt: ri.run.Timestamp, + } + if err := dbc.DB.Create(&failResult).Error; err != nil { + return fmt.Errorf("failed to create failure ProwJobRunTest: %w", err) + } + + // Add output for the first failure only + if ri.prIdx == 0 && ri.run.Timestamp.Equal(runs[0].run.Timestamp) { + output := models.ProwJobRunTestOutput{ + ProwJobRunTestID: failResult.ID, + Output: "Expected install to succeed but got timeout after 30m", + ProwJobRunTestTimestamp: ri.run.Timestamp, + ProwJobRunTestRelease: models.ReleasePresubmits, + } + if err := dbc.DB.Create(&output).Error; err != nil { + return fmt.Errorf("failed to create ProwJobRunTestOutput: %w", err) + } + } + + // Success result for network test + successResult := models.ProwJobRunTest{ + ProwJobRunID: ri.run.ID, + ProwJobID: ri.run.ProwJobID, + ProwJobRunRelease: models.ReleasePresubmits, + ProwJobRunTimestamp: ri.run.Timestamp, + TestID: networkTestID, + SuiteID: &suite.ID, + Status: int(v1.TestStatusSuccess), + Duration: 3.0, + CreatedAt: ri.run.Timestamp, + } + if err := dbc.DB.Create(&successResult).Error; err != nil { + return fmt.Errorf("failed to create success ProwJobRunTest: %w", err) + } + + // Success result for install test (used by include_successes=install e2e test) + installSuccessResult := models.ProwJobRunTest{ + ProwJobRunID: ri.run.ID, + ProwJobID: ri.run.ProwJobID, + ProwJobRunRelease: models.ReleasePresubmits, + ProwJobRunTimestamp: ri.run.Timestamp, + TestID: installTestID, + SuiteID: &suite.ID, + Status: int(v1.TestStatusSuccess), + Duration: 2.0, + CreatedAt: ri.run.Timestamp, + } + if err := dbc.DB.Create(&installSuccessResult).Error; err != nil { + return fmt.Errorf("failed to create install success ProwJobRunTest: %w", err) + } + + // Flake result for install test + flakeResult := models.ProwJobRunTest{ + ProwJobRunID: ri.run.ID, + ProwJobID: ri.run.ProwJobID, + ProwJobRunRelease: models.ReleasePresubmits, + ProwJobRunTimestamp: ri.run.Timestamp, + TestID: installTestID, + SuiteID: &suite.ID, + Status: int(v1.TestStatusFlake), + Duration: 4.0, + CreatedAt: ri.run.Timestamp, + } + if err := dbc.DB.Create(&flakeResult).Error; err != nil { + return fmt.Errorf("failed to create flake ProwJobRunTest: %w", err) + } + } + + log.Infof("Created presubmit seed data: %d jobs, %d PRs, %d runs", len(presubmitJobs), len(prs), len(runs)) + return nil +} + const syntheticViewsFile = "config/seed-views.yaml" // variantMapToArray converts a variant map to a pq.StringArray. diff --git a/pkg/api/job_runs.go b/pkg/api/job_runs.go index 2364b97b8e..c3f8a184a1 100644 --- a/pkg/api/job_runs.go +++ b/pkg/api/job_runs.go @@ -371,7 +371,7 @@ func JobRunRiskAnalysis( // master, so it should be safe to assume the latest compareRelease in the db. compareRelease := jobRun.ProwJob.Release neverStableJob := false - if compareRelease == "Presubmits" { + if compareRelease == models.ReleasePresubmits { ar, err := GetReleasesFromDB(ctx, dbc) if err != nil { return apitype.ProwJobRunRiskAnalysis{}, err diff --git a/pkg/api/prtestresults.go b/pkg/api/prtestresults.go index c9bdb4558d..642e502f25 100644 --- a/pkg/api/prtestresults.go +++ b/pkg/api/prtestresults.go @@ -1,324 +1,150 @@ package api import ( - "context" "fmt" "net/http" "strconv" "time" - "cloud.google.com/go/bigquery" - "cloud.google.com/go/civil" - "github.com/openshift/sippy/pkg/bigquery/bqlabel" - "github.com/pkg/errors" log "github.com/sirupsen/logrus" - "google.golang.org/api/iterator" - bq "github.com/openshift/sippy/pkg/bigquery" + sippyprocessingv1 "github.com/openshift/sippy/pkg/apis/sippyprocessing/v1" + "github.com/openshift/sippy/pkg/db" + "github.com/openshift/sippy/pkg/db/models" "github.com/openshift/sippy/pkg/util/param" ) -// PRTestResult represents a test result from a pull request job +// PRTestResult represents a test result from a pull request job run type PRTestResult struct { - ProwJobBuildID string `json:"prowjob_build_id"` - ProwJobName string `json:"prowjob_name"` - ProwJobURL string `json:"prowjob_url"` - PRSha string `json:"pr_sha"` - ProwJobStart time.Time `json:"prowjob_start"` - TestName string `json:"test_name"` - TestSuite string `json:"test_suite"` - Success bool `json:"success"` - Flaked bool `json:"flaked"` - FailureContent string `json:"failure_content"` + ProwJobRunID uint `json:"prow_job_run_id"` + ProwJobName string `json:"prow_job_name"` + ProwJobURL string `json:"prow_job_url"` + PRSha string `json:"pr_sha"` + ProwJobStart time.Time `json:"prow_job_start"` + TestName string `json:"test_name"` + TestSuite string `json:"test_suite"` + Status string `json:"status"` + Output string `json:"output,omitempty"` } -// GetPRTestResults fetches test failures for a specific pull request from BigQuery -// This queries both junit_pr and junit tables: -// - junit_pr: Contains results from normal presubmit jobs -// - junit: Contains results from /payload jobs (manually invoked jobs) -// Note: Only returns test failures (success = false), excluding flakes and passes -// includeSuccesses: Optional list of test name substrings to also include successes for -func GetPRTestResults(ctx context.Context, bqc *bq.Client, org, repo string, prNumber int, startDate, endDate time.Time, includeSuccesses []string) ([]PRTestResult, error) { - log.WithFields(log.Fields{ - "org": org, - "repo": repo, - "pr_number": prNumber, - "start_date": startDate.Format("2006-01-02"), - "end_date": endDate.Format("2006-01-02"), - "include_successes": includeSuccesses, - }).Info("querying test results for pull request") - - // Query junit_pr table (normal presubmit jobs) - log.Infof("querying junit_pr table for org=%s, repo=%s, pr_number=%d, start=%s, end=%s", - org, repo, prNumber, startDate.Format("2006-01-02"), endDate.Format("2006-01-02")) - queryPR := buildPRTestResultsQuery(bqc, org, repo, prNumber, startDate, endDate, "junit_pr", includeSuccesses) - bqc.ApplyQueryLabels(ctx, bqlabel.PRTestResults, queryPR) - resultsPR, err := executePRTestResultsQuery(ctx, queryPR) - if err != nil { - log.WithError(err).Error("error querying junit_pr table") - return nil, errors.Wrap(err, "failed to execute PR test results query for junit_pr table") - } - log.Infof("found %d test results from presubmit jobs (junit_pr)", len(resultsPR)) - - // Query junit table (/payload jobs) - log.Infof("querying junit table for org=%s, repo=%s, pr_number=%d, start=%s, end=%s", - org, repo, prNumber, startDate.Format("2006-01-02"), endDate.Format("2006-01-02")) - queryPayload := buildPRTestResultsQuery(bqc, org, repo, prNumber, startDate, endDate, "junit", includeSuccesses) - bqc.ApplyQueryLabels(ctx, bqlabel.PRTestResults, queryPayload) - resultsPayload, err := executePRTestResultsQuery(ctx, queryPayload) - if err != nil { - log.WithError(err).Error("error querying junit table") - return nil, errors.Wrap(err, "failed to execute PR test results query for junit table") - } - log.Infof("found %d test results from /payload jobs (junit)", len(resultsPayload)) - - // Combine results from both tables - allResults := append(resultsPR, resultsPayload...) - log.Infof("found %d total test results for PR %s/%s#%d", len(allResults), org, repo, prNumber) - return allResults, nil -} - -// buildPRTestResultsQuery constructs the BigQuery query to fetch test results for a PR -// junitTable should be either "junit_pr" (for normal presubmits) or "junit" (for /payload jobs) -// includeSuccesses: Optional list of test name substrings to also include successes for -func buildPRTestResultsQuery(bqc *bq.Client, org, repo string, prNumber int, startDate, endDate time.Time, junitTable string, includeSuccesses []string) *bigquery.Query { - // Query joins jobs and specified junit table, filtering by org/repo/pr_number - // Uses partitioning on prowjob_start and modified_time for efficiency - // Note: junit_pr contains normal presubmit jobs, junit contains /payload jobs - // - // The query de-dupes test results because junit tables model the XML directly, - // which means retried tests appear as multiple rows. We prefer: - // 1. Flakes (flake_count > 0) - test that failed then passed on retry - // 2. Successes (success_val > 0) - test that passed - // 3. Failures (else) - test that failed - - // Build the WHERE clause for including successes - // By default, only include failures (adjusted_flake_count = 0 AND adjusted_success_val = 0) - // If includeSuccesses is specified, also include successes for matching test names - whereClause := ` - AND ( - (deduped.adjusted_flake_count = 0 AND deduped.adjusted_success_val = 0)` - - if len(includeSuccesses) > 0 { - whereClause += ` - OR ( - deduped.adjusted_success_val > 0 - AND (` - for i := range includeSuccesses { - if i > 0 { - whereClause += " OR " - } - whereClause += fmt.Sprintf("deduped.test_name LIKE @IncludeSuccess%d", i) - } - whereClause += ` - ) - )` - } - whereClause += ` - )` - - queryString := fmt.Sprintf(` - WITH deduped_testcases AS ( - SELECT - junit.*, - ROW_NUMBER() OVER(PARTITION BY prowjob_build_id, file_path, test_name, testsuite ORDER BY - CASE - WHEN flake_count > 0 THEN 0 - WHEN success_val > 0 THEN 1 - ELSE 2 - END) AS row_num, - CASE - WHEN flake_count > 0 THEN 0 - ELSE success_val - END AS adjusted_success_val, - CASE - WHEN flake_count > 0 THEN 1 - ELSE 0 - END AS adjusted_flake_count - FROM - %s.%s AS junit - WHERE - junit.modified_time >= DATETIME(@StartDate) - AND junit.modified_time < DATETIME(@EndDate) - AND junit.skipped = false - ) - SELECT - jobs.prowjob_build_id, - jobs.prowjob_job_name AS prowjob_name, - jobs.prowjob_url, - jobs.pr_sha, - jobs.prowjob_start, - deduped.test_name, - deduped.testsuite, - CASE - WHEN deduped.adjusted_flake_count > 0 THEN TRUE - ELSE FALSE - END AS flaked, - CASE - WHEN deduped.adjusted_flake_count > 0 THEN TRUE - WHEN deduped.adjusted_success_val > 0 THEN TRUE - ELSE FALSE - END AS success, - deduped.failure_content - FROM - %s.jobs AS jobs - INNER JOIN - deduped_testcases AS deduped - ON - jobs.prowjob_build_id = deduped.prowjob_build_id - AND deduped.row_num = 1 - WHERE - jobs.org = @Org - AND jobs.repo = @Repo - AND jobs.pr_number = @PRNumber - AND jobs.prowjob_start >= DATETIME(@StartDate) - AND jobs.prowjob_start < DATETIME(@EndDate)%s - ORDER BY - jobs.prowjob_start DESC, - deduped.test_name ASC - `, bqc.Dataset, junitTable, bqc.Dataset, whereClause) - - query := bqc.BQ.Query(queryString) - query.Parameters = []bigquery.QueryParameter{ - { - Name: "Org", - Value: org, - }, - { - Name: "Repo", - Value: repo, - }, - { - Name: "PRNumber", - Value: strconv.Itoa(prNumber), - }, - { - Name: "StartDate", - Value: startDate, - }, - { - Name: "EndDate", - Value: endDate, - }, +func testStatusString(status int) string { + switch sippyprocessingv1.TestStatus(status) { + case sippyprocessingv1.TestStatusSuccess: + return "success" + case sippyprocessingv1.TestStatusFlake: + return "flake" + case sippyprocessingv1.TestStatusFailure: + return "failure" + default: + return fmt.Sprintf("unknown(%d)", status) } - - // Add parameters for includeSuccesses LIKE clauses - for i, testName := range includeSuccesses { - query.Parameters = append(query.Parameters, bigquery.QueryParameter{ - Name: fmt.Sprintf("IncludeSuccess%d", i), - Value: "%" + testName + "%", // Wrap in % for SQL LIKE partial matching - }) - } - - return query } -// executePRTestResultsQuery executes the BigQuery query and returns the results -func executePRTestResultsQuery(ctx context.Context, query *bigquery.Query) ([]PRTestResult, error) { - results := []PRTestResult{} +// GetPRTestResults fetches test results for a specific pull request from PostgreSQL. +const defaultPRTestResultsLimit = 10000 - it, err := query.Read(ctx) - if err != nil { - return nil, errors.Wrap(err, "error reading from bigquery") +func GetPRTestResults(dbc *db.DB, org, repo string, prNumber int, latestSHAOnly bool, startDate, endDate time.Time, includeSuccesses []string, limit int) ([]PRTestResult, error) { + if limit <= 0 || limit > defaultPRTestResultsLimit { + limit = defaultPRTestResultsLimit } + log.WithFields(log.Fields{ + "org": org, + "repo": repo, + "pr_number": prNumber, + "latest_sha_only": latestSHAOnly, + "start_date": startDate.Format("2006-01-02"), + "end_date": endDate.Format("2006-01-02"), + }).Info("querying test results for pull request") - for { - var row []bigquery.Value - err := it.Next(&row) - if err == iterator.Done { - break - } - if err != nil { - return nil, errors.Wrap(err, "error iterating bigquery results") - } - - result, err := deserializePRTestResult(row, it.Schema) - if err != nil { - log.WithError(err).Warn("error deserializing row, skipping") - continue + // Start from the PR side and use partition keys (release, timestamp) on + // prow_job_run_tests to allow PostgreSQL to prune partitions and avoid + // locking every partition in the table. + query := dbc.DB.Table("prow_pull_requests pp"). + Select(`pjr.id AS prow_job_run_id, + pj.name AS prow_job_name, + pjr.url AS prow_job_url, + pp.sha AS pr_sha, + pjr.timestamp AS prow_job_start, + t.name AS test_name, + COALESCE(s.name, '') AS test_suite, + pjrt.status, + COALESCE(pjrto.output, '') AS output`). + Joins("JOIN prow_job_run_prow_pull_requests jrpr ON jrpr.prow_pull_request_id = pp.id AND jrpr.prow_job_run_release = ? AND jrpr.prow_job_run_timestamp >= ? AND jrpr.prow_job_run_timestamp < ?", models.ReleasePresubmits, startDate, endDate). + Joins("JOIN prow_job_runs pjr ON pjr.id = jrpr.prow_job_run_id"). + Joins("JOIN prow_jobs pj ON pj.id = pjr.prow_job_id AND pj.release = ?", models.ReleasePresubmits). + Joins("JOIN prow_job_run_tests pjrt ON pjrt.prow_job_run_id = pjr.id AND pjrt.prow_job_run_release = ? AND pjrt.prow_job_run_timestamp >= ? AND pjrt.prow_job_run_timestamp < ?", models.ReleasePresubmits, startDate, endDate). + Joins("JOIN tests t ON t.id = pjrt.test_id"). + Joins("LEFT JOIN suites s ON s.id = pjrt.suite_id"). + Joins("LEFT JOIN prow_job_run_test_outputs pjrto ON pjrto.prow_job_run_test_id = pjrt.id AND pjrto.prow_job_run_test_timestamp = pjrt.prow_job_run_timestamp AND pjrto.prow_job_run_test_release = pjrt.prow_job_run_release"). + Where("pp.org = ? AND pp.repo = ? AND pp.number = ?", org, repo, prNumber). + Where("pp.deleted_at IS NULL AND pjr.deleted_at IS NULL AND pj.deleted_at IS NULL"). + Where("pjr.timestamp >= ? AND pjr.timestamp < ?", startDate, endDate) + + if latestSHAOnly { + query = query.Where("pp.sha = (SELECT pp2.sha FROM prow_pull_requests pp2 JOIN prow_job_run_prow_pull_requests jrpr2 ON jrpr2.prow_pull_request_id = pp2.id JOIN prow_job_runs pjr2 ON pjr2.id = jrpr2.prow_job_run_id WHERE pp2.org = ? AND pp2.repo = ? AND pp2.number = ? ORDER BY pjr2.timestamp DESC LIMIT 1)", org, repo, prNumber) + } + + // By default only return failures (no flakes, no successes). + // include_successes adds successes for matching test names (flakes + // are excluded to match the previous BigQuery implementation). + if len(includeSuccesses) == 0 { + query = query.Where("pjrt.status = ?", int(sippyprocessingv1.TestStatusFailure)) + } else { + conditions := dbc.DB.Where("pjrt.status = ?", int(sippyprocessingv1.TestStatusFailure)) + for _, pattern := range includeSuccesses { + conditions = conditions.Or("pjrt.status = ? AND t.name LIKE ?", int(sippyprocessingv1.TestStatusSuccess), "%"+pattern+"%") } - - results = append(results, result) + query = query.Where(conditions) + } + + query = query.Order("pjr.timestamp DESC, t.name ASC").Limit(limit) + + type rawRow struct { + ProwJobRunID uint `gorm:"column:prow_job_run_id"` + ProwJobName string `gorm:"column:prow_job_name"` + ProwJobURL string `gorm:"column:prow_job_url"` + PRSha string `gorm:"column:pr_sha"` + ProwJobStart time.Time `gorm:"column:prow_job_start"` + TestName string `gorm:"column:test_name"` + TestSuite string `gorm:"column:test_suite"` + Status int `gorm:"column:status"` + Output string `gorm:"column:output"` + } + + var rows []rawRow + if err := query.Find(&rows).Error; err != nil { + return nil, fmt.Errorf("error querying PR test results: %w", err) + } + + results := make([]PRTestResult, 0, len(rows)) + for _, r := range rows { + results = append(results, PRTestResult{ + ProwJobRunID: r.ProwJobRunID, + ProwJobName: r.ProwJobName, + ProwJobURL: r.ProwJobURL, + PRSha: r.PRSha, + ProwJobStart: r.ProwJobStart, + TestName: r.TestName, + TestSuite: r.TestSuite, + Status: testStatusString(r.Status), + Output: r.Output, + }) } + log.Infof("found %d test results for PR %s/%s#%d", len(results), org, repo, prNumber) return results, nil } -// deserializePRTestResult converts a BigQuery row into a PRTestResult -func deserializePRTestResult(row []bigquery.Value, schema bigquery.Schema) (PRTestResult, error) { - if len(row) != len(schema) { - return PRTestResult{}, fmt.Errorf("row length %d does not match schema length %d", len(row), len(schema)) - } - - result := PRTestResult{} - - for i, field := range schema { - switch field.Name { - case "prowjob_build_id": - if row[i] != nil { - result.ProwJobBuildID = row[i].(string) - } - case "prowjob_name": - if row[i] != nil { - result.ProwJobName = row[i].(string) - } - case "prowjob_url": - if row[i] != nil { - result.ProwJobURL = row[i].(string) - } - case "pr_sha": - if row[i] != nil { - result.PRSha = row[i].(string) - } - case "prowjob_start": - if row[i] != nil { - // BigQuery returns civil.DateTime for DATETIME columns - civilDT := row[i].(civil.DateTime) - layout := "2006-01-02T15:04:05" - parsedTime, err := time.Parse(layout, civilDT.String()) - if err != nil { - return PRTestResult{}, errors.Wrap(err, "failed to parse prowjob_start") - } - result.ProwJobStart = parsedTime - } - case "test_name": - if row[i] != nil { - result.TestName = row[i].(string) - } - case "testsuite": - if row[i] != nil { - result.TestSuite = row[i].(string) - } - case "flaked": - if row[i] != nil { - result.Flaked = row[i].(bool) - } - case "success": - if row[i] != nil { - result.Success = row[i].(bool) - } - case "failure_content": - if row[i] != nil { - result.FailureContent = row[i].(string) - } - } - } - - return result, nil -} - // PrintPRTestResultsJSON is the HTTP handler for /api/pull_requests/test_results -func PrintPRTestResultsJSON(w http.ResponseWriter, req *http.Request, bqc *bq.Client) { - // Parse and validate query parameters +func PrintPRTestResultsJSON(w http.ResponseWriter, req *http.Request, dbc *db.DB) { org := param.SafeRead(req, "org") if org == "" { - // Default to openshift. org = "openshift" } repo := param.SafeRead(req, "repo") if repo == "" { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ "code": http.StatusBadRequest, "message": "required parameter 'repo' is missing", }) @@ -327,7 +153,7 @@ func PrintPRTestResultsJSON(w http.ResponseWriter, req *http.Request, bqc *bq.Cl prNumberStr := param.SafeRead(req, "pr_number") if prNumberStr == "" { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ "code": http.StatusBadRequest, "message": "required parameter 'pr_number' is missing", }) @@ -336,80 +162,73 @@ func PrintPRTestResultsJSON(w http.ResponseWriter, req *http.Request, bqc *bq.Cl prNumber, err := strconv.Atoi(prNumberStr) if err != nil { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ "code": http.StatusBadRequest, "message": fmt.Sprintf("invalid pr_number: %v", err), }) return } - startDateStr := param.SafeRead(req, "start_date") - if startDateStr == "" { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ - "code": http.StatusBadRequest, - "message": "required parameter 'start_date' is missing (format: YYYY-MM-DD)", - }) - return - } + now := time.Now().UTC() + startDate := now.AddDate(0, 0, -14) + endDate := now.AddDate(0, 0, 1) - startDate, err := time.Parse("2006-01-02", startDateStr) - if err != nil { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ - "code": http.StatusBadRequest, - "message": fmt.Sprintf("invalid start_date format (expected YYYY-MM-DD): %v", err), - }) - return + startDateStr := param.SafeRead(req, "start_date") + if startDateStr != "" { + parsed, err := time.Parse("2006-01-02", startDateStr) + if err != nil { + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ + "code": http.StatusBadRequest, + "message": fmt.Sprintf("invalid start_date format (expected YYYY-MM-DD): %v", err), + }) + return + } + startDate = parsed } endDateStr := param.SafeRead(req, "end_date") - if endDateStr == "" { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ - "code": http.StatusBadRequest, - "message": "required parameter 'end_date' is missing (format: YYYY-MM-DD)", - }) - return - } - - endDate, err := time.Parse("2006-01-02", endDateStr) - if err != nil { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ - "code": http.StatusBadRequest, - "message": fmt.Sprintf("invalid end_date format (expected YYYY-MM-DD): %v", err), - }) - return + if endDateStr != "" { + parsed, err := time.Parse("2006-01-02", endDateStr) + if err != nil { + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ + "code": http.StatusBadRequest, + "message": fmt.Sprintf("invalid end_date format (expected YYYY-MM-DD): %v", err), + }) + return + } + // Make end_date inclusive + endDate = parsed.AddDate(0, 0, 1) } - // Validate date range if endDate.Before(startDate) { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ "code": http.StatusBadRequest, "message": "end_date must be after start_date", }) return } - // Add one day to end_date to make it inclusive - endDate = endDate.AddDate(0, 0, 1) - - // Limit date range to 30 days to prevent expensive queries - maxDuration := 30 * 24 * time.Hour - duration := endDate.Sub(startDate) - if duration > maxDuration { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ + latestSHAOnly, err := param.ReadBool(req, "latest_sha_only", false) + if err != nil { + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ "code": http.StatusBadRequest, - "message": fmt.Sprintf("date range too large: %d days (maximum is 30 days)", int(duration.Hours()/24)), + "message": fmt.Sprintf("invalid latest_sha_only: %v", err), }) return } - - // Parse optional include_successes parameter (multi-valued) includeSuccesses := req.URL.Query()["include_successes"] - // Execute query - results, err := GetPRTestResults(req.Context(), bqc, org, repo, prNumber, startDate, endDate, includeSuccesses) + limit := defaultPRTestResultsLimit + if limitStr := param.SafeRead(req, "limit"); limitStr != "" { + if parsed, err := strconv.Atoi(limitStr); err == nil { + limit = parsed + } + } + + results, err := GetPRTestResults(dbc, org, repo, prNumber, latestSHAOnly, startDate, endDate, includeSuccesses, limit) if err != nil { log.WithError(err).Error("error fetching PR test results") - RespondWithJSON(http.StatusInternalServerError, w, map[string]interface{}{ + RespondWithJSON(http.StatusInternalServerError, w, map[string]any{ "code": http.StatusInternalServerError, "message": fmt.Sprintf("error fetching test results: %v", err), }) diff --git a/pkg/dataloader/prowloader/bigqueryjobs.go b/pkg/dataloader/prowloader/bigqueryjobs.go index 8730a201e4..f8e1ddc462 100644 --- a/pkg/dataloader/prowloader/bigqueryjobs.go +++ b/pkg/dataloader/prowloader/bigqueryjobs.go @@ -1,6 +1,7 @@ package prowloader import ( + "fmt" "strconv" "strings" "time" @@ -129,11 +130,56 @@ func (pl *ProwLoader) fetchProwJobsFromOpenShiftBigQuery() ([]prow.ProwJob, []er filteredAnnotations[key] = value } + jobName := bqjr.JobName + jobType := bqjr.Type + + // Detect /payload sub-jobs: they have a releaseJobName annotation and PR data, + // and are run as periodic jobs. Transform them into presubmit-style records so + // they appear on presubmit UI pages. Skip aggregator jobs (they just orchestrate + // sub-jobs and don't have test results themselves). + if releaseJobName, ok := filteredAnnotations["releaseJobName"]; ok && + refs != nil && + !strings.HasPrefix(bqjr.JobName, "aggregator-") { + + // Normalize releaseJobName: the BQ query generator splits on comma + // to strip suffixes, so we must do the same here. + releaseJobName = strings.SplitN(releaseJobName, ",", 2)[0] + filteredAnnotations["releaseJobName"] = releaseJobName + + prNumber := bqjr.PRNumber.StringVal + org := bqjr.PROrg.StringVal + repo := bqjr.PRRepo.StringVal + + // Strip the PR number from the sub-job name to create a stable name + // shared across all PRs running the same canonical job. + // e.g. openshift-origin-31301-ci-5.0-... -> openshift-origin-ci-5.0-... + prPrefix := fmt.Sprintf("%s-%s-%s-", org, repo, prNumber) + stablePrefix := fmt.Sprintf("%s-%s-", org, repo) + stableName := strings.Replace(bqjr.JobName, prPrefix, stablePrefix, 1) + + if stableName == bqjr.JobName { + // PR number not found in job name pattern, use releaseJobName-based fallback + stableName = "payload-pr-" + releaseJobName + log.WithField("job", bqjr.JobName). + WithField("releaseJobName", releaseJobName). + Warningf("could not strip PR number from /payload sub-job name, using fallback") + } + + jobName = stableName + jobType = "presubmit" + + log.WithField("originalName", bqjr.JobName). + WithField("stableName", stableName). + WithField("releaseJobName", releaseJobName). + WithField("prNumber", prNumber). + Debugf("transformed /payload sub-job for presubmit ingestion") + } + prowJobs[bqjr.BuildID] = prow.ProwJob{ Spec: prow.ProwJobSpec{ - Type: bqjr.Type, + Type: jobType, Cluster: bqjr.Cluster, - Job: bqjr.JobName, + Job: jobName, Refs: refs, DecorationConfig: prow.DecorationConfig{ GCSConfiguration: prow.GCSConfiguration{ diff --git a/pkg/dataloader/prowloader/prow.go b/pkg/dataloader/prowloader/prow.go index 6f06e3084f..ad31106d35 100644 --- a/pkg/dataloader/prowloader/prow.go +++ b/pkg/dataloader/prowloader/prow.go @@ -11,6 +11,7 @@ import ( "os" "regexp" "strconv" + "strings" "sync" "sync/atomic" "time" @@ -602,8 +603,18 @@ ORDER BY } // matchRelease returns the release a prow job belongs to, or "" if it -// doesn't match any configured release. -func (pl *ProwLoader) matchRelease(jobName string) string { +// doesn't match any configured release. For /payload sub-jobs (identified +// by releaseJobName annotation + PR refs), it returns the Presubmits +// pseudo-release. +func (pl *ProwLoader) matchRelease(pj *prow.ProwJob) string { + if _, ok := pj.Annotations["releaseJobName"]; ok && pj.Spec.Refs != nil { + if pl.releaseSet.Has(models.ReleasePresubmits) { + return models.ReleasePresubmits + } + return "" + } + + jobName := pj.Spec.Job if release, ok := pl.syntheticReleaseJobOverrides.Lookup(jobName); ok { if pl.releaseSet.Has(release) { return release @@ -628,6 +639,12 @@ func (pl *ProwLoader) matchRelease(jobName string) string { return "" } +// isPayloadPresubmit returns true if the prow job is a /payload sub-job. +func isPayloadPresubmit(pj *prow.ProwJob) bool { + _, hasAnnotation := pj.Annotations["releaseJobName"] + return hasAnnotation && pj.Spec.Refs != nil +} + // preprocessProwJobs matches each BigQuery prow job to a release, filters out // already-processed runs and non-terminal states, bulk-upserts ProwJob // definitions, and returns only entries that need GCS fetching. @@ -650,7 +667,7 @@ func (pl *ProwLoader) preprocessProwJobs(ctx context.Context, prowJobs []prow.Pr continue } - release := pl.matchRelease(pj.Spec.Job) + release := pl.matchRelease(pj) if release == "" { continue } @@ -668,12 +685,36 @@ func (pl *ProwLoader) preprocessProwJobs(ctx context.Context, prowJobs []prow.Pr } seenJobs.Insert(pj.Spec.Job) + variantJobName := pj.Spec.Job + isPayload := isPayloadPresubmit(pj) + if isPayload { + variantJobName = pj.Annotations["releaseJobName"] + } + + variants := pl.variantManager.IdentifyVariants(variantJobName) + if isPayload { + for vi, v := range variants { + parts := strings.SplitN(v, ":", 2) + if len(parts) == 2 { + if _, isRel := pl.config.Releases[parts[1]]; isRel { + variants[vi] = parts[0] + ":" + models.ReleasePresubmits + break + } + } + } + } + + testGridURL := "" + if !isPayload { + testGridURL = pl.generateTestGridURL(release, pj.Spec.Job).String() + } + jobDefs = append(jobDefs, models.ProwJob{ Name: pj.Spec.Job, Kind: models.ProwKind(pj.Spec.Type), Release: release, - Variants: pl.variantManager.IdentifyVariants(pj.Spec.Job), - TestGridURL: pl.generateTestGridURL(release, pj.Spec.Job).String(), + Variants: variants, + TestGridURL: testGridURL, }) } diff --git a/pkg/dataloader/prowloader/prow_test.go b/pkg/dataloader/prowloader/prow_test.go index da53380ebe..2d63eb3f46 100644 --- a/pkg/dataloader/prowloader/prow_test.go +++ b/pkg/dataloader/prowloader/prow_test.go @@ -1,10 +1,17 @@ package prowloader import ( + "regexp" "testing" "time" "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/util/sets" + + v1config "github.com/openshift/sippy/pkg/apis/config/v1" + "github.com/openshift/sippy/pkg/apis/prow" + "github.com/openshift/sippy/pkg/db/models" + "github.com/openshift/sippy/pkg/releaseoverride" ) func TestDateTimeNameComparisons(t *testing.T) { @@ -178,3 +185,130 @@ func TestGetTestAnalysisByJobFromToDates(t *testing.T) { }) } } + +func TestIsPayloadPresubmit(t *testing.T) { + tests := []struct { + name string + pj *prow.ProwJob + expected bool + }{ + { + name: "has annotation and refs", + pj: &prow.ProwJob{ + Annotations: map[string]string{"releaseJobName": "periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn"}, + Spec: prow.ProwJobSpec{Refs: &prow.Refs{Org: "openshift", Repo: "origin"}}, + }, + expected: true, + }, + { + name: "has annotation but nil refs", + pj: &prow.ProwJob{ + Annotations: map[string]string{"releaseJobName": "some-job"}, + Spec: prow.ProwJobSpec{}, + }, + expected: false, + }, + { + name: "no annotation but has refs", + pj: &prow.ProwJob{ + Annotations: map[string]string{}, + Spec: prow.ProwJobSpec{Refs: &prow.Refs{Org: "openshift", Repo: "origin"}}, + }, + expected: false, + }, + { + name: "no annotation and nil refs", + pj: &prow.ProwJob{ + Annotations: map[string]string{}, + Spec: prow.ProwJobSpec{}, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, isPayloadPresubmit(tt.pj)) + }) + } +} + +func TestMatchRelease(t *testing.T) { + tests := []struct { + name string + pl *ProwLoader + pj *prow.ProwJob + expected string + }{ + { + name: "payload presubmit with Presubmits in release set", + pl: &ProwLoader{ + releaseSet: sets.New[string](models.ReleasePresubmits), + releases: []string{models.ReleasePresubmits}, + config: &v1config.SippyConfig{Releases: map[string]v1config.ReleaseConfig{}}, + releaseRegexps: map[string][]*regexp.Regexp{}, + syntheticReleaseJobOverrides: releaseoverride.New(), + }, + pj: &prow.ProwJob{ + Annotations: map[string]string{"releaseJobName": "periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn"}, + Spec: prow.ProwJobSpec{Refs: &prow.Refs{Org: "openshift", Repo: "origin"}}, + }, + expected: models.ReleasePresubmits, + }, + { + name: "payload presubmit without Presubmits in release set", + pl: &ProwLoader{ + releaseSet: sets.New[string]("4.18"), + releases: []string{"4.18"}, + config: &v1config.SippyConfig{Releases: map[string]v1config.ReleaseConfig{}}, + releaseRegexps: map[string][]*regexp.Regexp{}, + syntheticReleaseJobOverrides: releaseoverride.New(), + }, + pj: &prow.ProwJob{ + Annotations: map[string]string{"releaseJobName": "some-job"}, + Spec: prow.ProwJobSpec{Refs: &prow.Refs{Org: "openshift", Repo: "origin"}}, + }, + expected: "", + }, + { + name: "regular job matching configured release by regex", + pl: &ProwLoader{ + releaseSet: sets.New[string]("4.18"), + releases: []string{"4.18"}, + config: &v1config.SippyConfig{Releases: map[string]v1config.ReleaseConfig{ + "4.18": {}, + }}, + releaseRegexps: map[string][]*regexp.Regexp{"4.18": {regexp.MustCompile(`-4\.18-`)}}, + syntheticReleaseJobOverrides: releaseoverride.New(), + }, + pj: &prow.ProwJob{ + Annotations: map[string]string{}, + Spec: prow.ProwJobSpec{Job: "periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn"}, + }, + expected: "4.18", + }, + { + name: "regular job matching no release", + pl: &ProwLoader{ + releaseSet: sets.New[string]("4.18"), + releases: []string{"4.18"}, + config: &v1config.SippyConfig{Releases: map[string]v1config.ReleaseConfig{ + "4.18": {}, + }}, + releaseRegexps: map[string][]*regexp.Regexp{"4.18": {regexp.MustCompile(`-4\.18-`)}}, + syntheticReleaseJobOverrides: releaseoverride.New(), + }, + pj: &prow.ProwJob{ + Annotations: map[string]string{}, + Spec: prow.ProwJobSpec{Job: "periodic-ci-openshift-release-master-nightly-4.17-e2e-gcp"}, + }, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.pl.matchRelease(tt.pj)) + }) + } +} diff --git a/pkg/db/models/prow.go b/pkg/db/models/prow.go index d8c1d23943..631eede57f 100644 --- a/pkg/db/models/prow.go +++ b/pkg/db/models/prow.go @@ -85,7 +85,7 @@ type ProwJobRun struct { // ProwJobRun for query optimization. type ProwJobRunProwPullRequest struct { ProwJobRunID uint `gorm:"primaryKey"` - ProwPullRequestID uint `gorm:"primaryKey"` + ProwPullRequestID uint `gorm:"primaryKey;index:idx_prow_job_run_prow_pull_requests_pr_id"` ProwJobRunRelease string `gorm:"index:idx_prow_job_run_prow_pull_requests_release_timestamp"` ProwJobRunTimestamp time.Time `gorm:"index:idx_prow_job_run_prow_pull_requests_release_timestamp"` } @@ -265,11 +265,11 @@ type ProwPullRequest struct { Model // Org is something like kubernetes or k8s.io - Org string `json:"org"` + Org string `json:"org" gorm:"index:idx_prow_pull_requests_org_repo_number"` // Repo is something like test-infra - Repo string `json:"repo"` + Repo string `json:"repo" gorm:"index:idx_prow_pull_requests_org_repo_number"` - Number int `json:"number"` + Number int `json:"number" gorm:"index:idx_prow_pull_requests_org_repo_number"` Author string `json:"author"` Title string `json:"title,omitempty"` diff --git a/pkg/db/models/releases.go b/pkg/db/models/releases.go index 01cc7c62c1..fcd904bc1a 100644 --- a/pkg/db/models/releases.go +++ b/pkg/db/models/releases.go @@ -26,6 +26,8 @@ type ReleaseDefinition struct { } const ( + ReleasePresubmits = "Presubmits" + CapComponentReadiness = "componentReadiness" CapSippyClassic = "sippyClassic" CapMetrics = "metrics" diff --git a/pkg/sippyserver/server.go b/pkg/sippyserver/server.go index 3107a2fb2c..e6957633a6 100644 --- a/pkg/sippyserver/server.go +++ b/pkg/sippyserver/server.go @@ -1412,11 +1412,7 @@ func (s *Server) jsonPullRequestsReportFromDB(w http.ResponseWriter, req *http.R } func (s *Server) jsonPullRequestTestResults(w http.ResponseWriter, req *http.Request) { - if s.bigQueryClient == nil { - failureResponse(w, http.StatusBadRequest, "pull request test results API is only available when google-service-account-credential-file is configured") - return - } - api.PrintPRTestResultsJSON(w, req, s.bigQueryClient) + api.PrintPRTestResultsJSON(w, req, s.db) } func (s *Server) jsonJobRunSummary(w http.ResponseWriter, req *http.Request) { @@ -2565,13 +2561,11 @@ func (s *Server) Serve() { HandlerFunc: s.jsonPullRequestsReportFromDB, }, { - EndpointPath: "/api/pull_requests/test_results", - Description: "Fetches test failures for a specific pull request from BigQuery (presubmits and /payload jobs). Optional: include_successes param to also return successes for matching test names", - Capabilities: []string{ComponentReadinessCapability}, - HandlerFunc: s.jsonPullRequestTestResults, - CacheTime: 1 * time.Hour, - RateLimitRequests: 20, - RateLimitPeriod: 1 * time.Hour, + EndpointPath: "/api/pull_requests/test_results", + Description: "Fetches test results for a specific pull request from PostgreSQL (presubmit and /payload jobs)", + Capabilities: []string{LocalDBCapability}, + HandlerFunc: s.jsonPullRequestTestResults, + CacheTime: 1 * time.Minute, }, { EndpointPath: "/api/repositories", diff --git a/pkg/util/param/param.go b/pkg/util/param/param.go index 2e5164e6d5..aea0d0aae2 100644 --- a/pkg/util/param/param.go +++ b/pkg/util/param/param.go @@ -76,6 +76,8 @@ var paramRegexp = map[string]*regexp.Regexp{ // recent test failures params "previousPeriod": wordRegexp, "includeOutputs": boolRegexp, + // pull request test results params + "limit": uintRegexp, } // SafeRead returns the value of a query parameter only if it matches the given regexp. diff --git a/pkg/variantregistry/ocp.go b/pkg/variantregistry/ocp.go index cb1cd50655..605d451556 100644 --- a/pkg/variantregistry/ocp.go +++ b/pkg/variantregistry/ocp.go @@ -24,6 +24,7 @@ import ( v1 "github.com/openshift/sippy/pkg/apis/config/v1" "github.com/openshift/sippy/pkg/dataloader/prowloader" "github.com/openshift/sippy/pkg/dataloader/prowloader/gcs" + "github.com/openshift/sippy/pkg/db/models" "github.com/openshift/sippy/pkg/releaseoverride" "github.com/openshift/sippy/pkg/util" ) @@ -696,9 +697,9 @@ func setNetworkStack(_ logrus.FieldLogger, variants map[string]string, jobName s } func (v *OCPVariantLoader) setRelease(logger logrus.FieldLogger, variants map[string]string, jobName string) { - // Presubmits on main branch are set as "Presubmits" + // Presubmits on main branch use the Presubmits pseudo-release if presubmitRegex.MatchString(jobName) { - variants[VariantRelease] = "Presubmits" + variants[VariantRelease] = models.ReleasePresubmits return } @@ -955,7 +956,7 @@ func (v *OCPVariantLoader) setJobTier(_ logrus.FieldLogger, variants map[string] variants[VariantJobTier] = "blocking" case util.StrSliceContainsEither(v.config.Releases[release].InformingJobs, jobName, mainJobName): variants[VariantJobTier] = "informing" - case release == "Presubmits", v.config.Releases[release].Jobs[jobName], v.config.Releases[release].Jobs[mainJobName]: + case release == models.ReleasePresubmits, v.config.Releases[release].Jobs[jobName], v.config.Releases[release].Jobs[mainJobName]: variants[VariantJobTier] = "standard" default: variants[VariantJobTier] = "candidate" diff --git a/test/e2e/pull_requests_test.go b/test/e2e/pull_requests_test.go new file mode 100644 index 0000000000..4d136edbab --- /dev/null +++ b/test/e2e/pull_requests_test.go @@ -0,0 +1,101 @@ +package e2e + +import ( + "testing" + + "github.com/openshift/sippy/pkg/api" + "github.com/openshift/sippy/test/e2e/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func prTestResultsPath(extra string) string { + return "/api/pull_requests/test_results?org=openshift&repo=origin&pr_number=99001" + extra +} + +func TestPRTestResultsDefaultFailuresOnly(t *testing.T) { + var results []api.PRTestResult + err := util.SippyGet(prTestResultsPath(""), &results) + require.NoError(t, err, "error making http request") + require.Greater(t, len(results), 0, "expected at least one result") + + for _, r := range results { + assert.Equal(t, "failure", r.Status, "default query should only return failures, got %s for test %s", r.Status, r.TestName) + } +} + +func TestPRTestResultsIncludeSuccesses(t *testing.T) { + var results []api.PRTestResult + err := util.SippyGet(prTestResultsPath("&include_successes=install"), &results) + require.NoError(t, err, "error making http request") + require.Greater(t, len(results), 0, "expected at least one result") + + statuses := map[string]int{} + for _, r := range results { + statuses[r.Status]++ + } + + assert.Greater(t, statuses["failure"], 0, "should still have failures") + // include_successes=install should match "install should succeed: overall" and return + // successes for it alongside the failures (flakes are excluded) + assert.Greater(t, statuses["success"], 0, "should have successes for matching tests") + assert.Equal(t, 0, statuses["flake"], "flakes should be excluded from include_successes results") +} + +func TestPRTestResultsMultiplePRs(t *testing.T) { + var results []api.PRTestResult + err := util.SippyGet("/api/pull_requests/test_results?org=openshift&repo=origin&pr_number=99002", &results) + require.NoError(t, err, "error making http request") + require.Greater(t, len(results), 0, "expected results for PR 99002") + + for _, r := range results { + assert.Contains(t, r.ProwJobName, "gcp", "PR 99002 should be linked to GCP job, got %s", r.ProwJobName) + } +} + +func TestPRTestResultsLatestSHAOnly(t *testing.T) { + var results []api.PRTestResult + err := util.SippyGet(prTestResultsPath("&latest_sha_only=true"), &results) + require.NoError(t, err, "error making http request") + require.Greater(t, len(results), 0, "expected results for latest SHA") + + // All results should share the same SHA (the one from the most recent job run) + sha := results[0].PRSha + for _, r := range results { + assert.Equal(t, sha, r.PRSha, "all results should have the same SHA when latest_sha_only=true") + } +} + +func TestPRTestResultsDefaultDateRange(t *testing.T) { + // Query without start_date/end_date should work (defaults to last 14 days) + var results []api.PRTestResult + err := util.SippyGet(prTestResultsPath(""), &results) + require.NoError(t, err, "error making http request without date params") + assert.Greater(t, len(results), 0, "expected results with default date range") +} + +func TestPRTestResultsMissingParams(t *testing.T) { + tests := []struct { + name string + path string + }{ + {"missing repo", "/api/pull_requests/test_results?org=openshift&pr_number=1"}, + {"missing pr_number", "/api/pull_requests/test_results?org=openshift&repo=origin"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var result any + err := util.SippyGet(tt.path, &result) + assert.Error(t, err, "expected error for %s", tt.name) + assert.Contains(t, err.Error(), "400", "expected 400 status code") + }) + } +} + +func TestPRTestResultsEmptyResults(t *testing.T) { + var results []api.PRTestResult + err := util.SippyGet("/api/pull_requests/test_results?org=openshift&repo=origin&pr_number=99999", &results) + require.NoError(t, err, "error making http request") + assert.Empty(t, results, "expected empty results for non-existent PR") +}