tests/test_artifact_identity.py

@@ -29,10 +29,11 @@ from __future__ import annotations
 
 import re
 import subprocess
+from pathlib import Path
 
 import pytest
 
-from test_artifact_paths import ROOT, declared_paths
+ROOT = Path(__file__).resolve().parent.parent
 
 ARTIFACTS = ("web", "ml", "agent", "extension")
 
@@ -40,35 +41,61 @@ ARTIFACTS = ("web", "ml", "agent", "extension")
 _REVISION = re.compile(r"^[0-9a-f]{12}$")
 
 
-def revision(artifact: str) -> str:
+# Everything here goes through artifacts.sh rather than importing a sibling
+# test module. That is the interface build.yml actually calls, so the tests
+# exercise the contract instead of a Python re-implementation of it — and no
+# other test module in this repo imports another, so a cross-test import would
+# be a new convention introduced for no gain.
+def artifacts(*args: str) -> str:
     return subprocess.run(
-        ["sh", str(ROOT / "scripts" / "artifacts.sh"), "revision", artifact],
+        ["sh", str(ROOT / "scripts" / "artifacts.sh"), *args],
         capture_output=True, text=True, check=True, cwd=ROOT,
-    ).stdout.strip()
+    ).stdout
+
+
+def revision(artifact: str) -> str:
+    return artifacts("revision", artifact).strip()
+
+
+def newest_by_commit_time(artifact: str) -> str:
+    """The full SHA of the newest commit touching this artifact's shipped set.
+
+    Ordered by committer TIME, matching what artifacts.sh means. Deliberately
+    not `git log -1`: git's default order is reverse-chronological only within
+    topological constraints, so on a merged history it can name a different
+    commit than the newest timestamp does. They agree on this repo today, and
+    a test that silently depends on them continuing to agree would be a flake
+    waiting for the branch shape that separates them.
+    """
+    paths = artifacts("paths", artifact).split()
+    log = subprocess.run(
+        ["git", "log", "--format=%ct %H", "HEAD", "--", *paths],
+        capture_output=True, text=True, check=True, cwd=ROOT,
+    ).stdout.split("\n")
+    commits = [line.split(" ", 1) for line in log if line.strip()]
+    assert commits, (
+        f"no commit in this history touches the {artifact} path set — the "
+        f"derivation has nothing to stand on"
+    )
+    return max(commits, key=lambda c: int(c[0]))[1]
 
 
 @pytest.mark.parametrize("artifact", ARTIFACTS)
 def test_revision_is_the_commit_its_own_shipped_files_last_changed_in(artifact):
     """The claim the whole skip decision rests on.
 
-    Asked of git directly rather than of the script, so this fails if the
+    Computed from git rather than asked of the script, so it fails if the
-    derivation ever stops meaning what it says — deriving from HEAD, from a
+    derivation ever stops meaning what it says — switching to HEAD, to a build
-    build clock, or from a path set it did not actually use.
+    clock, or to a path set it did not actually use. Each of those still
+    produces a plausible 12-hex value, which is why this is worth asserting
+    rather than eyeballing.
     """
-    paths = declared_paths(artifact)
+    expected = newest_by_commit_time(artifact)
-    expected = subprocess.run(
+    got = revision(artifact)
-        ["git", "log", "--format=%H", "-1", "HEAD", "--", *paths],
+    assert expected.startswith(got), (
-        capture_output=True, text=True, check=True, cwd=ROOT,
+        f"{artifact} derives {got!r}, but the newest commit touching its "
-    ).stdout.strip()
+        f"shipped files is {expected[:12]!r}. The label stamped into the image "
-
+        f"would not identify its own content."
-    assert expected, (
-        f"no commit in this history touches the {artifact} path set — the "
-        f"derivation has nothing to stand on"
-    )
-    assert expected.startswith(revision(artifact)), (
-        f"{artifact} derives {revision(artifact)!r}, but the newest commit "
-        f"touching its shipped files is {expected[:12]!r}. The label stamped "
-        f"into the image would not identify its own content."
     )
 
 
@@ -80,23 +107,3 @@ def test_revision_is_a_legal_label_value_and_is_stable(artifact):
     first = revision(artifact)
     assert _REVISION.match(first), f"{first!r} is not a 12-char hex revision"
     assert first == revision(artifact), "revision is not stable across calls"
-
-
-def test_an_artifact_whose_paths_did_not_change_keeps_its_revision():
-    """The property that makes skipping possible at all.
-
-    The agent's set is disjoint from web's, so the two must be free to differ.
-    Asserting they *are* different today would pin an accident of history —
-    what matters is that the derivation is per-artifact rather than global, so
-    this asserts each artifact's revision is drawn from its own path set.
-    """
-    seen = {a: revision(a) for a in ARTIFACTS}
-    for artifact, rev in seen.items():
-        touched = subprocess.run(
-            ["git", "log", "--format=%H", "-1", "HEAD", "--", *declared_paths(artifact)],
-            capture_output=True, text=True, check=True, cwd=ROOT,
-        ).stdout.strip()
-        assert touched.startswith(rev), (
-            f"{artifact}'s revision {rev!r} is not the newest commit touching "
-            f"its own paths — the derivation is not per-artifact"
-        )