Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions ci/run_ctests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ extract_failed_tests() {
python3 "${JUNIT_HELPERS}" failed "${xml_file}"
}

# True when gtest wrote a complete all-green JUnit XML. gtest emits XML in
# OnTestIterationEnd (before main returns); a later signal death with this
# XML means the crash was during static/atexit teardown, not a test failure.
xml_reports_all_passed() {
local xml_file="$1"
[ -f "${xml_file}" ] && python3 "${JUNIT_HELPERS}" all-passed "${xml_file}"
}

OVERALL_RC=0
FAILED_BINARIES=()

Expand Down Expand Up @@ -83,6 +91,27 @@ write_binary_crash_marker() {
"Process terminated by ${sig} mid-run. gtest did not emit a JUnit XML because RUN_ALL_TESTS() did not complete; inspect the run log for [FAILED] / stack-trace lines that preceded the crash."
}

# If the binary died on a signal but JUnit XML shows every testcase passed,
# treat as a known post-RUN_ALL_TESTS teardown flake and return 0.
# Caller must have removed any stale XML before launching the binary.
# Args: <test_name> <xml_file> <rc>
# Returns 0 if handled as pass, 1 if not applicable.
accept_post_pass_teardown_crash() {
local test_name="$1"
local xml_file="$2"
local rc="$3"
if ! was_signal_death "${rc}"; then
return 1
fi
if ! xml_reports_all_passed "${xml_file}"; then
return 1
fi
local sig
sig=$(signal_name "${rc}")
echo "WARNING: ${test_name} died from ${sig} (exit code ${rc}) after RUN_ALL_TESTS, but JUnit XML reports all tests passed — treating as pass (post-teardown crash)"
return 0
}

run_gtest_with_retry() {
local gt="$1"
shift
Expand All @@ -92,6 +121,10 @@ run_gtest_with_retry() {

echo "Running gtest ${test_name}"

# Drop any stale XML so a mid-run crash cannot be mistaken for an
# all-green post-teardown abort (gtest only rewrites XML at the end).
rm -f "${xml_file}"

# First run — full binary
local rc=0
"${gt}" --gtest_output="xml:${xml_file}" "$@" || rc=$?
Expand All @@ -100,6 +133,10 @@ run_gtest_with_retry() {
return 0
fi

if accept_post_pass_teardown_crash "${test_name}" "${xml_file}" "${rc}"; then
return 0
fi

# For non-nightly builds: fail immediately, no retries
# PRs should surface failures directly so authors can see what broke
if [ "${IS_NIGHTLY}" != "nightly" ]; then
Expand Down Expand Up @@ -146,6 +183,12 @@ run_gtest_with_retry() {
fi

if [ -z "${tests_to_retry}" ]; then
# Empty retry set: either listing failed, or every listed test
# already passed (post-teardown crash with a complete XML).
if xml_reports_all_passed "${xml_file}"; then
echo "WARNING: ${test_name} crashed after exit but all listed tests already passed — treating as pass"
return 0
fi
echo "FAILED: Could not list tests in ${test_name}, cannot retry"
write_crash_xml "${xml_file}" "${test_name}" "PROCESS_CRASH" \
"${test_name} crashed with $(signal_name ${rc}) (exit code ${rc})" \
Expand Down Expand Up @@ -181,6 +224,7 @@ run_gtest_with_retry() {
echo " Retry ${attempt}/${GTEST_MAX_RETRIES}: ${tc}"

local retry_rc=0
rm -f "${retry_xml}"
"${gt}" --gtest_filter="${tc}" --gtest_output="xml:${retry_xml}" "$@" || retry_rc=$?

if [ "${retry_rc}" -eq 0 ]; then
Expand All @@ -189,6 +233,12 @@ run_gtest_with_retry() {
break
fi

if accept_post_pass_teardown_crash "${test_name}" "${retry_xml}" "${retry_rc}"; then
echo " FLAKY: ${tc} passed on retry ${attempt} (post-teardown crash ignored)"
tc_passed=true
break
fi

if was_signal_death "${retry_rc}"; then
echo " CRASH: ${tc} died from $(signal_name ${retry_rc}) on retry ${attempt}"
write_crash_xml "${retry_xml}" "${test_name}" "${tc}" \
Expand Down
30 changes: 30 additions & 0 deletions ci/utils/junit_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
Commands:
failed <xml_file> [--sep SEP] Print failed/errored test names
passed <xml_file> [--sep SEP] Print passed test names (excludes skipped)
all-passed <xml_file> Exit 0 if XML has >=1 testcase and 0 failures/errors
gtest-list Parse gtest --gtest_list_tests from stdin
"""

Expand Down Expand Up @@ -50,6 +51,26 @@ def extract_tests(xml_path, status="failed", sep=".", include_skipped=False):
print(f"{cls}{sep}{name}")


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.
"""
Comment on lines +54 to +60

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

try:
tree = ElementTree.parse(xml_path)
except (ElementTree.ParseError, FileNotFoundError, OSError):
return False

saw_testcase = False
for tc in tree.iter("testcase"):
saw_testcase = True
if tc.find("failure") is not None or tc.find("error") is not None:
return False
return saw_testcase


def parse_gtest_list():
"""Parse gtest --gtest_list_tests output from stdin into Suite.TestName."""
suite = ""
Expand Down Expand Up @@ -84,6 +105,15 @@ def main():
sep = sys.argv[i + 1]
extract_tests(xml_path, status=cmd, sep=sep)

elif cmd == "all-passed":
if len(sys.argv) < 3:
print(
f"Usage: {sys.argv[0]} all-passed <xml_file>",
file=sys.stderr,
)
sys.exit(1)
sys.exit(0 if xml_all_passed(sys.argv[2]) else 1)

elif cmd == "gtest-list":
parse_gtest_list()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2383,5 +2383,11 @@ TEST_F(ChunkValidationTests, AcceptsValidChunk)
int main(int argc, char** argv)
{
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
const int rc = RUN_ALL_TESTS();
// Skip C++ static / atexit destructors. gRPC / Abseil / protobuf can
// intermittently double-free during process-exit teardown after a fully
// green run (CI sees SIGABRT / "double free or corruption (fasttop)" after
// "[ PASSED ]"). Shared ServerProcess instances are already stopped in
// each suite's TearDownTestSuite, so skipping global dtors is safe here.
std::_Exit(rc);
}
Loading