Skip to content

workaround for signal during grpc test framework teardown - #1691

Open
tmckayus wants to merge 1 commit into
NVIDIA:mainfrom
tmckayus:fix/grpc-teardown-ci-unblock
Open

workaround for signal during grpc test framework teardown#1691
tmckayus wants to merge 1 commit into
NVIDIA:mainfrom
tmckayus:fix/grpc-teardown-ci-unblock

Conversation

@tmckayus

@tmckayus tmckayus commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Tests themselves pass, but something in the teardown of the test framework does a double free. Unblock CI while we search for the root cause.

Best guess is something in the gRPC/abseil stack. Will investigate separately.

Tests themselves pass, but something in the teardown of the
test framework does a double free. Unblock CI while we
search for the root cause.
@tmckayus
tmckayus requested review from a team as code owners August 7, 2026 15:12
@tmckayus tmckayus added non-breaking Introduces a non-breaking change improvement Improves an existing functionality labels Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The integration test now bypasses post-test teardown with std::_Exit. JUnit helpers validate complete all-passed reports. CI clears stale XML files and accepts post-teardown signal deaths when those reports contain only passing testcases.

Changes

Post-teardown crash handling

Layer / File(s) Summary
Test process exit behavior
cpp/tests/linear_programming/grpc/grpc_integration_test.cpp
The test captures the RUN_ALL_TESTS() result and exits with std::_Exit(rc).
JUnit all-passed validation
ci/utils/junit_helpers.py
The helper validates non-empty JUnit reports without failures or errors and adds the all-passed command.
CI crash acceptance and retry handling
ci/run_ctests.sh
The runner removes stale XML files and accepts signal deaths when initial or retry reports contain only passing testcases.

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

Suggested reviewers: gforsyth, aliceb-nv, akifcorduk

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the teardown signal workaround implemented for the gRPC test framework.
Description check ✅ Passed The description explains the teardown double-free failure and the temporary CI workaround.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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/junit_helpers.py`:
- Around line 54-60: Update xml_all_passed with the requested type annotations,
using xml_path: str and -> bool. Expand its docstring to document the xml_path
parameter, the conditions for returning True, and that file or XML parsing
errors return False.
🪄 Autofix

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: 10b14469-f02d-4d95-95d8-6683c1ee57ee

📥 Commits

Reviewing files that changed from the base of the PR and between ae0a38a and 1bf2db5.

📒 Files selected for processing (3)
  • ci/run_ctests.sh
  • ci/utils/junit_helpers.py
  • cpp/tests/linear_programming/grpc/grpc_integration_test.cpp

Comment thread ci/utils/junit_helpers.py
Comment on lines +54 to +60
def xml_all_passed(xml_path):
"""Return True if XML parses, has >=1 testcase, and no failure/error nodes.

Used to detect post-RUN_ALL_TESTS teardown crashes: gtest writes XML in
OnTestIterationEnd before main returns, so a complete all-green XML plus
a later SIGABRT means the crash was during static/atexit destruction.
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- file outline ---'
ast-grep outline ci/utils/junit_helpers.py
printf '%s\n' '--- relevant source ---'
cat -n ci/utils/junit_helpers.py | sed -n '1,130p'
printf '%s\n' '--- usages and declarations ---'
rg -n -C 3 'xml_all_passed|all-passed' ci .github 2>/dev/null || true
printf '%s\n' '--- repository lint/config references ---'
rg -n -C 2 'ruff|mypy|pyright|pylint|pydocstyle|type.?check' pyproject.toml setup.cfg tox.ini .pre-commit-config.yaml Makefile ci 2>/dev/null || true

Repository: NVIDIA/cuopt

Length of output: 29103


Add type annotations and API documentation for xml_all_passed.

Use xml_path: str and -> bool. Document the parameter, return conditions, and that parse or file errors return False.

🤖 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/junit_helpers.py` around lines 54 - 60, Update xml_all_passed with
the requested type annotations, using xml_path: str and -> bool. Expand its
docstring to document the xml_path parameter, the conditions for returning True,
and that file or XML parsing errors return False.

Source: Coding guidelines

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

CI Test Summary

✅ All 31 test job(s) passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants