ci(unit-tests): save each xdist worker's coverage at session finish and keep the process log (#1423)

.github/coverage.toml

@@ -13,6 +13,14 @@
 #                data before os._exit(), which skips atexit: execnet calls it on
 #                a worker whose execution outlives shutdown, and pytest-timeout's
 #                thread method on a hung test (#1387).
+#   sigterm   -- installs a SIGTERM handler that saves the data. `patch =
+#                ["_exit"]` covers os._exit and atexit covers a clean exit, but
+#                nothing covered a signal, so a worker SIGTERMed during
+#                shutdown would leave no file -- one candidate for #1423, whose
+#                actual cause is not established. Does not help against
+#                SIGKILL -- execnet's post-timeout kill is `Popen.kill()` --
+#                which is why `tests/conftest.py` also saves at
+#                `pytest_sessionfinish` rather than relying on this alone.
 #   core      -- "ctrace", not the sys.monitoring default on 3.12+. Tests
 #                purge and re-import pcapkit, and sysmon keeps state for every
 #                generation's code objects: measured on test_http_unit.py
@@ -30,6 +38,7 @@ patch = [
     "subprocess",
     "_exit",
 ]
+sigterm = true
 core = "ctrace"
 
 [tool.coverage.report]

.github/workflows/unit-tests.yml

@@ -172,16 +172,26 @@ jobs:
           # direct children in the log above) left one.
           python - <<'PY'
           import glob, os, re, sys
-          parent, cmd = {}, {}
+          parent, cmd, atexit_ = {}, {}, {}
           log = 'coverage-processes.log'
           for line in open(log, errors='replace').readlines() if os.path.exists(log) else []:
               if m := re.match(r' *(\d+)\.\w+: New process(: cmd| parent pid): (.*)', line):
                   (parent if m[2] == ' parent pid' else cmd)[m[1]] = m[3]
+              elif m := re.match(r' *(\d+)\.\w+: (atexit|sigterm): pid: \d+, (.*)', line):
+                  atexit_[m[1]] = f'{m[2]}: {m[3]}'
           root = [p for p, c in cmd.items() if "'run', '--rcfile=.github/coverage.toml'" in c]
           workers = sorted(p for p, q in parent.items() if root and q == root[0])
           saved = set(re.findall(r'\.pid(\d+)\.', ' '.join(glob.glob('.coverage.*'))))
           missing = [p for p in root + workers if p not in saved]
           print(f'controller {root}, {len(workers)} xdist workers, missing data: {missing}')
+          # #1423: which process it was, and whether coverage's atexit handler ran
+          # in it at all -- a pid with no `atexit:` line died on a signal before
+          # saving, one with such a line reached the save and lost it afterwards.
+          # Those are different bugs, and the log that distinguishes them is kept
+          # as an artifact by the step below rather than only read here.
+          for p in missing:
+              print(f'  pid {p}: cmd {cmd.get(p, "<no cmd line logged>")}')
+              print(f'  pid {p}: {atexit_.get(p, "no atexit/sigterm entry -- died before saving")}')
           if len(root) != 1 or not workers or missing:
               why = f'no data file from pid {missing}' if missing else 'the process log is missing or unreadable'
               with open(os.environ['GITHUB_STEP_SUMMARY'], 'a') as summary:
@@ -227,6 +237,21 @@ jobs:
           print('Per-file detail: the `coverage-html` artifact of this run.')
           PY
 
+      # #1423: the guard above fires on runs whose tests all pass, and the only
+      # record of what became of the lost process is this log -- it carries each
+      # measured process's command line, its parent, and the `atexit:` line
+      # coverage writes as it saves. Reading it off the runner is impossible once
+      # the job ends, so keep it whenever the leg ran, not only on failure: a run
+      # that passed is the baseline the failing one has to be read against.
+      - name: Upload coverage process log
+        if: ${{ !cancelled() && matrix.coverage }}
+        uses: actions/upload-artifact@v7
+        with:
+          name: coverage-processes-log
+          path: coverage-processes.log
+          retention-days: 30
+          if-no-files-found: warn
+
       - name: Upload coverage HTML report
         if: ${{ !cancelled() && matrix.coverage }}
         uses: actions/upload-artifact@v7

docs/source/contributing/releasing.rst

@@ -46,7 +46,7 @@ Editing ``__version__`` by hand without also moving :file:`CITATION.cff`'s
 (``:494-514``) asserts the two agree, and ``create-release.yml``'s
 ``unit-tests`` job (``:362-376``) calls ``unit-tests.yml`` with
 ``gate-only: true``, which runs the **full** suite rather than the tiered
