Always wait for child to exit

.github/workflows/dist.yml

@@ -311,9 +311,88 @@ jobs:
         python -m devtools ci install-test-pure-wheels
 
     - name: Test examples
+      if: runner.os != 'Windows'
       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: 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:
+        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

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)

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
@@ -442,10 +457,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:
@@ -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):