Build images / sign-extension (push) Successful in 4s
Build images / build-agent (push) Successful in 4s
Build images / build-ml (push) Successful in 5s
CI / lint (push) Successful in 2s
CI / extension-version (push) Successful in 3s
CI / frontend-build (push) Successful in 22s
Build images / build-web (push) Successful in 3s
CI / backend-lint-and-test (push) Successful in 31s
CI / integration (push) Successful in 3m40s
extension / lint (pull_request) Successful in 21s
Missed in 2e01242. This module pinned zero-padded `YYYY.MM.DD.HHMM` for all
four artifacts, which is the family shape and was right until the extension
acquired a documented reason not to use it. Both assertions failed exactly as
written, on the value they were written to catch.
Rather than exempt the extension, the exception is pinned to the constraint
that justifies it:
* `test_version_is_zero_padded_calver` now covers the three padded artifacts.
* A new sibling covers the unpadded one against **AMO's own grammar** —
`2026.08.29.0201` fails it, so a regression to padding fires immediately.
Matching only `YYYY.M.D.HHMM` would not: on a date with no leading zeros the
two renderings are the same string, so a padding regression would sit unseen
until the first single-digit month, and surface as a burned AMO version
rather than a red lane.
* `test_version_and_revision_describe_the_same_commit` compares NUMBERS, per
rule 148's own definition of comparison — so one assertion covers both
renderings and says the real thing: whatever the padding, the extension must
denote exactly the value its own commit stamps. Exact-string equality is
still asserted for everything not in AMO_UNPADDED, so the exception cannot
quietly spread.
Scribe #3138.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
237 lines
11 KiB
Python
237 lines
11 KiB
Python
"""The two values `artifacts.sh` derives, and what each of them promises.
|
|
|
|
`revision` decides whether a build gets skipped; `version` is what an instance
|
|
reports about itself and what a release tag is named after. Neither has a
|
|
consumer that would notice it going subtly wrong.
|
|
|
|
## revision
|
|
|
|
Milestone 318 step 3: each image carries its revision as an `fc.revision`
|
|
label, and build.yml reads that label back off the moving channel tag. Equal
|
|
to the derived revision means the bytes this push would produce are already
|
|
published, so the build is skipped.
|
|
|
|
That makes the revision load-bearing in a way a version string is not — it is
|
|
compared for equality against a value stamped into a real published artifact.
|
|
Both ways of getting it wrong are silent:
|
|
|
|
* **it does not identify the content** — a revision that moves when the source
|
|
did not (a HEAD-derived value, say) never matches, nothing is ever skipped,
|
|
and the mechanism quietly buys nothing while every lane stays green.
|
|
* **it identifies the wrong content** — a revision that holds still when the
|
|
source DID change matches a stale label, the build is skipped, and the
|
|
channel serves bytes that do not correspond to the commit. This is the
|
|
dangerous direction, and it is what `test_artifact_paths.py` guards from the
|
|
other side by pinning the path sets.
|
|
|
|
This module owns the narrower claim: whatever the path sets say, the revision
|
|
is genuinely the commit those paths last changed in.
|
|
|
|
## version
|
|
|
|
`YYYY.MM.DD.HHMM`, zero-padded, UTC — one shape across the family (note #3127
|
|
§1, rule 148), so the string this project emits is the same string its siblings
|
|
emit. Two nearly-identical formats are more dangerous than two obviously
|
|
different ones, and the only thing keeping them identical is a test.
|
|
|
|
**The extension is the one exception, and it is a rendering exception only.**
|
|
AMO's version grammar forbids a leading zero, so the extension emits the same
|
|
numbers unpadded — `2026.8.29.201` where the family says `2026.08.29.0201`
|
|
(#3138, milestone 318 step 8). Rule 148 defines comparison as numeric per
|
|
dot-segment, under which the two are equal, so this is pinned in both
|
|
directions below: the extension must satisfy AMO's grammar, and every artifact
|
|
must derive the same NUMBERS its own commit stamps. An exception left as "the
|
|
extension is different" would drift into being differently different.
|
|
|
|
The identity-TAG tests this file used to hold are gone with the tag. There is
|
|
no longer a `CHANNELLED` list to drift (the channel is which tag you inspect),
|
|
and no `identity` subcommand to refuse an unqualified call.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
import re
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
ROOT = Path(__file__).resolve().parent.parent
|
|
|
|
ARTIFACTS = ("web", "ml", "agent", "extension")
|
|
|
|
# 12 hex chars — the prefix build.yml stamps and compares.
|
|
_REVISION = re.compile(r"^[0-9a-f]{12}$")
|
|
|
|
# YYYY.MM.DD.HHMM, every segment zero-padded to its full width.
|
|
_VERSION = re.compile(r"^\d{4}\.\d{2}\.\d{2}\.\d{4}$")
|
|
|
|
# The artifacts that cannot use the padded rendering. Exactly one, and the
|
|
# reason is external: `packaging.sh` derives the extension's version and AMO
|
|
# refuses to sign a padded one.
|
|
AMO_UNPADDED = frozenset({"extension"})
|
|
|
|
# Mozilla's published grammar for addons.mozilla.org, transcribed from MDN's
|
|
# manifest.json/version page. A segment is the single digit `0` or starts 1-9,
|
|
# and there are at most four. This is the constraint the exception exists for,
|
|
# so it is what the exception is tested against — `2026.08.29.0201` fails it.
|
|
_AMO = re.compile(r"^(0|[1-9][0-9]{0,8})(\.(0|[1-9][0-9]{0,8})){0,3}$")
|
|
|
|
# YYYY.M.D.HHMM — four segments, none of them zero-padded.
|
|
_UNPADDED = re.compile(r"^\d{4}(\.(0|[1-9]\d*)){3}$")
|
|
|
|
|
|
def segments(value: str) -> tuple[int, ...]:
|
|
"""A version as the numbers it denotes, which is how rule 148 says to
|
|
compare one. `2026.08.29.0201` and `2026.8.29.201` are one value here."""
|
|
return tuple(int(part) for part in value.split("."))
|
|
|
|
|
|
# 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"), *args],
|
|
capture_output=True, text=True, check=True, cwd=ROOT,
|
|
).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.
|
|
|
|
Computed from git rather than asked of the script, so it fails if the
|
|
derivation ever stops meaning what it says — switching to HEAD, to a build
|
|
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.
|
|
"""
|
|
expected = newest_by_commit_time(artifact)
|
|
got = revision(artifact)
|
|
assert expected.startswith(got), (
|
|
f"{artifact} derives {got!r}, but the newest commit touching its "
|
|
f"shipped files is {expected[:12]!r}. The label stamped into the image "
|
|
f"would not identify its own content."
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize("artifact", ARTIFACTS)
|
|
def test_revision_is_a_legal_label_value_and_is_stable(artifact):
|
|
"""It is stamped as a docker label and compared for string equality, so a
|
|
stray newline or a varying value breaks the comparison rather than the
|
|
build — the mechanism would simply stop hitting, silently."""
|
|
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"
|
|
|
|
|
|
@pytest.mark.parametrize("artifact", sorted(set(ARTIFACTS) - AMO_UNPADDED))
|
|
def test_version_is_zero_padded_calver(artifact):
|
|
"""The family shape, pinned.
|
|
|
|
Padding was stripped until 2026-08-28 on the reasoning that each segment
|
|
should read as a plain integer — which never held, since comparison strips
|
|
leading zeros on parse anyway. What it did do was make this project emit
|
|
`2026.8.28.1432` while a sibling emitted `2026.08.28.1432`: two shapes one
|
|
character apart, which is the hard kind of difference to notice.
|
|
|
|
Also catches the midnight case. A `%H%M` of `0322` must survive as `0322`;
|
|
the old strip-leading-zeros helper turned it into `322`, silently changing
|
|
a four-digit field into three.
|
|
"""
|
|
value = artifacts("version", artifact).strip()
|
|
assert _VERSION.match(value), (
|
|
f"{artifact} derives {value!r}, which is not zero-padded "
|
|
f"YYYY.MM.DD.HHMM. Note #3127 §1 and rule 148 both specify the padded "
|
|
f"form, and a release tag is this string with a `v` in front."
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize("artifact", sorted(AMO_UNPADDED))
|
|
def test_the_unpadded_artifacts_derive_something_amo_will_sign(artifact):
|
|
"""The other half of the family shape: the documented exception, tested
|
|
against the constraint that justifies it rather than against itself.
|
|
|
|
A padded value passes `_UNPADDED` on any date with no leading zeros, so
|
|
that pattern alone would let a regression sit unnoticed until the first
|
|
single-digit month — at which point the failure is a burned AMO version,
|
|
not a red lane. AMO's grammar is the assertion that fires immediately.
|
|
"""
|
|
value = artifacts("version", artifact).strip()
|
|
assert _AMO.match(value), (
|
|
f"{artifact} derives {value!r}, which AMO refuses: a segment must be "
|
|
f"the single digit `0` or start 1-9, and there are at most four. "
|
|
f"Almost certainly a zero-padded segment — the family pads and this "
|
|
f"artifact must not (#3138). AMO 409s on re-signing, so a version it "
|
|
f"rejects is burned."
|
|
)
|
|
assert _UNPADDED.match(value), (
|
|
f"{artifact} derives {value!r}, which is not YYYY.M.D.HHMM. AMO would "
|
|
f"also accept the pre-318 `1.0.<minutes>`, and that orders below every "
|
|
f"ext-2026.* release already signed."
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize("artifact", ARTIFACTS)
|
|
def test_version_and_revision_describe_the_same_commit(artifact):
|
|
"""They are derived independently and must not be able to disagree.
|
|
|
|
A build reports the version and skips on the revision, so a divergence
|
|
would mean an instance naming one commit while carrying another's bytes —
|
|
unfalsifiable from outside, since both values look perfectly well-formed.
|
|
"""
|
|
sha = newest_by_commit_time(artifact)
|
|
stamped = subprocess.run(
|
|
["git", "show", "-s", "--format=%cd", "--date=format-local:%Y.%m.%d.%H%M", sha],
|
|
capture_output=True, text=True, check=True, cwd=ROOT,
|
|
env={"TZ": "UTC", "PATH": os.environ.get("PATH", "")},
|
|
).stdout.strip()
|
|
derived = artifacts("version", artifact).strip()
|
|
|
|
# Compared as NUMBERS, which is how rule 148 defines comparison and the
|
|
# only way one assertion can cover both renderings. This is what makes the
|
|
# extension's exception cosmetic rather than semantic: it must denote
|
|
# exactly the value its own commit stamps, whatever the padding.
|
|
assert segments(derived) == segments(stamped), (
|
|
f"{artifact} derives {derived!r}, but its newest shipped commit "
|
|
f"{sha[:12]} is {stamped!r}. The instance would name one commit while "
|
|
f"carrying another's bytes."
|
|
)
|
|
if artifact not in AMO_UNPADDED:
|
|
assert derived == stamped, (
|
|
f"{artifact} derives {derived!r} where the family shape is "
|
|
f"{stamped!r} — same numbers, wrong rendering. Only the artifacts "
|
|
f"in AMO_UNPADDED may differ here."
|
|
)
|
|
assert sha.startswith(revision(artifact))
|