Skip to content

Failure Detector Expansion 2/5: Recover test timeouts from CI console logs - #82

Open
BChan-0 wants to merge 2 commits into
valkey-io:mainfrom
BChan-0:failure-detector-expansion-timeout-final
Open

Failure Detector Expansion 2/5: Recover test timeouts from CI console logs#82
BChan-0 wants to merge 2 commits into
valkey-io:mainfrom
BChan-0:failure-detector-expansion-timeout-final

Conversation

@BChan-0

@BChan-0 BChan-0 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Stacked on #79 (1/5). This branch contains 1/5's commit as its base, so the file list and diff are inclusive. It's best to review and merge in numerical order; the diff size will lower as they merge (I think)

Second of five PRs expanding the Failure Detector past assertion failures. Builds on 1/5, which carries the schema and dedup plumbing. The producer side is valkey PR 4292.

Issue

A timed-out job can leave no timeout entry in the artifact at all. The runner'swatchdog kills the process, and if it fires before write_test_failures runs, nothing is written: the job goes red and the sweep reports no failure for it.

Two smaller gaps around the same path:

  • A run discovered by event filter could be the wrong run. daily.yml also runs on pull_request, and a PR from a branch in the same repository runs without approval and reaches a real conclusion, so its failures would be filed against unstable.
  • A red run with no artifact was reported as "passed cleanly". The artifact can be missing because it expired, the upload failed, or the run died before the consolidate step; none of those are a clean run.

PR Summary

Failed jobs whose artifact captured no timeout entry have their console log scanned for the runner's [TIMEOUT] lines. The marker is read only from the start of the runner's own line, allowing the Actions timestamp and the summary's *** prefix, so a test that prints or asserts on text containing a [TIMEOUT] line is not filed as a timeout it never hit.

A captured timeout carries only "Test timed out", because that is all the watchdog knows. The runner's clients-state report, which names the test each client was running when it fired, is read back from the log and attached. Collection stops at the first server-log header, since what follows is mostly startup banner.

Run discovery now filters on the event locally alongside the conclusion check, so both rejection reasons are logged against the run they skipped, and bounds the scan instead. A run named explicitly with --run-id bypasses the filter.

A missing artifact on a run that concluded failure, timed_out, or startup_failure is reported as a reporting gap and exits non-zero. If the conclusion cannot be read at all, the sweep fails closed rather than calling it clean.

Job metadata is fetched once: get_job_info returns URLs, failed job names, and per-suite step anchors off one response, so a failure links to the step that ran its suite. Matrix jobs are recorded under both spellings, the API's base (value) and the artifact's base-value, because the two sides of recovery join on opposite ones.

Finally, a red job that no reported failure accounts for is named in the job summary and exits non-zero, so a job that failed for a reason nothing can see does not pass as green.

Testing

Added:

  • tests/test_testfailuredetector_timeout_parser.py: TestJobsNeedingLogScan, TestParseTimeoutsFromLog, TestFindJobLog, TestMergeTimeoutRecoveries, TestRunLogs, TestRecoverAndEnrichTimeouts, TestTimeoutMarkerIsAnchored, TestMatrixJobNameHandoff
  • tests/test_testfailuredetector_download.py: TestGetJobInfoStepUrls, TestGetJobInfoFailedJobs
  • tests/test_testfailuredetector_main.py: TestExplicitRunIdMissingArtifact, TestUnknownConclusionFailsClosed, TestUnexplainedFailedJobs
  • tests/test_testfailuredetector_issue_manager.py: TestVolatileTimeoutFingerprint

1517 total tests passed.

@BChan-0 BChan-0 changed the title Failure Detector Expansion: 2/5 — Recover test timeouts from CI console logs Failure Detector Expansion 2/5: Recover test timeouts from CI console logs Aug 7, 2026
@BChan-0
BChan-0 force-pushed the failure-detector-expansion-timeout-final branch from 22c53f4 to fd6aa08 Compare August 7, 2026 19:36
@BChan-0
BChan-0 marked this pull request as ready for review August 7, 2026 20:17
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@BChan-0, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 55f2c464-8655-4adc-b5b3-df6f84166924

📥 Commits

Reviewing files that changed from the base of the PR and between cab64ac and 7f1786e.

📒 Files selected for processing (16)
  • scripts/common/issue_dedup.py
  • scripts/common/workflow_artifacts.py
  • scripts/test_failure_detector/download.py
  • scripts/test_failure_detector/issue_renderer.py
  • scripts/test_failure_detector/main.py
  • scripts/test_failure_detector/manage_issues.py
  • scripts/test_failure_detector/parse_failures.py
  • scripts/test_failure_detector/timeout_parser.py
  • scripts/test_failure_detector/timeout_recovery.py
  • tests/test_issue_dedup.py
  • tests/test_testfailuredetector_download.py
  • tests/test_testfailuredetector_failure_parser.py
  • tests/test_testfailuredetector_issue_manager.py
  • tests/test_testfailuredetector_main.py
  • tests/test_testfailuredetector_timeout_parser.py
  • tests/test_workflow_artifacts.py

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.

BChan-0 added 2 commits August 7, 2026 13:36
Signed-off-by: Bonnie Chan <bonniecv@amazon.com>
Signed-off-by: Bonnie Chan <bonniecv@amazon.com>
@BChan-0
BChan-0 force-pushed the failure-detector-expansion-timeout-final branch from fd6aa08 to 7f1786e Compare August 7, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant