fix: tgtrace writes its trace atomically; verify refuses a missing or broken trace (#108)

README.md

@@ -12,7 +12,7 @@ instead of being folded into PASS.
 python3 -m testgraph.verify --repo . --base origin/main -- tests/ -q
 # exit 0  every selected journey ran and passed
 # exit 1  a journey's tests failed
-# exit 2  refused: the index can't be trusted
+# exit 2  refused: the index can't be trusted, or the run left no readable trace
 # exit 3  a selected journey has no journey-level test, or the selection is UNKNOWN — named, not hidden
 ```
 

harness/plugin/tgtrace.py

@@ -183,8 +183,20 @@ def pytest_unconfigure(config):
     directory = os.path.dirname(os.path.abspath(_out_path))
     if directory:
         os.makedirs(directory, exist_ok=True)
-    with open(_out_path, "w") as f:
-        json.dump(payload, f, indent=1, sort_keys=True)
+    # Write beside the target and rename (#108): a run killed mid-dump used to
+    # leave a truncated file where a reader expects a whole one. os.replace is
+    # atomic on the same filesystem, so the target is either absent or complete.
+    tmp = f"{_out_path}.tmp.{os.getpid()}"
+    try:
+        with open(tmp, "w") as f:
+            json.dump(payload, f, indent=1, sort_keys=True)
+        os.replace(tmp, _out_path)
+    except BaseException:
+        try:
+            os.remove(tmp)
+        except OSError:
+            pass
+        raise
 
 
 def pytest_runtest_call(item):

testgraph/pytest_adapter.py

@@ -320,8 +320,15 @@ def run(repo, registry_path=None, db_path=None, pytest_args=(), commit=None,
             )
 
         import json
-        with open(trace_path) as f:
-            trace = json.load(f)
+        try:
+            with open(trace_path) as f:
+                trace = json.load(f)
+        except (OSError, ValueError) as exc:
+            # Same refusal as verify (#108): a cut-short run is not a verdict.
+            return None, f"the tgtrace trace is truncated or not JSON ({exc})"
+        if not isinstance(trace, dict):
+            return None, (f"the trace file is not a tgtrace payload (a JSON "
+                          f"{type(trace).__name__}, not an object)")
         if trace_out:
             with open(trace_out, "w") as f:
                 json.dump(trace, f, indent=1, sort_keys=True)

testgraph/verify.py

@@ -214,10 +214,33 @@ def run(repo, base, head="HEAD", registry_path=None, db_path=None,
             proc = runner()
 
             import json
-            trace = {}
-            if os.path.isfile(trace_path):
+            # A missing or unreadable trace is a broken run, not a verdict
+            # (#108). Read as `{}` it reported every selected journey as having
+            # no journey-level test (exit 3); a truncated one raised out of here
+            # and exited 1, the code for "tests failed".
+            if not os.path.isfile(trace_path):
+                # pytest's own stderr usually names the cause (a bad flag, the
+                # plugin's xdist refusal); the guess below is only a fallback.
+                tail = (getattr(proc, "stderr", "") or "").strip()[-400:]
+                return None, (
+                    f"pytest exited {getattr(proc, 'returncode', '?')} and tgtrace "
+                    f"wrote no trace — the run was killed, or the plugin did not "
+                    f"load; no verdict was recorded"
+                    + (f"\n  pytest stderr (tail): {tail}" if tail else "")
+                )
+            try:
                 with open(trace_path) as fh:
                     trace = json.load(fh)
+            except (OSError, ValueError) as exc:
+                return None, (
+                    f"the tgtrace trace is truncated or not JSON ({exc}) — the run "
+                    f"was cut short; no verdict was recorded"
+                )
+            if not isinstance(trace, dict):
+                return None, (
+                    f"the trace file is not a tgtrace payload (a JSON "
+                    f"{type(trace).__name__}, not an object); no verdict was recorded"
+                )
             verdicts = {}
             if os.path.isfile(junit_path):
                 verdicts = pa.parse_junit(junit_path)
@@ -383,7 +406,8 @@ def main(argv=None):
         prog="testgraph.verify",
         description="run the declared tests for the journeys a diff endangers",
         epilog="Exit: 0 every selected journey ran and passed, 1 a journey "
-               "failed, 2 refused (untrustworthy index), 3 incomplete — at "
+               "failed, 2 refused (untrustworthy index, or no readable trace), "
+               "3 incomplete — at "
                "least one selected journey had no passing declared test, or "
                "the selection was NONE but UNKNOWN.\n"
                "Put pytest's own flags after `--`.",

tests/test_verify.py

@@ -9,6 +9,9 @@
 run printed `NO JOURNEY-LEVEL TEST: J1, J5, J6, J7` and exited 0 — four journeys
 unverified behind a green from the other four.
 """
+import contextlib
+import io
+import json
 import os
 import shutil
 import sqlite3
@@ -343,16 +346,214 @@ def close(self):
             def __getattr__(self, n):
                 return getattr(self._c, n)
 
+        # A whole (empty) trace, so the run goes through validate and the
+        # ledger section — the part a leak would hide in. With no trace it now
+        # stops at the #108 refusal and would test nothing past it.
+        work = os.path.join(tmp, "work")
+        os.makedirs(work)
+
+        class FixedDir:
+            def __enter__(self):
+                return work
+
+            def __exit__(self, *exc):
+                return False
+
+        def runner():
+            with open(os.path.join(work, "trace.json"), "w") as fh:
+                fh.write('{"tests": {}, "declared": {}}')
+            return types.SimpleNamespace(returncode=0)
+
         with unittest.mock.patch.object(
-                verify.dbmod, "connect", lambda p: Proxy(real(p))):
-            verify.run(repo, "HEAD~1", "HEAD", registry_path=registry, db_path=db,
-                       _runner=lambda: types.SimpleNamespace(returncode=0),
-                       append=lambda row: True)
+                verify.dbmod, "connect", lambda p: Proxy(real(p))), \
+                unittest.mock.patch.object(verify.tempfile, "TemporaryDirectory",
+                                           lambda: FixedDir()):
+            summary, err = verify.run(repo, "HEAD~1", "HEAD", registry_path=registry,
+                                      db_path=db, _runner=runner,
+                                      append=lambda row: True)
+        self.assertIsNone(err)
+        self.assertIsNotNone(summary["marker_expression"])
         # select opens one and verify opens one; both must be closed
         self.assertEqual(len(opened), 2)
         self.assertEqual(len(closed), len(opened))
 
 
+class UnreadableTraceTests(unittest.TestCase):
+    """Issue #108: a truncated trace made `json.load` raise, and the uncaught
+    exception exited 1 — the code for "tests failed". A missing trace read as
+    an empty one, so every selected journey was reported as having no
+    journey-level test (exit 3). Both are a broken run, not a verdict."""
+
+    def setUp(self):
+        self.tmp = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True)
+        self.db = _db_on_disk(self.tmp, build_fixture())
+        self.registry = _registry_file(self.tmp, {
+            "J1": {"name": "ok", "entries": [{"name": "handler_a", "file": "app/svc.py"}]},
+        })
+        self.repo, run = _git_repo(self.tmp, {"app/svc.py": 22})
+        full = os.path.join(self.repo, "app", "svc.py")
+        with open(full) as fh:
+            lines = fh.readlines()
+        lines[11] = "changed = 1\n"  # inside handler_a -> J1
+        with open(full, "w") as fh:
+            fh.writelines(lines)
+        run("git", "add", "-A")
+        run("git", "commit", "-qm", "touch")
+        self.work = os.path.join(self.tmp, "work")
+        os.makedirs(self.work)
+
+    def _run(self, trace_text):
+        work = self.work
+
+        class FixedDir:
+            def __enter__(self):
+                return work
+
+            def __exit__(self, *exc):
+                return False
+
+        def runner():
+            if trace_text is not None:
+                with open(os.path.join(work, "trace.json"), "w") as fh:
+                    fh.write(trace_text)
+            return types.SimpleNamespace(returncode=0)
+
+        with unittest.mock.patch.object(verify.tempfile, "TemporaryDirectory",
+                                        lambda: FixedDir()):
+            return verify.run(self.repo, "HEAD~1", "HEAD", registry_path=self.registry,
+                              db_path=self.db, _runner=runner, append=lambda r: True)
+
+    def test_a_truncated_trace_is_refused_not_a_crash(self):
+        summary, err = self._run('{"tests": {"t::a": [["app/svc.py", "hand')
+        self.assertIsNone(summary)
+        self.assertIn("trace", err)
+        self.assertIn("truncated or not JSON", err)
+
+    def test_a_trace_that_is_not_an_object_is_refused(self):
+        """Review H1: `null` or `[]` parse, then crashed validate() with an
+        AttributeError — exit 1 again."""
+        for text in ("null", "[]", '"x"'):
+            with self.subTest(text):
+                summary, err = self._run(text)
+                self.assertIsNone(summary)
+                self.assertIn("not a tgtrace payload", err)
+
+    def test_the_refusal_carries_pytest_stderr(self):
+        """Review H3: the real cause (an xdist refusal, a bad flag) was in
+        pytest's stderr, which was dropped."""
+        work = self.work
+
+        class FixedDir:
+            def __enter__(self):
+                return work
+
+            def __exit__(self, *exc):
+                return False
+
+        runner = lambda: types.SimpleNamespace(
+            returncode=4, stderr="ERROR: usage: unrecognized arguments: --bogus")
+        with unittest.mock.patch.object(verify.tempfile, "TemporaryDirectory",
+                                        lambda: FixedDir()):
+            summary, err = verify.run(self.repo, "HEAD~1", "HEAD",
+                                      registry_path=self.registry, db_path=self.db,
+                                      _runner=runner, append=lambda r: True)
+        self.assertIn("unrecognized arguments: --bogus", err)
+
+    def test_a_missing_trace_is_refused_not_uncovered(self):
+        summary, err = self._run(None)
+        self.assertIsNone(summary)
+        self.assertIn("wrote no trace", err)
+
+    def test_main_exits_refused(self):
+        with unittest.mock.patch.object(verify, "run", return_value=(None, "trace broke")), \
+                contextlib.redirect_stdout(io.StringIO()):
+            rc = verify.main(["--repo", self.repo, "--base", "HEAD~1"])
+        self.assertEqual(rc, verify.EXIT_REFUSED)
+
+
+class AtomicTraceWriteTests(unittest.TestCase):
+    """#108: the trace was written in place, so a run killed mid-write left a
+    truncated file where a reader expects a whole one."""
+
+    def test_a_write_that_dies_leaves_no_partial_file(self):
+        from harness.plugin import tgtrace
+        tmp = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, tmp, ignore_errors=True)
+        out = os.path.join(tmp, "trace.json")
+
+        seen_target = []
+
+        def dying_dump(obj, fh, **kw):
+            fh.write('{"tests": {')
+            # a SIGKILL here runs no cleanup: the target must not exist yet
+            seen_target.append(os.path.exists(out))
+            raise KeyboardInterrupt
+
+        with unittest.mock.patch.object(tgtrace, "_out_path", out), \
+                unittest.mock.patch.object(tgtrace, "_stop", lambda: None), \
+                unittest.mock.patch.object(tgtrace.json, "dump", dying_dump):
+            with self.assertRaises(KeyboardInterrupt):
+                tgtrace.pytest_unconfigure(None)
+        self.assertFalse(os.path.exists(out))
+        self.assertEqual(os.listdir(tmp), [])
+        self.assertEqual(seen_target, [False])
+
+    def test_a_whole_write_lands(self):
+        from harness.plugin import tgtrace
+        tmp = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, tmp, ignore_errors=True)
+        out = os.path.join(tmp, "trace.json")
+        with unittest.mock.patch.object(tgtrace, "_out_path", out), \
+                unittest.mock.patch.object(tgtrace, "_stop", lambda: None):
+            tgtrace.pytest_unconfigure(None)
+        with open(out) as fh:
+            self.assertIn("tests", json.load(fh))
+        self.assertEqual(os.listdir(tmp), ["trace.json"])
+
+
+class AdapterTruncatedTraceTests(unittest.TestCase):
+    def test_the_adapter_refuses_a_truncated_trace(self):
+        from testgraph import pytest_adapter as pa
+        tmp = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, tmp, ignore_errors=True)
+        db = _db_on_disk(tmp, build_fixture())
+        registry = _registry_file(tmp, {
+            "J1": {"name": "ok", "entries": [{"name": "handler_a", "file": "app/svc.py"}]},
+        })
+        repo, _ = _git_repo(tmp, {"app/svc.py": 22})
+        work = os.path.join(tmp, "work")
+        os.makedirs(work)
+
+        class FixedDir:
+            def __enter__(self):
+                return work
+
+            def __exit__(self, *exc):
+                return False
+
+        def runner():
+            with open(os.path.join(work, "junit.xml"), "w") as fh:
+                fh.write("<testsuite/>")
+            with open(os.path.join(work, "trace.json"), "w") as fh:
+                fh.write(self.trace_text)
+            return types.SimpleNamespace(returncode=0, stderr="")
+
+        with unittest.mock.patch.object(pa.tempfile, "TemporaryDirectory",
+                                        lambda: FixedDir()):
+            summary, err = pa.run(repo, registry_path=registry, db_path=db,
+                                  _runner=runner, append=lambda r: True)
+        self.assertIsNone(summary)
+        self.assertIn(self.expect, err)
+
+    trace_text, expect = '{"tests": {', "truncated or not JSON"
+
+
+class AdapterNonObjectTraceTests(AdapterTruncatedTraceTests):
+    """Review H1, adapter side: `[]` crashed at `trace.get("tests")`."""
+    trace_text, expect = "[]", "not a tgtrace payload"
+
+
 class SummaryShapeTests(unittest.TestCase):
     """Every exit from `run` carries the same keys.