From 755b77cb5b997a50018f059c3c2fc6940a5644b7 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Tue, 15 Sep 2026 21:17:53 -0400 Subject: [PATCH 1/5] Always wait for child to exit --- .../wpilib/testing/pytest_isolated_tests_plugin.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py b/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py index f6f100108..794a4f30b 100644 --- a/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py +++ b/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py @@ -442,10 +442,10 @@ def _cleanup_job(self, job: IsolatedTestJob): if job.process.is_alive(): job.process.kill() - try: - job.process.join(timeout=1) - except TimeoutError: - pass + # kill() is asynchronous, particularly on Windows. Wait for the OS to + # finish termination before close(); a timed join can return while the + # process is still running (without raising TimeoutError). + job.process.join() ec = job.process.exitcode if ec is not None: From 2bf4bd797b3be6f858b78cb9443a3fe003b738e9 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Tue, 15 Sep 2026 22:36:58 -0400 Subject: [PATCH 2/5] Add diagnostics --- .../tests/test_pytest_plugins.py | 140 ++++++++++++++++++ .../testing/pytest_isolated_tests_plugin.py | 42 +++++- 2 files changed, 178 insertions(+), 4 deletions(-) diff --git a/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py b/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py index ee4f11720..c9e122dbb 100644 --- a/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py +++ b/subprojects/robotpy-wpilib/tests/test_pytest_plugins.py @@ -1,8 +1,14 @@ +import multiprocessing import pathlib import sys import pytest +from wpilib.testing.pytest_isolated_tests_plugin import ( + IsolatedTestJob, + IsolatedTestsPlugin, +) + from pytest_plugin_test_helpers import ( _configure_isolated_plugin, _configure_robot_testing_plugin, @@ -43,6 +49,55 @@ def test_robot_failure(robot): ) +@pytest.mark.parametrize("worker_exit_code", [None, 0, 1]) +def test_isolated_plugin_cleanup_waits_for_termination(worker_exit_code): + # Model delayed OS termination, keeping Process.join/close real so closing + # a still-running process raises the same ValueError as it does on Windows. + class DelayedTerminationPopen: + pid = 12345 + returncode = None + killed = False + + def poll(self): + return self.returncode + + def kill(self): + self.killed = True + + def wait(self, timeout=None): + assert self.killed, "cleanup must kill the worker before waiting" + if timeout is None: + self.returncode = -15 + # A timed-out join returns without raising TimeoutError. + return self.returncode + + def close(self): + pass + + process = multiprocessing.Process() + process._popen = DelayedTerminationPopen() + process._sentinel = object() + conn, peer = multiprocessing.Pipe() + job = IsolatedTestJob( + item=None, + conn=conn, + process=process, + start_time=0, + exit_code=worker_exit_code, + ) + plugin = IsolatedTestsPlugin(None, pathlib.Path("robot.py"), False, False, 1) + + try: + plugin._cleanup_job(job) + assert conn.closed + assert process._closed + # Cleanup must not replace a reported pytest status with the kill status. + assert job.exit_code == (-15 if worker_exit_code is None else worker_exit_code) + finally: + conn.close() + peer.close() + + def test_isolated_plugin_process_and_output(pytester): _make_robot_module(pytester) _configure_isolated_plugin(pytester) @@ -224,6 +279,91 @@ def test_robot_two(robot): ) +@pytest.mark.parametrize( + "interrupt", + [ + "raise KeyboardInterrupt('worker interruption detail')", + "pytest.exit('worker interruption detail')", + ], +) +def test_isolated_plugin_reports_worker_interruption_details(pytester, interrupt): + _make_robot_module(pytester) + _configure_isolated_plugin(pytester) + pytester.makepyfile(test_isolated=f""" +import pathlib +import pytest + + +def interrupt_worker(): + {interrupt} + + +def test_robot_interrupt(robot): + interrupt_worker() + + +def test_later_robot(robot): + pathlib.Path("later-robot-ran").touch() +""") + + result = pytester.runpytest_subprocess("-v") + + assert result.ret == pytest.ExitCode.INTERRUPTED + result.stdout.fnmatch_lines( + [ + "*test_isolated.py::test_robot_interrupt*", + "*interrupt_worker*", + "*worker interruption detail*", + ] + ) + assert not (pytester.path / "later-robot-ran").exists() + + +@pytest.mark.parametrize( + "error_in_target, args, exit_code, passed", + [ + (True, [], pytest.ExitCode.TESTS_FAILED, 0), + (True, ["--show-capture=no"], pytest.ExitCode.TESTS_FAILED, 0), + (False, ["."], pytest.ExitCode.INTERRUPTED, 0), + ( + False, + [".", "--continue-on-collection-errors"], + pytest.ExitCode.TESTS_FAILED, + 1, + ), + ], +) +def test_isolated_plugin_reports_worker_collection_details( + pytester, error_in_target, args, exit_code, passed +): + _make_robot_module(pytester) + _configure_isolated_plugin(pytester) + collection_error = """ +import multiprocessing + +if multiprocessing.parent_process() is not None: + raise RuntimeError("worker collection detail") +""" + test_source = """ +def test_robot(robot): + assert robot.did_init +""" + if error_in_target: + test_source = collection_error + test_source + else: + pytester.makepyfile(test_collection_error=collection_error) + pytester.makepyfile(test_isolated=test_source) + + result = pytester.runpytest_subprocess("-v", *args) + + assert result.ret == exit_code + result.assert_outcomes(errors=1, passed=passed) + output = result.stdout.str() + assert "RuntimeError: worker collection detail" in output + assert "Running test_isolated.py::test_robot" in output + assert "subprocess exited with exit code" not in output + + def test_isolated_plugin_reports_worker_collection_exit(pytester): _make_robot_module(pytester) _configure_isolated_plugin(pytester) diff --git a/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py b/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py index 794a4f30b..f4d35e745 100644 --- a/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py +++ b/subprojects/robotpy-wpilib/wpilib/testing/pytest_isolated_tests_plugin.py @@ -109,6 +109,18 @@ def pytest_internalerror(self, excrepr: object): print("IERROR>", line, file=sys.stderr) self.sendevent("internal_error", formatted_error=formatted_error) + @pytest.hookimpl + def pytest_collectreport(self, report: pytest.CollectReport): + if report.failed: + data = self.config.hook.pytest_report_to_serializable( + config=self.config, report=report + ) + self.sendevent("collectreport", data=data) + + @pytest.hookimpl + def pytest_keyboard_interrupt(self, excinfo: pytest.ExceptionInfo): + self.sendevent("interruption", formatted_error=str(excinfo.getrepr())) + @pytest.hookimpl def pytest_runtest_logstart( self, @@ -201,6 +213,9 @@ class IsolatedTestJob: # set when the worker indicates it has finished worker_completed: bool = False + collection_failed: bool = False + interruption: str | None = None + def set_exit_code(self, ec: int): if self.exit_code is None: self.exit_code = ec @@ -496,6 +511,22 @@ def worker_testreport(self, job: IsolatedTestJob, data: object): self._config.hook.pytest_runtest_logreport(report=report) self._handlefailures(report) + def worker_collectreport(self, job: IsolatedTestJob, data: object): + report = self._config.hook.pytest_report_from_serializable( + config=self._config, data=data + ) + # Include the assigned test in the error itself, not a captured-output + # section that --show-capture could hide. + report.longrepr = ( + f"Running {job.item.nodeid} in isolated worker:\n\n{report.longrepr}" + ) + job.collection_failed = True + self._config.hook.pytest_collectreport(report=report) + + def worker_interruption(self, job: IsolatedTestJob, formatted_error: str): + # Wait for the finished event to preserve the worker's exit status. + job.interruption = formatted_error + def worker_internal_error(self, job: IsolatedTestJob, formatted_error: str): """Emitted when a node calls the pytest_internalerror hook.""" for line in formatted_error.split("\n"): @@ -511,16 +542,19 @@ def worker_finished(self, job: IsolatedTestJob, exit_code: object | None = None) job.exit_code = int(exit_code) if job.exit_code == pytest.ExitCode.INTERRUPTED and not self._shouldstop: - self._shouldstop = "interrupted in worker" + self._shouldstop = f"interrupted in worker for {job.item.nodeid}" + if job.interruption: + self._shouldstop += f"\n\n{job.interruption}" # Normal test failures have already produced reports, and interruptions - # stop the parent session. Other exit codes need a synthetic failure - # report from _finalize_job. + # stop the parent session. A collection error in the selected module + # can also produce USAGE_ERROR ("found no collectors"); its report has + # already been forwarded. Other exits need a synthetic failure report. job.worker_completed = job.exit_code in ( pytest.ExitCode.OK, pytest.ExitCode.TESTS_FAILED, pytest.ExitCode.INTERRUPTED, - ) + ) or (job.collection_failed and job.exit_code == pytest.ExitCode.USAGE_ERROR) job.finished = True def _handlefailures(self, rep: pytest.TestReport): From 7578524231f06d3e908d5909300567e8d44e0ec6 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Wed, 16 Sep 2026 01:10:40 -0400 Subject: [PATCH 3/5] Run more on windows --- .github/workflows/dist.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/dist.yml b/.github/workflows/dist.yml index 30385adec..caeea11a2 100644 --- a/.github/workflows/dist.yml +++ b/.github/workflows/dist.yml @@ -311,9 +311,19 @@ jobs: python -m devtools ci install-test-pure-wheels - name: Test examples + if: runner.os != 'Windows' run: | python -m devtools test-examples + - name: Test examples (Windows stress test) + if: runner.os == 'Windows' + shell: bash + run: | + for iteration in {1..50}; do + echo "Example tests: iteration ${iteration}/50" + python -m devtools test-examples --exitfirst || exit $? + done + - name: Ensure all headers are accounted for run: | python -m devtools ci scan-headers From 7216044d83a710024d3c9a8a4db965b554155d8f Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Wed, 16 Sep 2026 11:00:39 -0400 Subject: [PATCH 4/5] Enable crash dumps --- .github/workflows/dist.yml | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/.github/workflows/dist.yml b/.github/workflows/dist.yml index caeea11a2..adf64e94b 100644 --- a/.github/workflows/dist.yml +++ b/.github/workflows/dist.yml @@ -315,15 +315,47 @@ jobs: run: | python -m devtools test-examples + # Native fast-fail crashes can bypass Python's exception handlers. + - name: Configure Windows crash dumps + if: runner.os == 'Windows' + shell: python + run: | + import os + from pathlib import Path + import winreg + + dump_folder = Path(os.environ["RUNNER_TEMP"]) / "robotpy-crash-dumps" + dump_folder.mkdir(exist_ok=True) + key_path = r"SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps\python.exe" + with winreg.CreateKeyEx(winreg.HKEY_LOCAL_MACHINE, key_path, 0, winreg.KEY_SET_VALUE) as key: + winreg.SetValueEx(key, "DumpFolder", 0, winreg.REG_EXPAND_SZ, str(dump_folder)) + winreg.SetValueEx(key, "DumpType", 0, winreg.REG_DWORD, 2) + winreg.SetValueEx(key, "DumpCount", 0, winreg.REG_DWORD, 10) + print(f"Windows crash dumps: {dump_folder}") + - name: Test examples (Windows stress test) if: runner.os == 'Windows' shell: bash + env: + PYTEST_ADDOPTS: "-s" + PYTHONFAULTHANDLER: "1" + PYTHONUNBUFFERED: "1" run: | for iteration in {1..50}; do echo "Example tests: iteration ${iteration}/50" python -m devtools test-examples --exitfirst || exit $? done + - name: Upload Windows crash dumps + if: failure() && runner.os == 'Windows' + uses: actions/upload-artifact@v7 + with: + name: "windows-example-crash-dumps-${{ runner.arch }}-${{ matrix.python_version }}" + path: ${{ runner.temp }}/robotpy-crash-dumps/ + if-no-files-found: warn + compression-level: 1 + retention-days: 7 + - name: Ensure all headers are accounted for run: | python -m devtools ci scan-headers From 96da66332b13be4a829ceb2e0ad38336f5301677 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Wed, 16 Sep 2026 20:17:53 -0400 Subject: [PATCH 5/5] This might help --- .github/workflows/dist.yml | 39 +++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/.github/workflows/dist.yml b/.github/workflows/dist.yml index adf64e94b..3e81f03f2 100644 --- a/.github/workflows/dist.yml +++ b/.github/workflows/dist.yml @@ -333,11 +333,48 @@ jobs: winreg.SetValueEx(key, "DumpCount", 0, winreg.REG_DWORD, 10) print(f"Windows crash dumps: {dump_folder}") + - name: Verify Windows crash dumps + if: runner.os == 'Windows' + shell: python + timeout-minutes: 3 + run: | + import os + from pathlib import Path + import subprocess + import sys + + # Crash only a disposable child, using a native fast-fail that invokes WER. + crash_code = ( + "import ctypes; " + "ctypes.windll.kernel32.RaiseFailFastException(None, None, 0)" + ) + process = subprocess.Popen([sys.executable, "-c", crash_code]) + print(f"Verifying native crash dump for probe PID {process.pid}", flush=True) + try: + returncode = process.wait(timeout=120) + except subprocess.TimeoutExpired: + process.kill() + process.wait(timeout=10) + raise RuntimeError("Timed out waiting for the Windows crash-dump probe") from None + # RaiseFailFastException(NULL, NULL, 0) uses STATUS_FAIL_FAST_EXCEPTION. + if returncode != 0xC0000602: + raise RuntimeError(f"Unexpected crash-probe exit code: {returncode:#x}") + + dump_folder = Path(os.environ["RUNNER_TEMP"]) / "robotpy-crash-dumps" + dump_path = dump_folder / f"python.exe.{process.pid}.dmp" + if not dump_path.is_file(): + raise RuntimeError(f"Windows did not produce the expected crash dump: {dump_path}") + with dump_path.open("rb") as dump: + if dump.read(4) != b"MDMP": + raise RuntimeError(f"Invalid or empty Windows crash dump: {dump_path}") + print(f"Verified crash dump: {dump_path} ({dump_path.stat().st_size} bytes)") + # Keep failure artifacts reserved for real crashes, not this intentional one. + dump_path.unlink() + - name: Test examples (Windows stress test) if: runner.os == 'Windows' shell: bash env: - PYTEST_ADDOPTS: "-s" PYTHONFAULTHANDLER: "1" PYTHONUNBUFFERED: "1" run: |