diff --git a/changelog.d/7086-windows-parity-review.md b/changelog.d/7086-windows-parity-review.md new file mode 100644 index 0000000000..b87ea99a10 --- /dev/null +++ b/changelog.d/7086-windows-parity-review.md @@ -0,0 +1 @@ +fix(parity): enforce Windows timeouts and include crashes in platform-aware failure gates diff --git a/run_parity_tests.sh b/run_parity_tests.sh index a67514e261..a1e32864cc 100755 --- a/run_parity_tests.sh +++ b/run_parity_tests.sh @@ -51,6 +51,11 @@ if [[ -z "$PYTHON_CMD" ]]; then echo "Python 3 is required by the parity output normalizer" >&2 exit 1 fi +PERRY_RUN_TIMEOUT="${PERRY_RUN_TIMEOUT:-10}" +if [[ ! "$PERRY_RUN_TIMEOUT" =~ ^[1-9][0-9]*$ ]]; then + echo "Invalid PERRY_RUN_TIMEOUT '$PERRY_RUN_TIMEOUT' (want a positive integer)" >&2 + exit 1 +fi # Per-run scratch dir for compiled test binaries (2026-07-02 audit): the old # fixed /tmp/perry_parity_ paths meant two concurrent suite runs # (two agents / two worktrees on one machine) executed EACH OTHER'S compiler @@ -129,13 +134,14 @@ YELLOW='\033[1;33m' CYAN='\033[0;36m' NC='\033[0m' # No Color -# Find timeout command (GNU coreutils on Linux, gtimeout on macOS via Homebrew) +# Find a native timeout command where available. Git Bash may resolve +# Windows' unrelated timeout.exe (or no timeout at all), so Windows always +# uses the Python process-tree fallback in run_with_timeout. if command -v timeout &> /dev/null; then TIMEOUT_CMD="timeout" elif command -v gtimeout &> /dev/null; then TIMEOUT_CMD="gtimeout" else - # No timeout available - run without timeout TIMEOUT_CMD="" fi @@ -162,11 +168,55 @@ perry_abnormal_exit() { run_with_timeout() { local seconds=$1 shift - if [[ -n "$TIMEOUT_CMD" ]]; then + if [[ "$HOST_PLATFORM" != "windows" && -n "$TIMEOUT_CMD" ]]; then $TIMEOUT_CMD "$seconds" "$@" - else - "$@" + return fi + + # Python is already a required harness dependency. Start the test in its + # own process group and kill the entire tree on expiry; returning 124 + # matches GNU timeout so the existing crash classifier stays unchanged. + "$PYTHON_CMD" -c ' +import os +import signal +import subprocess +import sys + +seconds = int(sys.argv[1]) +command = sys.argv[2:] +options = {} +if os.name == "nt": + options["creationflags"] = subprocess.CREATE_NEW_PROCESS_GROUP +else: + options["start_new_session"] = True + +process = subprocess.Popen(command, **options) +try: + returncode = process.wait(timeout=seconds) +except subprocess.TimeoutExpired: + if os.name == "nt": + subprocess.run( + ["taskkill.exe", "/PID", str(process.pid), "/T", "/F"], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + ) + else: + try: + os.killpg(process.pid, signal.SIGKILL) + except ProcessLookupError: + pass + try: + process.wait(timeout=5) + except subprocess.TimeoutExpired: + process.kill() + process.wait() + raise SystemExit(124) + +if returncode < 0: + raise SystemExit(128 - returncode) +raise SystemExit(returncode) +' "$seconds" "$@" } wait_for_tcp_port() { @@ -675,6 +725,8 @@ cleanup_parity_run() { rm -rf "$PARITY_TMP" } +# Supersede the scratch-only trap installed immediately after mktemp now that +# the optional echo server also has lifecycle state to clean. trap cleanup_parity_run EXIT # The granular node-suite starts with deterministic module cases (path, url, @@ -810,14 +862,14 @@ for test_file in "${TEST_FILES[@]}"; do # `$(...)`, so capturing the exit code requires the file detour # rather than a `cmd | cap_output` pipeline. node_tmp="$PARITY_TMP/${safe_test_id}.node-output" - run_with_timeout 10 env FORCE_COLOR=0 NO_COLOR=1 NODE_DISABLE_COLORS=1 \ + run_with_timeout "$PERRY_RUN_TIMEOUT" env FORCE_COLOR=0 NO_COLOR=1 NODE_DISABLE_COLORS=1 \ node --experimental-strip-types "${node_argv[@]}" "$test_file" "${test_argv[@]}" > "$node_tmp" 2>&1 node_exit=$? if [[ $node_exit -ne 0 ]] && can_retry_node_globals_as_commonjs "$test_id" "$test_file"; then parity_test_file="$PARITY_TMP/${safe_test_id}.cts" cp "$test_file" "$parity_test_file" perry_compile_command=(compile) - run_with_timeout 10 env FORCE_COLOR=0 NO_COLOR=1 NODE_DISABLE_COLORS=1 \ + run_with_timeout "$PERRY_RUN_TIMEOUT" env FORCE_COLOR=0 NO_COLOR=1 NODE_DISABLE_COLORS=1 \ node --experimental-strip-types "${node_argv[@]}" "$parity_test_file" "${test_argv[@]}" > "$node_tmp" 2>&1 node_exit=$? fi @@ -910,12 +962,12 @@ for test_file in "${TEST_FILES[@]}"; do # classified as crash rather than a wedged machine. if [[ "$HOST_PLATFORM" == "windows" ]]; then # Git Bash has no enforceable RLIMIT_NPROC (`ulimit -u`) for native - # Windows processes. The outer timeout still bounds the direct test; - # Windows CI should additionally use the job-level Actions timeout. - run_with_timeout 10 env PERRY_STUB_DIAG=off "${parity_env[@]}" \ + # Windows processes. The Python-backed timeout starts a fresh process + # group and kills its tree, even when GNU timeout is unavailable. + run_with_timeout "$PERRY_RUN_TIMEOUT" env PERRY_STUB_DIAG=off "${parity_env[@]}" \ "$perry_binary" "${test_argv[@]}" > "$perry_tmp" 2>&1 else - run_with_timeout 10 "$BASH" -c ' + run_with_timeout "$PERRY_RUN_TIMEOUT" "$BASH" -c ' if [ "$(uname -s)" = "Linux" ]; then proc_list=$(ps -u "$(id -u)" -L -o lwp= 2>/dev/null) else diff --git a/scripts/parity_known_failures.py b/scripts/parity_known_failures.py index 3ef7c37e58..60401522dd 100644 --- a/scripts/parity_known_failures.py +++ b/scripts/parity_known_failures.py @@ -12,22 +12,23 @@ ROOT = Path(__file__).resolve().parent.parent DEFAULT_REPORT = ROOT / "test-parity" / "reports" / "latest.json" DEFAULT_KNOWN = ROOT / "test-parity" / "known_failures.json" -PLATFORMS = frozenset({"linux", "macos", "windows", "other"}) +PLATFORM_ALIASES = { + "cygwin": "windows", + "darwin": "macos", + "linux": "linux", + "macos": "macos", + "mingw": "windows", + "msys": "windows", + "other": "other", + "win32": "windows", + "windows": "windows", +} +PLATFORMS = frozenset(PLATFORM_ALIASES.values()) def normalize_platform(value: str) -> str: folded = value.strip().lower() - aliases = { - "darwin": "macos", - "macos": "macos", - "linux": "linux", - "win32": "windows", - "windows": "windows", - "cygwin": "windows", - "msys": "windows", - "mingw": "windows", - } - return aliases.get(folded, "other") + return PLATFORM_ALIASES.get(folded, "other") def load_json(path: Path) -> dict: @@ -43,7 +44,7 @@ def report_failures(report: dict) -> set[str]: if not isinstance(failures, dict): raise ValueError("parity report must contain a failures object") result: set[str] = set() - for category in ("parity", "compile"): + for category in ("parity", "compile", "crash"): values = failures.get(category) or [] if not isinstance(values, list) or not all(isinstance(item, str) for item in values): raise ValueError(f"failures.{category} must be a string array") @@ -77,7 +78,9 @@ def known_for_platform(known: dict, platform: str) -> tuple[set[str], list[str]] problems.append(f"{test_id}: platforms must be a non-empty string array") continue normalized = [normalize_platform(item) for item in platforms] - unknown = sorted({item for item in platforms if item.strip().lower() not in PLATFORMS}) + unknown = sorted( + {item for item in platforms if item.strip().lower() not in PLATFORM_ALIASES} + ) if unknown: problems.append(f"{test_id}: unknown platforms: {', '.join(unknown)}") continue @@ -102,7 +105,11 @@ def check(report: dict, known: dict, platform_override: str | None = None) -> tu def self_test() -> int: report = { "platform": "windows", - "failures": {"parity": ["all_hosts", "windows_only", "linux_only"], "compile": [""]}, + "failures": { + "parity": ["all_hosts", "windows_only", "linux_only"], + "compile": [""], + "crash": ["windows_crash"], + }, } known = { "_schema": {}, @@ -110,7 +117,12 @@ def self_test() -> int: "windows_only": { "category": "bug-open", "reason": "win", - "platforms": ["windows"], + "platforms": ["win32"], + }, + "windows_crash": { + "category": "bug-open", + "reason": "crash", + "platforms": ["msys"], }, "linux_only": { "category": "bug-open", @@ -122,6 +134,7 @@ def self_test() -> int: assert platform == "windows" assert new == ["linux_only"] assert problems == [] + assert normalize_platform("darwin") == "macos" malformed = { "bad": {"category": "", "reason": "", "platforms": ["plan9"]}, diff --git a/scripts/parity_matrix_trend.py b/scripts/parity_matrix_trend.py index 770d9f56ab..75b419463b 100755 --- a/scripts/parity_matrix_trend.py +++ b/scripts/parity_matrix_trend.py @@ -18,6 +18,8 @@ from datetime import datetime, timezone from pathlib import Path +from parity_known_failures import normalize_platform + REPO_ROOT = Path(__file__).resolve().parent.parent DEFAULT_REPORT = REPO_ROOT / "test-parity" / "reports" / "latest.json" @@ -27,7 +29,7 @@ DEFAULT_JSON = REPO_ROOT / "test-parity" / "reports" / "parity_matrix_latest.json" DEFAULT_MARKDOWN = REPO_ROOT / "test-parity" / "reports" / "parity_matrix_latest.md" -FAIL_STATUSES = {"parity_fail", "compile_fail", "node_fail"} +FAIL_STATUSES = {"parity_fail", "compile_fail", "crash", "node_fail"} @dataclass @@ -82,17 +84,6 @@ def diff_line_count(node_lines: list[str] | None, perry_lines: list[str] | None) return count -def normalize_platform(value: str) -> str: - folded = value.strip().lower() - if folded in {"win32", "windows", "cygwin", "msys", "mingw"}: - return "windows" - if folded in {"darwin", "macos"}: - return "macos" - if folded == "linux": - return "linux" - return "other" - - def known_failures(path: Path, platform: str | None = None) -> set[str]: if not path.exists(): return set() @@ -141,6 +132,9 @@ def report_results(report: dict) -> list[dict[str, str]]: for test_id in failures.get("compile", []) or []: if test_id: out.append({"id": test_id, "status": "compile_fail"}) + for test_id in failures.get("crash", []) or []: + if test_id: + out.append({"id": test_id, "status": "crash"}) return out diff --git a/tests/test_parity_build_reuse.sh b/tests/test_parity_build_reuse.sh index 7d079b084f..dd36fd6e12 100755 --- a/tests/test_parity_build_reuse.sh +++ b/tests/test_parity_build_reuse.sh @@ -23,6 +23,9 @@ while [ "$#" -gt 0 ]; do if [ "$1" = "-o" ]; then cat > "$2" <<'BIN' #!/bin/sh +if [ "${PERRY_TEST_HANG:-0}" = "1" ]; then + sleep 30 +fi echo reuse-ok BIN chmod +x "$2" @@ -107,6 +110,29 @@ if find "$WORK/windows-temp" -maxdepth 1 -name 'perry-parity.*' | grep -q .; the exit 1 fi +# Git Bash does not ship GNU timeout. Exercise the Python-backed Windows +# process-tree timeout with a one-second test hook and require crash +# classification rather than waiting for the mock binary's 30-second sleep. +SECONDS=0 +set +e +env -u TMPDIR -u TMP \ + PERRY_HOST_PLATFORM=windows \ + PERRY_RUN_TIMEOUT=1 \ + PERRY_TEST_HANG=1 \ + TEMP="$WORK/windows-temp" \ + PERRY_SKIP_BUILD=1 \ + PERRY_BIN="$WORK/perry.exe" \ + PERRY_RUNTIME_DIR="$WORK" \ + "$WORK/run_parity_tests.sh" --suite node-suite --module reuse >"$WORK/timeout-output" 2>&1 +timeout_status=$? +set -e +[[ "$timeout_status" -ne 0 ]] +grep -F "TIMEOUT (killed after 1s)" "$WORK/timeout-output" >/dev/null +if (( SECONDS > 8 )); then + echo "Windows timeout fallback took ${SECONDS}s" >&2 + exit 1 +fi + # The gap wrapper must select an independent Windows snapshot instead of # comparing the Windows result with the committed Linux baseline. env -u TMPDIR -u TMP \ @@ -115,7 +141,12 @@ env -u TMPDIR -u TMP \ PERRY_SKIP_BUILD=1 \ PERRY_BIN="$WORK/perry.exe" \ PERRY_RUNTIME_DIR="$WORK" \ - "$WORK/scripts/run_gap_tests.sh" --filter test_gap_reuse >"$WORK/gap-output" 2>&1 + "$WORK/scripts/run_gap_tests.sh" --filter test_gap_reuse >"$WORK/gap-output" 2>&1 || { + gap_status=$? + echo "gap wrapper failed:" >&2 + cat "$WORK/gap-output" >&2 + exit "$gap_status" + } grep -F "test-parity/gap_snapshot.windows.json" "$WORK/gap-output" >/dev/null echo "PASS" diff --git a/tests/test_parity_matrix_trend.py b/tests/test_parity_matrix_trend.py index 9b7b77853c..cce0f9c938 100644 --- a/tests/test_parity_matrix_trend.py +++ b/tests/test_parity_matrix_trend.py @@ -99,6 +99,23 @@ def test_new_untriaged_failure_fails(self): self.assertIn("test_parity_path", result.stdout) self.assertIn("not listed in known_failures.json", result.stdout) + def test_new_untriaged_crash_fails(self): + with tempfile.TemporaryDirectory() as td: + root = Path(td) + self.write_json(root / "report.json", { + "results": [{"id": "test_parity_zlib", "status": "crash"}], + "failures": {"parity": [], "compile": [], "crash": ["test_parity_zlib"]}, + }) + self.write_json(root / "known_failures.json", {}) + self.write_json(root / "baseline.json", {"modules": {}}) + self.write_output(root, "test_parity_zlib", "node\n", "partial\n") + + result = self.run_checker(root) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("test_parity_zlib", result.stdout) + self.assertIn("crash is not listed in known_failures.json", result.stdout) + def test_diff_lines_regression_fails_even_for_known_failure(self): with tempfile.TemporaryDirectory() as td: root = Path(td)