Skip to content

TRT-2821: Use cumulative summaries for findLastPass in recent test failures - #3877

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
mstaeble:TRT-2821-recent-test-failures-summary-tables
Aug 5, 2026
Merged

TRT-2821: Use cumulative summaries for findLastPass in recent test failures#3877
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
mstaeble:TRT-2821-recent-test-failures-summary-tables

Conversation

@mstaeble

@mstaeble mstaeble commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace the day-by-day backwards loop in findLastPass (up to 90 queries over prow_job_run_tests) with a single query on test_cumulative_summaries using prefix_max_last_success
  • No API signature or frontend changes; only the internal findLastPass implementation is rewritten
  • Add integration tests for the recent test failures API

Test plan

  • Integration tests pass (6 tests covering failure counts, previous-period exclusion, new regressions, last-pass from cumulative summary, multi-job aggregation, suite separation)
  • Benchmarked against prod-cloned staging DB (release 5.0, 14-day lookback): ~440ms warm cache
  • Verify last_pass populates correctly in staging after deployment

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Improvements

    • Improved recent test failure reporting with more accurate last-pass timestamps.
    • Failure counts are now aggregated consistently across multiple jobs.
    • Test failures are reported separately by suite for clearer results.
    • New regressions are surfaced while tests failing in both comparison periods are excluded.
  • Tests

    • Added comprehensive coverage for failure counts, timestamps, regressions, aggregation, cumulative summaries, and suite-specific reporting.

…ilures

Replace the day-by-day backwards loop over prow_job_run_tests (up to 90
queries) with a single query on test_cumulative_summaries using
prefix_max_last_success. Add integration tests for the recent test
failures API.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 5, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 5, 2026
@openshift-ci

openshift-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci-robot

openshift-ci-robot commented Aug 5, 2026

Copy link
Copy Markdown

@mstaeble: This pull request references TRT-2821 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

  • Replace the day-by-day backwards loop in findLastPass (up to 90 queries over prow_job_run_tests) with a single query on test_cumulative_summaries using prefix_max_last_success
  • No API signature or frontend changes; only the internal findLastPass implementation is rewritten
  • Add integration tests for the recent test failures API

Test plan

  • Integration tests pass (6 tests covering failure counts, previous-period exclusion, new regressions, last-pass from cumulative summary, multi-job aggregation, suite separation)
  • Benchmarked against prod-cloned staging DB (release 5.0, 14-day lookback): ~440ms warm cache
  • Verify last_pass populates correctly in staging after deployment

🤖 Generated with Claude Code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR changes recent test failure reporting to read last-pass timestamps from test_cumulative_summaries instead of scanning historical test results. It also adds integration tests for counts, period filtering, regression detection, last-pass population, multi-job aggregation, and per-suite separation.

Changes

Recent test failures reporting

Layer / File(s) Summary
Last-pass summary lookup
pkg/api/recent_test_failures.go
findLastPass now queries test_cumulative_summaries for the report date and returns the maximum non-null prefix_max_last_success per test and suite. The helper comment matches this behavior. testIDsFromKeys now accepts []testSuiteKey, and fetchOutputs passes its key slice directly.
Integration coverage for report behavior
test/integration/recent_test_failures_test.go
Adds integration setup and tests for basic failure rows, exclusion when a test also failed in the previous period, inclusion of new regressions, LastPass values from cumulative summaries, aggregation across multiple jobs, and separate rows for identical test names in different suites.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • openshift/sippy#3870: Also changes recent test reporting logic that reads from test_cumulative_summaries.
🚥 Pre-merge checks | ✅ 18 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Go Error Handling ⚠️ Warning The new cumulative-summary Scan error at pkg/api/recent_test_failures.go:208-209 is returned as return nil, err without fmt.Errorf context or %w. Wrap the error with context, for example fmt.Errorf("querying cumulative summaries for last pass: %w", err).
Test Structure And Quality ⚠️ Warning The added tests use standard testify, not Ginkgo, and many assertions lack diagnostic messages, including require.NoError(t, err), require.NotNil(t, result), and several assert.Equal calls. Add meaningful context to every assertion in recent_test_failures_test.go, especially operation-specific require.NoError messages and field-specific assertion messages.
Test Coverage For New Features ❓ Inconclusive The PR adds six integration tests, including a cumulative-summary regression test, but testIDsFromKeys is a modified pure function with no direct unit test found. Add a focused unit test for testIDsFromKeys, or document why its indirect coverage through API integration tests is sufficient.
✅ Passed checks (18 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Sql Injection Prevention ✅ Passed The changed findLastPass query binds release, date, and testIDs through GORM placeholders; SQL formatting uses only fixed query text, with no user values concatenated.
Excessive Css In React Should Use Styles ✅ Passed The patch changes only two Go files and adds no JSX, TSX, CSS, or React inline style objects, so this check is not applicable.
Single Responsibility And Clear Naming ✅ Passed The modified helpers have specific names and focused responsibilities; testSuiteKey has two fields, and all six added tests use explicit scenario names. No generic or over-broad declarations were a...
Feature Documentation ✅ Passed The only docs/features document covers symptoms and labels, not recent test failures; this PR changes internal query logic without changing the API contract.
Stable And Deterministic Test Names ✅ Passed The PR adds standard Go test names only; all six titles are static, descriptive behavior statements with no timestamps, generated IDs, node/namespace names, IPs, or dynamic interpolation.
Microshift Test Compatibility ✅ Passed The added tests are standard Go integration tests using testing and testify, not Ginkgo e2e tests; they reference no MicroShift-unavailable APIs or resources.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The only added tests are standard Go integration tests using testing/testify and PostgreSQL fixtures; they are not Ginkgo e2e tests and make no multi-node or HA assumptions.
Topology-Aware Scheduling Compatibility ✅ Passed The patch changes only API query logic and integration tests. It adds no manifests, operators, controllers, replicas, affinity, topology spread, node selectors, tolerations, or PDBs.
Ote Binary Stdout Contract ✅ Passed The PR adds no process-level stdout writes, logging setup, or OTE binary code; the new output-free integration tests run as individual test cases.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The PR adds standard Go integration tests, not Ginkgo e2e tests; the added file has no IPv4 assumptions, URL construction, IP parsing, or external service calls.
No-Weak-Crypto ✅ Passed The PR changes only database query logic and integration tests; changed-file imports and added lines contain no weak crypto, custom crypto, or secret/token comparisons.
Container-Privileges ✅ Passed The PR changes only Go source and integration tests. No privilege settings were added; existing E2E pods disable privilege escalation and run as non-root.
No-Sensitive-Data-In-Logs ✅ Passed The pull request adds no logging calls or print statements. The changed files only query and return test data; no passwords, tokens, PII, or customer data are written to logs.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change to use cumulative summaries for findLastPass in recent test failures.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 5, 2026
@mstaeble

mstaeble commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@openshift-ci openshift-ci Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label Aug 5, 2026
@mstaeble
mstaeble marked this pull request as ready for review August 5, 2026 13:25
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 5, 2026
@openshift-ci
openshift-ci Bot requested review from sosiouxme and stbenjam August 5, 2026 13:31
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling required tests:
/test e2e

@neisw

neisw commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2026
@openshift-ci

openshift-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mstaeble, neisw

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci

openshift-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@mstaeble: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot
openshift-merge-bot Bot merged commit 5da7ac2 into openshift:main Aug 5, 2026
10 checks passed
@mstaeble
mstaeble deleted the TRT-2821-recent-test-failures-summary-tables branch August 5, 2026 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants