Skip to content

fix(ci): exclude cancelled jobs from pr-test-summary failed count#1558

Open
ramakrishnap-nv wants to merge 2 commits into
mainfrom
fix/pr-test-summary-cancelled-jobs
Open

fix(ci): exclude cancelled jobs from pr-test-summary failed count#1558
ramakrishnap-nv wants to merge 2 commits into
mainfrom
fix/pr-test-summary-cancelled-jobs

Conversation

@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator

Summary

  • failed was computed as "anything not passed or skipped", so cancelled jobs (e.g. downstream jobs queued when one job fails) were counted as failures.
  • Fix: only count jobs with conclusion in ("failure", "timed_out").

Closes #1223

Cancelled jobs (conclusion="cancelled") were counted as failures because
the failed list was built as anything not passed or skipped. When one
job fails, GitHub cancels downstream queued jobs, inflating the headline
count (e.g. "17 failed" when only 1 test actually failed).

Fix: match only conclusion=="failure" or "timed_out".

Closes #1223

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ramakrishnap-nv ramakrishnap-nv requested a review from a team as a code owner July 10, 2026 20:14
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR test summary utility now distinguishes failed, skipped, passed, and cancelled test jobs, analyzes logs only for failures, and reports cancelled or incomplete jobs in the sticky PR comment.

Changes

PR test status summary

Layer / File(s) Summary
Render cancelled-job status
ci/utils/pr_test_summary.py
_build_body accepts cancelled jobs and updates summary messages to include cancelled or not-completed counts.
Classify test job conclusions
ci/utils/pr_test_summary.py
Failed jobs are limited to failure and timed_out conclusions; remaining unclassified test jobs are treated as cancelled, and log analysis runs only for failures.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • NVIDIA/cuopt#1412: Updates the same utility’s job-conclusion classification and sticky CI summary output.

Suggested labels: non-breaking, improvement

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning Issue #1223 requires centralizing PR test classification and artifact-based aggregation, but this PR only changes cancelled-job counting in the summary. Implement the artifact download/classification refactor described in #1223, or narrow the linked issue if this PR is only fixing cancelled-job accounting.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: excluding cancelled jobs from the PR test summary failure count.
Description check ✅ Passed The description is directly related to the code change and accurately summarizes the cancelled-job fix.
Out of Scope Changes check ✅ Passed The only change stays within pr-test-summary and is directly relevant to the described cancelled-job counting fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pr-test-summary-cancelled-jobs

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ci/utils/pr_test_summary.py`:
- Line 180: Update the job classification in _build_body() to track cancelled
and other non-terminal test jobs separately from failed, passed, and skipped;
when such jobs remain, render an excluded or not-completed status instead of the
“All ... test job(s) passed” message, while preserving existing failure
handling.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 83199161-67cd-4b70-aa0e-e5f502e9c7df

📥 Commits

Reviewing files that changed from the base of the PR and between 940ec59 and 1e4b386.

📒 Files selected for processing (1)
  • ci/utils/pr_test_summary.py

Comment thread ci/utils/pr_test_summary.py
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ci/utils/pr_test_summary.py`:
- Around line 136-140: Update the no-failure cancelled summary in the
summary-generation function to include the skipped-job count alongside passed
and cancelled counts, preserving the existing separate classification and
formatting for all non-passed categories.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 91dc501f-6959-4ab2-a675-cfb9c779ddcd

📥 Commits

Reviewing files that changed from the base of the PR and between 1e4b386 and 43e4170.

📒 Files selected for processing (1)
  • ci/utils/pr_test_summary.py

Comment on lines 136 to +140
if not failed:
lines.append(f"✅ All {len(passed)} test job(s) passed.")
if cancelled:
lines.append(
f"✅ {len(passed)} passed · {len(cancelled)} cancelled / not completed"
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve skipped jobs in the cancelled summary.

When a run has both skipped and cancelled jobs but no failures, this branch omits the skipped count even though those jobs are classified separately. The sticky comment should report all non-passed categories.

Proposed fix
         if cancelled:
-            lines.append(
-                f"✅ {len(passed)} passed · {len(cancelled)} cancelled / not completed"
-            )
+            summary = f"✅ {len(passed)} passed"
+            if skipped:
+                summary += f" · {len(skipped)} skipped"
+            summary += f" · {len(cancelled)} cancelled / not completed"
+            lines.append(summary)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if not failed:
lines.append(f"✅ All {len(passed)} test job(s) passed.")
if cancelled:
lines.append(
f"✅ {len(passed)} passed · {len(cancelled)} cancelled / not completed"
)
if not failed:
if cancelled:
summary = f"✅ {len(passed)} passed"
if skipped:
summary += f" · {len(skipped)} skipped"
summary += f" · {len(cancelled)} cancelled / not completed"
lines.append(summary)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/utils/pr_test_summary.py` around lines 136 - 140, Update the no-failure
cancelled summary in the summary-generation function to include the skipped-job
count alongside passed and cancelled counts, preserving the existing separate
classification and formatting for all non-passed categories.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

CI Test Summary

✅ All 9 test job(s) passed.

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.

ci: centralize PR test classification in pr-test-summary (follow-up to #1194)

2 participants