CI / lint (push) Successful in 3s
CI / extension-version (push) Successful in 4s
Build images / build-ml (push) Successful in 4s
Build images / build-agent (push) Successful in 5s
CI / frontend-build (push) Successful in 18s
extension / lint (push) Successful in 18s
CI / backend-lint-and-test (push) Successful in 30s
Build images / sign-extension (push) Successful in 1m41s
Build images / build-web (push) Successful in 11s
CI / integration (push) Successful in 3m42s
extension / lint (pull_request) Successful in 21s
`packaging.sh pathspec` excluded `extension/scripts/**` — the same list
web-ext ignores. But the two lists answer different questions, and this is
the one place they disagree.
packaging.sh is not packaged into the XPI. It does decide the version
string, and build.yml stamps that string into the manifest.json that IS
packaged. Changing how the version is computed therefore changes the shipped
bytes, and the derivation has to see it.
Harmless while every push rebuilt the web image. Step 4 made the rebuild
conditional on the derived revision moving, which turns it into a silent
failure: a packaging.sh change yields a NEW version, so sign-extension misses
its ext-<version> cache and signs — while build-web sees an unmoved revision,
reuses the published image and ships the OLD XPI. One orphaned AMO signature,
and an instance serving code the registry calls current. Found while checking
the ground under step 5, which changes the version format and is exactly the
commit that would have hit it.
Split the list rather than widening the shared one: NOT_VERSION_RELEVANT
drives the pathspec, NOT_PACKAGED_TRACKED still drives web-ext's ignore
list, and scripts/ stays out of the XPI. The two directions are not
symmetric, which is why the version list is the narrower one — too wide
costs a re-sign and a rebuild for a change that ships nothing new, too
narrow serves stale bytes and says nothing.
No version churn: the last packaging.sh commit predates the current
extension revision, so the derived version is unchanged at 1.0.3500147 and
web's revision stays a7e626a67a.
Both suites now pin the disagreement from their own side, because the
tempting fix for either half is to make the lists one again — version.spec.js
asserts the pathspec does NOT exclude scripts while the ignore list still
does, and test_artifact_paths.py asserts packaging.sh is inside the extension
and web path sets.
169 lines
7.1 KiB
Python
169 lines
7.1 KiB
Python
"""`scripts/artifacts.sh` path sets must match what the Dockerfiles copy.
|
|
|
|
Each published artifact's version derives from the newest commit touching its
|
|
own shipped file set (milestone 313). The whole scheme rests on those sets
|
|
being right, and both ways of being wrong are silent:
|
|
|
|
* **too narrow** — a file ships but is not in the set, so the version does not
|
|
move when the content does, and a pin serves stale bytes. This is the
|
|
dangerous direction and the one this module exists for.
|
|
* **too wide** — a file is in the set but never reaches the image, so the
|
|
artifact re-versions and rebuilds for a change it does not ship.
|
|
|
|
Nothing else notices either. The version still derives, CI still goes green,
|
|
and the mismatch only surfaces as "I pinned that build and got the wrong
|
|
bytes". So the Dockerfiles are read here and compared against the declaration.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
ROOT = Path(__file__).resolve().parent.parent
|
|
|
|
# artifact -> (dockerfile, build context relative to the repo root)
|
|
ARTIFACTS = {
|
|
"web": ("Dockerfile", ""),
|
|
"ml": ("Dockerfile.ml", ""),
|
|
"agent": ("agent/Dockerfile", "agent"),
|
|
}
|
|
|
|
# COPY --from=<stage> copies from an earlier build stage, not from the build
|
|
# context, so its source is not a repo path and cannot be in a path set.
|
|
_COPY = re.compile(r"^\s*COPY\s+(?!--from=)(?P<args>.+)$", re.MULTILINE)
|
|
|
|
|
|
def declared_paths(artifact: str) -> list[str]:
|
|
out = subprocess.run(
|
|
["sh", str(ROOT / "scripts" / "artifacts.sh"), "paths", artifact],
|
|
capture_output=True, text=True, check=True, cwd=ROOT,
|
|
).stdout
|
|
return out.split()
|
|
|
|
|
|
def includes(artifact: str) -> list[str]:
|
|
"""The set minus its `:(exclude)…` entries."""
|
|
return [p for p in declared_paths(artifact) if not p.startswith(":(exclude)")]
|
|
|
|
|
|
def copy_sources(dockerfile: str, context: str) -> list[str]:
|
|
"""Repo-relative sources of every context COPY in a Dockerfile."""
|
|
text = (ROOT / dockerfile).read_text()
|
|
sources: list[str] = []
|
|
for m in _COPY.finditer(text):
|
|
args = m.group("args").split()
|
|
# Last arg is the destination; everything before it is a source.
|
|
for src in args[:-1]:
|
|
# `frontend/package-lock.json*` — the glob is an optional-file
|
|
# idiom; the directory it sits in is what matters for coverage.
|
|
src = src.rstrip("*")
|
|
sources.append(f"{context}/{src}" if context else src)
|
|
return sources
|
|
|
|
|
|
def covered_by(path: str, include: str) -> bool:
|
|
"""`path` ships if an include names it or one of its ancestors."""
|
|
path = path.rstrip("/").lstrip("./")
|
|
include = include.rstrip("/")
|
|
return path == include or path.startswith(include + "/")
|
|
|
|
|
|
@pytest.mark.parametrize("artifact", sorted(ARTIFACTS))
|
|
def test_every_copied_path_is_in_the_artifacts_path_set(artifact):
|
|
"""The too-narrow direction — the one that serves stale bytes on a pin."""
|
|
dockerfile, context = ARTIFACTS[artifact]
|
|
inc = includes(artifact)
|
|
for src in copy_sources(dockerfile, context):
|
|
assert any(covered_by(src, i) for i in inc), (
|
|
f"{dockerfile} copies {src!r} into the {artifact} image, but no "
|
|
f"include in scripts/artifacts.sh covers it. The {artifact} "
|
|
f"version will not move when that file changes, so a pinned build "
|
|
f"will serve stale bytes. Add it to the path set.\n"
|
|
f" declared includes: {inc}"
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize("artifact", sorted(ARTIFACTS))
|
|
def test_the_dockerfile_itself_is_in_the_path_set(artifact):
|
|
"""Changing a base image or a RUN changes the artifact as surely as
|
|
changing a source file, so each set must include its own Dockerfile."""
|
|
dockerfile, _ = ARTIFACTS[artifact]
|
|
assert any(covered_by(dockerfile, i) for i in includes(artifact)), (
|
|
f"{dockerfile} is not in the {artifact} path set — a base-image bump "
|
|
f"would not move the version."
|
|
)
|
|
|
|
|
|
def test_the_web_image_versions_on_an_extension_change():
|
|
"""The web image bundles the signed XPI, so the extension's packaged files
|
|
are part of what it ships. Miss this and `:latest` serves a NEW extension
|
|
under an unchanged web version — a pin that quietly disagrees with itself.
|
|
"""
|
|
inc = includes("web")
|
|
assert any(covered_by("extension/background/background.js", i) for i in inc), (
|
|
"the web path set does not cover the extension's packaged files, but "
|
|
"build.yml downloads the signed XPI into frontend/public/extension/ "
|
|
"before the docker build"
|
|
)
|
|
|
|
|
|
def test_the_web_image_versions_on_a_version_derivation_change():
|
|
"""packaging.sh ships in no image, yet it belongs in the sets that bundle
|
|
the XPI — because it decides the version string build.yml stamps into the
|
|
packaged manifest.json. Changing the derivation changes the shipped bytes.
|
|
|
|
Left out, milestone 313 step 4 turns it silent: the new version misses the
|
|
ext-<version> cache and gets signed, while web's revision has not moved, so
|
|
the reuse path republishes the old image and the fresh signature is
|
|
orphaned. Guarded for web and the extension both, since web bundles what
|
|
the extension produces.
|
|
"""
|
|
for artifact in ("extension", "web"):
|
|
inc = includes(artifact)
|
|
excluded = [
|
|
p[len(":(exclude)"):] for p in declared_paths(artifact)
|
|
if p.startswith(":(exclude)")
|
|
]
|
|
path = "extension/scripts/packaging.sh"
|
|
assert any(covered_by(path, i) for i in inc), (
|
|
f"{path} is not in the {artifact} path set"
|
|
)
|
|
assert not any(
|
|
covered_by(path, e.rstrip("*").rstrip("/")) for e in excluded
|
|
), (
|
|
f"{path} is excluded from the {artifact} path set, so a change to "
|
|
f"how the version is derived would not move the version — and "
|
|
f"step 4 would reuse the image that carries the old one"
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"artifact, path",
|
|
[
|
|
# Deliberate exclusions — the too-wide direction. Each of these lives
|
|
# beside shipped code but never reaches an image, and including it
|
|
# would re-version the artifact for a change it does not carry.
|
|
("agent", "agent/README.md"),
|
|
("agent", "agent/ruff.toml"),
|
|
("agent", "agent/docker-compose.yml"),
|
|
# vite builds from src/, index.html and public/; it never reads test/,
|
|
# so a frontend test change cannot reach `dist`.
|
|
("web", "frontend/test/gallery.spec.js"),
|
|
],
|
|
)
|
|
def test_files_that_never_reach_an_image_do_not_version_it(artifact, path):
|
|
paths = declared_paths(artifact)
|
|
excluded = [p[len(":(exclude)"):] for p in paths if p.startswith(":(exclude)")]
|
|
inc = [p for p in paths if not p.startswith(":(exclude)")]
|
|
|
|
included = any(covered_by(path, i) for i in inc)
|
|
exempted = any(covered_by(path, e.rstrip("*").rstrip("/")) for e in excluded)
|
|
assert not included or exempted, (
|
|
f"{path} is in the {artifact} path set but is not copied into the "
|
|
f"image — it would re-version and rebuild {artifact} for a change it "
|
|
f"does not ship."
|
|
)
|