diff --git a/packaging/guard-forward.sh b/packaging/guard-forward.sh index 70cfae7..7d60069 100755 --- a/packaging/guard-forward.sh +++ b/packaging/guard-forward.sh @@ -79,19 +79,35 @@ fetch() { # What the channel is serving, per artifact. ONE definition of where to look, shared # with `should-build.sh` — the skip decision and the guard must agree about what is # published, and two readers of one fact is how this repo keeps producing #2181-2183. +# +# EVERY LOOKUP HERE MUST SUCCEED EVEN WHEN IT FINDS NOTHING. That is what the `|| true` +# on each pipeline is for, and it is load-bearing rather than defensive noise. +# +# An empty channel is a REAL state this guard is written to pass — `[ -z "$published" ]` +# further down says so in as many words. But the value is captured as +# `published="$(published_for ...)"`, and under `set -e` a command substitution that +# exits non-zero kills the script before that branch is ever reached. Silently, too: +# everything the pipeline would have said went into the capture rather than the log. +# +# WHICH COMMAND THE PIPELINE HAPPENS TO END ON decides whether that fires, which is the +# part worth remembering. `sed` on empty input exits 0; `grep` exits 1. Three of these +# four lookups end in `sed` and were fine. The one that ends in `grep -oE '[0-9]+$'` — +# Android's version_code — was not, and it failed the whole Android lane on the first +# merge to `main` (run 4857): exit 1, no output, 0.16 seconds, on the one channel that +# had no APK published yet. Its three neighbours hid it until then. published_for() { case "$1" in desktop) # What the UPDATER reads. The manifest is the thing that decides whether a # client is offered a build, so it is the authority on what is published. - fetch "$SERVER/$REPO/releases/download/$2/latest.json" \ + { fetch "$SERVER/$REPO/releases/download/$2/latest.json" \ | grep -oE '"version"[[:space:]]*:[[:space:]]*"[^"]+"' | head -1 \ - | sed -E 's/.*"([^"]+)"$/\1/' + | sed -E 's/.*"([^"]+)"$/\1/'; } || true ;; android) - fetch "$SERVER/$REPO/releases/download/$2/thoughtsync-android.json" \ + { fetch "$SERVER/$REPO/releases/download/$2/thoughtsync-android.json" \ | grep -oE '"version_code"[[:space:]]*:[[:space:]]*[0-9]+' | head -1 \ - | grep -oE '[0-9]+$' + | grep -oE '[0-9]+$'; } || true ;; esac } @@ -104,9 +120,9 @@ published_name() { case "$1" in desktop) published_for desktop "$2" ;; android) - fetch "$SERVER/$REPO/releases/download/$2/thoughtsync-android.json" \ + { fetch "$SERVER/$REPO/releases/download/$2/thoughtsync-android.json" \ | grep -oE '"version_name"[[:space:]]*:[[:space:]]*"[^"]+"' | head -1 \ - | sed -E 's/.*"([^"]+)"$/\1/' + | sed -E 's/.*"([^"]+)"$/\1/'; } || true ;; esac } diff --git a/tests/test_versioning.py b/tests/test_versioning.py index ab7d404..2dd2d59 100644 --- a/tests/test_versioning.py +++ b/tests/test_versioning.py @@ -24,6 +24,7 @@ import pytest SCRIPT = Path(__file__).resolve().parent.parent / "packaging" / "version.sh" GUARD = Path(__file__).resolve().parent.parent / "packaging" / "guard-forward.sh" +SHOULD_BUILD = Path(__file__).resolve().parent.parent / "packaging" / "should-build.sh" # 2020-01-01T00:00:00Z, the counter epoch. Duplicated from the script deliberately: # a test that imported the value could not catch the value being changed, and moving @@ -304,3 +305,113 @@ def test_the_guard_rejects_bad_invocations(args: list[str]) -> None: that answers confidently for input it does not understand is worse than none.""" r = subprocess.run(["sh", str(GUARD), *args], capture_output=True, text=True) assert r.returncode != 0 + + +# --- the backwards guard against a channel ------------------------------------ +# +# The half of the guard that talks to a release feed, which the `compare` tests above +# deliberately do not reach. Hermetic anyway: `curl` is shadowed on PATH by a stub, so +# these assert what the guard does with an answer rather than what the forge returns. +# +# WHY THIS SECTION EXISTS AT ALL. The empty-channel case — a channel that has never +# published this artifact — is one the guard is written to PASS, and it says so in a +# branch of its own. It did not: `published="$(published_for ...)"` under `set -e` +# died on the substitution before that branch could run, because Android's lookup +# ended in a `grep` that exits 1 on no match while its three siblings ended in a `sed` +# that exits 0. Silent, because everything the pipeline printed went into the capture. +# It took the Android lane down on the first merge to `main` (run 4857) and nothing +# here would have caught it. + + +@pytest.fixture +def curl_stub(tmp_path: Path): + """A `curl` on PATH that serves whatever the test says, or 404s. + + Returns a callable: `serve(None)` for a channel with nothing published (the stub + exits 22, as real curl does under `-f` on an HTTP error), `serve(body)` to hand + back a feed. + """ + bindir = tmp_path / "stubbin" + bindir.mkdir() + body = bindir / "body" + stub = bindir / "curl" + stub.write_text( + "#!/bin/sh\n" + f'[ -f "{body}" ] || exit 22\n' + f'cat "{body}"\n' + ) + stub.chmod(0o755) + + def serve(content: str | None) -> dict[str, str]: + if content is None: + body.unlink(missing_ok=True) + else: + body.write_text(content) + return {**os.environ, "PATH": f"{bindir}{os.pathsep}{os.environ['PATH']}"} + + return serve + + +def guard(repo: Path, env: dict[str, str], *args: str) -> subprocess.CompletedProcess: + return subprocess.run(["sh", str(GUARD), *args], + cwd=repo, env=env, capture_output=True, text=True) + + +@pytest.mark.parametrize("artifact", ["desktop", "android"]) +def test_an_empty_channel_passes_the_guard(repo: Path, curl_stub, artifact: str) -> None: + """Nothing published yet is not a backwards move — it is the first publish. + + Failing here would block the very first build to reach a channel, which is exactly + what happened to Android on `stable`. + """ + r = guard(repo, curl_stub(None), artifact, "stable") + assert r.returncode == 0, f"exit={r.returncode} stdout={r.stdout!r} stderr={r.stderr!r}" + assert "nothing to compare" in r.stdout + + +@pytest.mark.parametrize("artifact", ["desktop", "android"]) +def test_an_empty_channel_reports_no_published_version( + repo: Path, curl_stub, artifact: str +) -> None: + """`published` answers empty rather than failing — `should-build.sh` captures it + the same way the guard does, so a non-zero exit strands that caller too.""" + r = guard(repo, curl_stub(None), "published", artifact, "stable") + assert r.returncode == 0, f"exit={r.returncode} stderr={r.stderr!r}" + assert r.stdout.strip() == "" + + +def test_an_empty_channel_builds(repo: Path, curl_stub) -> None: + r = subprocess.run(["sh", str(SHOULD_BUILD), "android", "stable"], + cwd=repo, env=curl_stub(None), capture_output=True, text=True) + assert r.returncode == 0, f"exit={r.returncode} stderr={r.stderr!r}" + assert r.stdout.strip() == "true" + + +def test_a_lower_published_version_passes(repo: Path, curl_stub) -> None: + """The transition this milestone performs: `stable` served the bare 0.2.0 from + Cargo.toml, and the new key has to clear it.""" + env = curl_stub('{"version": "0.2.0", "platforms": {}}') + r = guard(repo, env, "desktop", "stable") + assert r.returncode == 0, f"stderr={r.stderr!r}" + assert "may be published" in r.stdout + + +def test_a_higher_published_version_fails_the_lane(repo: Path, curl_stub) -> None: + """The unrecoverable direction. A version below what is published leaves every + installed client reporting 'up to date' forever, so this fails rather than warns.""" + env = curl_stub('{"version": "9.9.9", "platforms": {}}') + r = guard(repo, env, "desktop", "stable") + assert r.returncode != 0 + assert "GUARD FAILED" in r.stderr + + +def test_an_equal_android_code_fails_because_android_will_not_install_it( + repo: Path, curl_stub +) -> None: + """Android hard-fails a non-rising versionCode, so equality is refused there — + unlike the desktop, where an unchanged source deriving its own value is ordinary.""" + code = version(repo, "key", "android") + env = curl_stub(f'{{"version_code": {code}, "version_name": "x"}}') + r = guard(repo, env, "android", "stable") + assert r.returncode != 0 + assert "EQUALS" in r.stderr