-subset an ordinary push runs (its ``gate`` job, ``unit-tests.yml:1189-1258``).
+subset an ordinary push runs (its ``gate`` job, ``unit-tests.yml:1214-1283``).
 That gate runs whenever a release will -- a moved ``__version__`` is exactly
 what makes ``version_check``'s evidence read "not yet published" -- and every
 publishing job requires it to have succeeded (``:392,493,573,674``), so a
@@ -56,7 +56,7 @@ Either run the script, or move both fields by hand in the same commit.
 This is the **only** edit a person makes to get a release started -- the tag,
 the Release, and every upload are the workflow's job from here. Two other
 checks run unconditionally on the same path, though neither is a step in *this*
-process: the ``changelog`` job (``unit-tests.yml:1049-1070``) fails outright if
+process: the ``changelog`` job (``unit-tests.yml:1074-1095``) fails outright if
 ``CHANGELOG.md`` has drifted from its source entry under
 :file:`docs/source/changelog/`, and the release-body step warns, without
 failing, if that entry's heading still reads "unreleased"

docs/source/contributing/workflows.rst

@@ -245,7 +245,7 @@ and ``project-tests``, and ``required-checks`` (see `Required Status
 Checks`_ below), gated out by their own ``if:`` rather than by having
 already run. ``changelog`` carries no
 ``if:`` at all and runs on every path regardless -- deliberately, per its own
-comment (``unit-tests.yml:1037-1043``): ``create-release.yml`` feeds
+comment (``unit-tests.yml:1062-1068``): ``create-release.yml`` feeds
 ``CHANGELOG.md`` to the GitHub Release body, so the release path is exactly
 where a drifted file must not go unchecked. Confirmed on run `36210743295
 <https://github.com/JarryShaw/PyPCAPKit/actions/runs/36210743295>`__ (a
@@ -352,9 +352,14 @@ and ``patch = ["subprocess"]``, without which only the xdist controller is
 measured, not its workers, plus ``core = "ctrace"``: with the suite purging
 and re-importing pcapkit, one module's peak RSS was 4.17 GiB under the default
 ``sys.monitoring`` core against 0.41 GiB under ``ctrace``, and that growth
-killed the runner. The leg then writes the total and a per-package table to the
-job summary and uploads the HTML report as the ``coverage-html``
-artifact (30 days), with the bare numbers as the ``coverage-summary`` artifact.
+killed the runner. ``sigterm = true`` and a ``pytest_sessionfinish`` hook in
+:file:`tests/conftest.py` that saves each xdist worker's data early keep a
+worker that dies during shutdown from losing its file
+(`#1423 <https://github.com/JarryShaw/PyPCAPKit/issues/1423>`__). The leg then
+writes the total and a per-package table to the job summary and uploads the HTML
+report as the ``coverage-html`` artifact (30 days), with the bare numbers as the
+``coverage-summary`` artifact and the per-process debug log as
+``coverage-processes-log``.
 
 The PR comment comes from **Coverage Comment** (:file:`coverage-comment.yml`),
 not from ``unit-tests.yml``. That file is also a reusable workflow, a called
@@ -384,7 +389,7 @@ grepping every workflow file for its name:
      - Where
    * - ``Required checks passed``
      - job ``required-checks``
-     - ``unit-tests.yml:1346``
+     - ``unit-tests.yml:1371``
    * - ``Compat Python 3.10``
      - job ``compatibility``, matrix leg ``3.10``
      - ``python-compatibility.yml:31,41``
@@ -402,20 +407,20 @@ grepping every workflow file for its name:
      - ``python-compatibility.yml:31,45``
 
 ``Required checks passed`` is defined by exactly one job
-(``unit-tests.yml:1346``). ``Compat Python`` is defined by two in
+(``unit-tests.yml:1371``). ``Compat Python`` is defined by two in
 ``python-compatibility.yml``: the required ``compatibility`` job (``:31``,
 ``Compat Python ${{ matrix.python-version }}``, expanding to the five required
 legs at ``:41-45``) and the non-required ``compatibility-nightly`` job (``:69``,
 ``Compat Python 3.15 (scheduled)``, see below). A comment at
-``unit-tests.yml:1261`` mentions the string without defining it.
+``unit-tests.yml:1286`` mentions the string without defining it.
 
 ``Required checks passed`` is an aggregate, not a single check run. The ruleset
 requires it and the five ``Compat Python 3.10``-``3.14`` legs, nothing else, as
-``unit-tests.yml``'s own comment (``unit-tests.yml:1260-1270``) records. It
+``unit-tests.yml``'s own comment (``unit-tests.yml:1285-1295``) records. It
 ``needs:`` the gating jobs of ``unit-tests.yml`` -- ``test``, ``integration``,
 ``engine-tests`` and ``pypcap-parity``, plus ``changes`` and ``project-tests``
 for the docs-only path -- and checks each result explicitly
-(``unit-tests.yml:1345-1399``). Rulesets match check names literally, with no
+(``unit-tests.yml:1370-1424``). Rulesets match check names literally, with no
 wildcard, so requiring the matrix cells one by one would mean editing the
 ruleset whenever a matrix changes; the aggregate keeps the required list fixed.
 It accepts the four legs as ``skipped`` only on a docs-only pull request, where
@@ -424,13 +429,13 @@ instead. The ``Compat`` legs stay required separately because they come from
 ``python-compatibility.yml``, and a job cannot ``needs:`` a job in another
 workflow file. This job runs on Unit Tests' own ``push``/``pull_request``
 triggers; the ``gate-only: true`` reusable calls documented above skip it via
-``if: ${{ always() && inputs.gate-only != true }}`` (``unit-tests.yml:1348``),
+``if: ${{ always() && inputs.gate-only != true }}`` (``unit-tests.yml:1373``),
 so it is never produced -- and never expected -- on those paths.
 
 ``Compat Python 3.10``-``3.14`` are five ordinary matrix legs, not an
 aggregate: ``python-compatibility.yml``'s own comment (``:37-40``) and
 ``unit-tests.yml``'s matching one for its own, differently-matrixed
-``test``/``integration`` jobs (``unit-tests.yml:59-62``, ``unit-tests.yml:256``) both say why 3.15 is
+``test``/``integration`` jobs (``unit-tests.yml:59-62``, ``unit-tests.yml:281``) both say why 3.15 is
 excluded from every *required* matrix in this repository -- the ruleset's
 required-checks list stops at 3.14, so a 3.15 leg cannot gate a merge and is
 kept advisory (``continue-on-error: true`` in ``python-compatibility.yml``'s

tests/conftest.py

@@ -168,6 +168,51 @@ def pytest_sessionstart(session: 'pytest.Session') -> 'None':
     _pin_module_snapshot()
 
 
+def pytest_sessionfinish(session: 'pytest.Session') -> 'None':
+    """Write an xdist worker's coverage data out before the process can die (#1423).
+
+    The coverage leg of ``unit-tests.yml`` measures each worker through
+    coverage's ``patch = ["subprocess"]`` hook, and that measurement is saved by
+    an :mod:`atexit` handler -- so it is written *after* the worker has already
+    reported its results and told the controller it is finished. Anything that
+    kills the process in that window takes the whole worker's data with it, and
+    takes it silently: pytest has its results, so every test still passes, while
+    ``coverage combine`` merges one file fewer and reports the rest as the whole.
+    That is #1387, and the guard in ``Report coverage`` turns it into a failed
+    step rather than a quietly low number -- four times on 2026-10-08.
+
+    Saving here closes the window instead of policing it. :func:`cov.save` is
+    safe to call mid-run (coverage's own long-running-process recipe), and the
+    later ``atexit`` save appends to the same file, so the data file exists from
+    this point on and a worker that dies during shutdown costs at most the arcs
+    executed after this hook.
+
+    A no-op outside that one leg: it returns immediately unless the process is
+    an xdist worker (``workerinput``) whose coverage was auto-started by the
+    subprocess patch.
+
+    Args:
+        session: The pytest session, for its config's ``workerinput``.
+
+    """
+    if not hasattr(session.config, 'workerinput'):
+        return
+    try:
+        import coverage  # pylint: disable=import-outside-toplevel
+    except ImportError:  # pragma: no cover
+        return
+    cov = getattr(coverage.process_startup, 'coverage', None)
+    if cov is None:
+        return
+    try:
+        cov.save()
+    except Exception:  # pragma: no cover  # pylint: disable=broad-except
+        # Deliberately broad, as in `pytest_sessionstart`: a failure to flush
+        # early must not fail a run whose tests have already passed. The atexit
+        # save still gets its turn, and the guard still catches a lost file.
+        pass
+
+
 @pytest.fixture(autouse=True)
 def restore_module_table() -> 'Iterator[None]':
     """Put the :mod:`pcapkit` region of :data:`sys.modules` back to a known-good state.

tests/project/test_coverage_data_guard.py

@@ -26,7 +26,13 @@
 import sys
 import tempfile
 import textwrap
+import types
+import typing
 import unittest
+import unittest.mock
+
+if typing.TYPE_CHECKING:
+    import pytest
 
 ROOT = pathlib.Path(__file__).resolve().parents[2]
 WORKFLOW = ROOT / '.github' / 'workflows' / 'unit-tests.yml'
@@ -155,6 +161,61 @@ def test_the_child_lookup_allows_a_padded_pid(self) -> None:
         """The real-log test above finds its child under a pid shorter than the padding."""
         self.assertEqual(CHILD.search(_entry(9832, 9800, "['-c']"))[1], '9832')
 
+    def test_a_missing_worker_is_named_with_its_fate(self) -> None:
+        """#1423: the failure says which process it was and whether its atexit save ran."""
+        log = (_entry(100, 1, CONTROLLER_CMD) + _entry(101, 100, "['-c']")
+               + _entry(102, 100, "['-c']") + _entry(103, 100, "['-c']")
+               + '  102.ab12: atexit: pid: 102, instance: <Coverage data_file=x>\n')
+        self.write(log, [100, 101])
+        result = self.run_guard()
+        self.assertIn("pid 102: cmd ['-c']", result.stdout)
+        self.assertIn('pid 102: atexit: instance: <Coverage data_file=x>', result.stdout)
+        self.assertIn('pid 103: no atexit/sigterm entry -- died before saving', result.stdout)
+        self.assert_fails("no data file from pid ['102', '103']")
+
+    def test_the_process_log_is_kept_as_an_artifact(self) -> None:
+        """The log is the only record of a lost worker's fate once the runner is gone (#1423)."""
+        text = WORKFLOW.read_text(encoding='utf-8')
+        step = text.split('- name: Upload coverage process log', 1)[1]
+        step = step.split('\n      - name:', 1)[0]
+        self.assertIn('if: ${{ !cancelled() && matrix.coverage }}', step)
+        self.assertIn(f'path: {LOG}', step)
+
+
+class WorkerSessionFinishSaveTests(unittest.TestCase):
+    """#1423: an xdist worker writes its data at session finish, not only at exit."""
+
+    def finish(self, *, worker: bool) -> int:
+        """Run the conftest hook under a stand-in auto-started coverage; return its saves."""
+        import coverage  # pylint: disable=import-outside-toplevel
+
+        from tests import conftest  # pylint: disable=import-outside-toplevel
+
+        cov = unittest.mock.Mock()
+        sentinel = object()
+        before = getattr(coverage.process_startup, 'coverage', sentinel)
+        coverage.process_startup.coverage = cov  # type: ignore[attr-defined]
+        try:
+            config = types.SimpleNamespace(**({'workerinput': {}} if worker else {}))
+            conftest.pytest_sessionfinish(typing.cast('pytest.Session',
+                                                      types.SimpleNamespace(config=config)))
+        finally:
+            if before is sentinel:
+                del coverage.process_startup.coverage  # type: ignore[attr-defined]
+            else:
+                coverage.process_startup.coverage = before  # type: ignore[attr-defined]
+        return int(cov.save.call_count)
+
+    @unittest.skipUnless(_has_coverage(), 'needs coverage 7.10+ (`[run] patch`)')
+    def test_a_worker_saves(self) -> None:
+        """The file exists before shutdown, so a death there cannot lose it whole."""
+        self.assertEqual(self.finish(worker=True), 1)
+
+    @unittest.skipUnless(_has_coverage(), 'needs coverage 7.10+ (`[run] patch`)')
+    def test_the_controller_does_not(self) -> None:
+        """The controller saves at its own exit; only workers die after reporting."""
+        self.assertEqual(self.finish(worker=False), 0)
+
 
 if __name__ == '__main__':
     unittest.main()

tests/project/test_coverage_rcfile.py

@@ -49,6 +49,10 @@ def test_the_xdist_workers_are_measured(self) -> None:
         self.assertIs(run.get('parallel'), True)
         self.assertIn('subprocess', run.get('patch', ()))
 
+    def test_a_sigterm_saves_the_data(self) -> None:
+        """A worker SIGTERMed during shutdown otherwise leaves no data file (#1423)."""
+        self.assertIs(self.rcfile['run'].get('sigterm'), True)
+
     def test_the_core_is_ctrace(self) -> None:
         """sysmon grew one module's peak RSS from 0.40 to 4.17 GiB and killed the runner (#1064)."""
         self.assertEqual(self.rcfile['run'].get('core'), 'ctrace')