From 460cdaef1179bef17a28c4c3f4126e50e8e21a2e Mon Sep 17 00:00:00 2001 From: Shane B Date: Sun, 9 Aug 2026 21:30:09 +0200 Subject: [PATCH] Lock the Android tag to versionName, the way the exporter's is The exporter cannot be released under a tag that disagrees with its source - the check runs before either binary builds. The Android app had no equivalent, so an android-app-v* tag could claim any version while the APK reported another. That asymmetry existed only because the exporter was the release being cut when the check was written. exporter_version.py is now release_version.py with a --kind, rather than a second copy of the same idea. Both releases work the same way: the version lives in the source, the tag is derived from it, and a disagreement fails the release with instructions naming the file to edit. exporter -> VERSION in python/main/wizard.py android -> versionName in app/build.gradle.kts build-release.yml now runs that check instead of parsing the tag by hand with awk and cut, and takes the version from the source. It runs before the Gradle build, so a mismatch costs seconds rather than a full signed build. The failure messages differ per kind because the reasons differ: the exporter's VERSION is stamped into every export as `via:`, while versionName is what the app reports in Settings and in bug reports. Both are baked in at build time by something no later step rewrites, which is why the tag alone cannot be trusted. Tests cover the Android side including the trap that makes a naive regex wrong - versionNameSuffix = "-debug" sits four lines below versionName - and assert the two tag prefixes cannot overlap, since both releases live in one repository and a shared prefix would let either check the other. CONTRIBUTING gains a Releasing the Android app section, which the new error message points at. It says to bump versionCode as well: Android refuses an APK whose code is not higher than the installed one, so a versionName-only bump leaves existing users unable to update, and it fails on their phone rather than in any build. Co-Authored-By: Claude Opus 5 --- .github/workflows/build-debug.yml | 2 +- .github/workflows/build-release.yml | 39 ++-- .github/workflows/macos-exporter-python.yml | 4 +- .github/workflows/macos-scripts-python.yml | 2 +- AGENTS.md | 2 +- CONTRIBUTING.md | 47 +++- python/README.md | 6 +- python/main/wizard.py | 2 +- ...exporter_version.py => release_version.py} | 141 +++++++++--- scripts/test/test_exporter_version.py | 139 ----------- scripts/test/test_release_version.py | 215 ++++++++++++++++++ 11 files changed, 393 insertions(+), 206 deletions(-) rename scripts/{exporter_version.py => release_version.py} (51%) delete mode 100644 scripts/test/test_exporter_version.py create mode 100644 scripts/test/test_release_version.py diff --git a/.github/workflows/build-debug.yml b/.github/workflows/build-debug.yml index 4194d906..31d57df4 100644 --- a/.github/workflows/build-debug.yml +++ b/.github/workflows/build-debug.yml @@ -63,7 +63,7 @@ jobs: run: python scripts/add_strings.py --check # These scripts gate other checks - add_strings.py gates the step above, and - # exporter_version.py gates the exporter release - so a bug in one of them reports + # release_version.py gates the exporter and Android releases - so a bug in one of them reports # green while protecting nothing. - name: Test the tooling in scripts/ run: | diff --git a/.github/workflows/build-release.yml b/.github/workflows/build-release.yml index c6cd849f..e7a7204f 100644 --- a/.github/workflows/build-release.yml +++ b/.github/workflows/build-release.yml @@ -23,32 +23,25 @@ jobs: && startsWith(github.event.release.tag_name, 'android-app-v') steps: - - name: Extract Version from Tag - id: extract_version - run: | - # GITHUB_REF for tags is like 'refs/tags/android-app-v1.0.3.1' - FULL_TAG_NAME="${{ github.ref }}" - - # Remove 'refs/tags/' prefix to get 'android-app-v1.0.3.1' - TAG_WITHOUT_PREFIX="${FULL_TAG_NAME##refs/tags/}" - - # Find the index of '-v' - # This will be 'android-app-' part, then we add 2 to skip '-v' - START_INDEX=$(( $(echo "$TAG_WITHOUT_PREFIX" | awk -F'-v' '{print length($1)}') + 2 )) - - # Extract everything from '-v' onwards - # Using cut or substring if available, or just awk - VERSION_STRING=$(echo "$TAG_WITHOUT_PREFIX" | cut -c "$START_INDEX"-) - - # Output the extracted version as a step output - echo "Extracted version: $VERSION_STRING" - - # !! this is reused below - echo "APP_VERSION=$VERSION_STRING" >> $GITHUB_OUTPUT - - name: Checkout code uses: actions/checkout@v4 + # Nothing rewrites versionName from the tag: it is what the app reports about itself in + # Settings and in bug reports, and it is baked into the APK. So a tag that disagrees + # with app/build.gradle.kts publishes an APK calling itself the old version, under a + # release page claiming the new one, and nothing anywhere says so. + # + # This replaces parsing the tag by hand. The version now comes from the source and the + # tag only has to agree with it, which is the same arrangement the macOS exporter has + # used since scripts/release_version.py was introduced. It runs before the Gradle build, + # so a mismatch costs seconds rather than a full signed build. + - name: Check the release tag matches the version in the source + id: extract_version + run: | + APP_VERSION="$(python3 scripts/release_version.py --kind android --tag "${{ github.ref }}")" + echo "Tag and source agree on version: $APP_VERSION" + echo "APP_VERSION=$APP_VERSION" >> $GITHUB_OUTPUT + # Set Current Date As Env Variable - name: Set current date as env variable run: echo "date_today=$(date +'%Y-%m-%d')" >> $GITHUB_ENV diff --git a/.github/workflows/macos-exporter-python.yml b/.github/workflows/macos-exporter-python.yml index c799c80b..cb76e345 100644 --- a/.github/workflows/macos-exporter-python.yml +++ b/.github/workflows/macos-exporter-python.yml @@ -39,11 +39,11 @@ jobs: # a mismatch costs one minute rather than two full PyInstaller builds. # # Deliberately not the reverse - patching the tag into the source - because the wizard - # also runs from source, and those exports stamp `via:` too. See scripts/exporter_version.py. + # also runs from source, and those exports stamp `via:` too. See scripts/release_version.py. - name: Check the release tag matches the version in the source id: version run: | - APP_VERSION="$(python3 scripts/exporter_version.py --tag "${{ github.ref }}")" + APP_VERSION="$(python3 scripts/release_version.py --kind exporter --tag "${{ github.ref }}")" echo "Tag and source agree on version: $APP_VERSION" echo "APP_VERSION=$APP_VERSION" >> $GITHUB_OUTPUT diff --git a/.github/workflows/macos-scripts-python.yml b/.github/workflows/macos-scripts-python.yml index 36589d3e..6a2e028c 100644 --- a/.github/workflows/macos-scripts-python.yml +++ b/.github/workflows/macos-scripts-python.yml @@ -66,4 +66,4 @@ jobs: - name: Check the exporter version is still readable working-directory: . run: | - python scripts/exporter_version.py --print + python scripts/release_version.py --kind exporter --print diff --git a/AGENTS.md b/AGENTS.md index b69f25ab..b694ba8b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -109,7 +109,7 @@ So releasing is two steps, in this order: 1. Commit the `VERSION` bump to `main` 2. Tag that commit `macos-exporter-v` and publish the release -`scripts/exporter_version.py --tag ` enforces it, and runs in `test-release-version` +`scripts/release_version.py --kind exporter --tag ` enforces it, and runs in `test-release-version` before either build job. A tag that disagrees fails the release rather than shipping a build that lies about itself. Full procedure: [CONTRIBUTING.md](./CONTRIBUTING.md#releasing-the-macos-exporter). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 84fcb61a..bf0a7b31 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -457,7 +457,7 @@ The tag must be `macos-exporter-v` followed by exactly what `VERSION` says. Befo build job starts, `test-release-version` runs: ```bash -python scripts/exporter_version.py --tag macos-exporter-v1.0.6 +python scripts/release_version.py --kind exporter --tag macos-exporter-v1.0.6 ``` which fails the release, with instructions, if the two disagree — so the mistake costs a @@ -468,14 +468,55 @@ name, the release title, and the version the app reports cannot come apart. You can run the same check locally before tagging: ```bash -python scripts/exporter_version.py --print # what the source declares -python scripts/exporter_version.py --tag macos-exporter-v1.0.6 # would this tag be accepted? +python scripts/release_version.py --kind exporter --print # what the source declares +python scripts/release_version.py --kind exporter --tag macos-exporter-v1.0.6 # would this tag be accepted? ``` If you tagged before bumping, the fix is to push the bump, delete the release and its tag, and re-tag the new commit. Releasing the Android app is unrelated and unaffected — its version lives in `app/build.gradle.kts`. +## Releasing the Android app + +Same shape as the exporter, with the version in a different file — `versionName` in +`app/build.gradle.kts`: + +```kotlin +versionCode = 3 +versionName = "1.0.5" +``` + +**Bump `versionCode` too.** Android refuses to install an APK whose code is not higher than +the installed one, so a `versionName`-only bump leaves existing users unable to update — and +it fails on their phone, never in any build. + +```bash +# 1. Bump both, commit, push +git commit -am "Bump the Android app to 1.0.6" +git push origin main + +# 2. Draft the release, then publish it from the GitHub UI +python scripts/release_notes.py draft --kind android --changes-file notes.md + +# 3. Collapse the release it replaced +python scripts/release_notes.py demote --kind android +``` + +Publishing runs `build-release.yml`, which checks the tag against the source before building +anything: + +```bash +python scripts/release_version.py --kind android --tag android-app-v1.0.6 +``` + +then runs the tests, builds and signs the APK, and attaches it to the release. The tag must be +`android-app-v` followed by exactly what `versionName` says; the check fails the release, with +instructions, if they disagree. + +Signing needs `SIGNING_KEY`, `KEY_STORE_PASSWORD`, `KEY_PASSWORD` and the `ALIAS` in the +**Android Build Release** environment. Those are separate from the exporter's token, and a +release is a bad time to discover one has expired. + ## Opening a pull request - Run `./gradlew testAll` and `python scripts/add_strings.py --check` first diff --git a/python/README.md b/python/README.md index ce8318bc..4e82e125 100644 --- a/python/README.md +++ b/python/README.md @@ -152,7 +152,7 @@ Zip up result (MacOS): ```shell # Read from the source rather than typed, so the zip cannot end up named after a version the # app does not report. See "Versioning" below. -APP_VERSION="$(python ../scripts/exporter_version.py --print)" +APP_VERSION="$(python ../scripts/release_version.py --kind exporter --print)" cd ./dist zip -r OpenTagViewer-ExportWizardMacOS-$APP_VERSION.zip OpenTagViewer.app/ OpenTagViewer ``` @@ -169,8 +169,8 @@ Releases are tagged `macos-exporter-v`, and CI refuses to publish one w disagrees with the source: ```shell -python ../scripts/exporter_version.py --print # what the source declares -python ../scripts/exporter_version.py --tag macos-exporter-v1.0.6 # would this tag be accepted? +python ../scripts/release_version.py --kind exporter --print # what the source declares +python ../scripts/release_version.py --kind exporter --tag macos-exporter-v1.0.6 # would this tag be accepted? ``` Not to be confused with `EXPORT_METADATA_VERSION`, a few lines below it: that is the version diff --git a/python/main/wizard.py b/python/main/wizard.py index 2546e6ae..72b0c86f 100644 --- a/python/main/wizard.py +++ b/python/main/wizard.py @@ -34,7 +34,7 @@ # The single source of truth for the exporter's version: it appears in the window title and # is stamped into every export as `via: OpenTagViewer.app:`. Releases are tagged # macos-exporter-v, and CI refuses to publish a release whose tag disagrees - see -# scripts/exporter_version.py and CONTRIBUTING.md -> Releasing the macOS exporter. +# scripts/release_version.py and CONTRIBUTING.md -> Releasing the macOS exporter. VERSION = "1.0.5" APP_TITLE = f"OpenTagViewer AirTag Exporter {VERSION}" diff --git a/scripts/exporter_version.py b/scripts/release_version.py similarity index 51% rename from scripts/exporter_version.py rename to scripts/release_version.py index 6d5606c6..27d103e0 100644 --- a/scripts/exporter_version.py +++ b/scripts/release_version.py @@ -1,4 +1,9 @@ -"""Read the macOS exporter's version out of the source, and check a release tag against it. +"""Read a release's version out of the source, and check its tag against it. + +Covers both things this repository releases: the macOS exporter, whose version is `VERSION` in +`python/main/wizard.py`, and the Android app, whose version is `versionName` in +`app/build.gradle.kts`. Neither is rewritten at build time, so in both cases the tag is a claim +about the source that nothing verifies unless something like this does. `VERSION` in `python/main/wizard.py` is the single source of truth for the exporter. It is shown in the window title and, more importantly, stamped into every export it produces as @@ -21,20 +26,25 @@ tkinter and yaml, neither of which is wanted on a lint runner, and one of which cannot even open a display in CI. +The Android app has the same problem for the same reason: `versionName` is what the app +reports about itself and what is baked into the APK, and the release workflow only parses the +tag to name the artifact. An `android-app-v*` tag could disagree with it indefinitely. + Usage ----- Print the version the source declares: - python scripts/exporter_version.py --print + python scripts/release_version.py --kind exporter --print + python scripts/release_version.py --kind android --print Check a release tag against it. Accepts a bare tag or a full ref, so `$GITHUB_REF` works: - python scripts/exporter_version.py --tag macos-exporter-v1.0.5 - python scripts/exporter_version.py --tag refs/tags/macos-exporter-v1.0.5 + python scripts/release_version.py --kind exporter --tag macos-exporter-v1.0.5 + python scripts/release_version.py --kind android --tag refs/tags/android-app-v1.0.5 On success it prints the version, so a workflow can use it directly: - APP_VERSION="$(python scripts/exporter_version.py --tag "$GITHUB_REF")" + APP_VERSION="$(python scripts/release_version.py --kind android --tag "$GITHUB_REF")" """ from __future__ import annotations @@ -47,17 +57,17 @@ REPO_ROOT = Path(__file__).resolve().parent.parent WIZARD_PATH = REPO_ROOT / "python" / "main" / "wizard.py" +GRADLE_PATH = REPO_ROOT / "app" / "build.gradle.kts" VERSION_CONSTANT = "VERSION" -TAG_PREFIX = "macos-exporter-v" REF_PREFIX = "refs/tags/" +GRADLE_VERSION_NAME = re.compile(r'^\s*versionName\s*=\s*"([^"]+)"', re.MULTILINE) + # Deliberately loose - the project has shipped three- and four-part versions (1.0.3.1) - but # strict enough to catch a tag like 'macos-exporter-vlatest' reaching the artifact name. VERSION_PATTERN = re.compile(r"^\d+(?:\.\d+)*$") -RELEASE_DOCS = "CONTRIBUTING.md -> Releasing the macOS exporter" - class VersionError(Exception): """Something is wrong with the tag or the source version. The message is the report.""" @@ -99,22 +109,78 @@ def read_version(path: Path | None = None) -> tuple[str, int]: raise VersionError( f"No module-level {VERSION_CONSTANT} = \"...\" found in {path}.\n" - f"The release check reads it from there; see {RELEASE_DOCS}." + f"The release check reads it from there; see {KINDS['exporter']['docs']}." ) -def version_from_tag(tag: str) -> str: +def read_gradle_version(path: Path | None = None) -> tuple[str, int]: + """Return `versionName` from the Gradle build, with the line it is declared on.""" + path = path or GRADLE_PATH + try: + source = path.read_text(encoding="utf-8") + except OSError as error: + raise VersionError(f"Could not read {path}: {error}") + + match = GRADLE_VERSION_NAME.search(source) + if not match: + raise VersionError( + f"No versionName = \"...\" found in {_display(path)}.\n" + f"The release check reads it from there." + ) + + lineno = source[:match.start()].count("\n") + 1 + return match.group(1), lineno + + +# Each releasable thing, and where its version actually lives. The tag is derived from the +# source rather than typed, so the two cannot drift. +KINDS = { + "exporter": { + "prefix": "macos-exporter-v", + "path": WIZARD_PATH, + "read": read_version, + "field": 'VERSION = "..."', + "docs": "CONTRIBUTING.md -> Releasing the macOS exporter", + "why": ( + "Its VERSION is shown in the window title and stamped into every export as\n" + "`via: OpenTagViewer.app:`, including exports made by running the wizard\n" + "from source - which no build step can rewrite." + ), + }, + "android": { + "prefix": "android-app-v", + "path": GRADLE_PATH, + "read": read_gradle_version, + "field": 'versionName = "..."', + "docs": "CONTRIBUTING.md -> Releasing the Android app", + "why": ( + "versionName is what the app reports about itself in Settings and in bug reports,\n" + "and it is baked into the APK - no build step rewrites it from the tag." + ), + }, +} + + +def _kind(name: str) -> dict: + if name not in KINDS: + raise VersionError(f"Unknown release kind '{name}'. Expected one of: {', '.join(KINDS)}") + return KINDS[name] + + +def version_from_tag(tag: str, kind: str = "exporter") -> str: """Return the version encoded in a release tag, accepting a bare tag or a full git ref.""" + spec = _kind(kind) + prefix = spec["prefix"] name = tag[len(REF_PREFIX):] if tag.startswith(REF_PREFIX) else tag - if not name.startswith(TAG_PREFIX): + if not name.startswith(prefix): raise VersionError( - f"'{name}' is not an exporter release tag.\n" - f"Exporter releases are tagged {TAG_PREFIX}, for example {TAG_PREFIX}1.0.5.\n" - f"See {RELEASE_DOCS}." + f"'{name}' is not a release tag for the {kind}.\n" + f"Those are tagged {prefix}, for example {prefix}1.0.5.\n" + f"See {spec['docs']}." ) - version = name[len(TAG_PREFIX):] + version = name[len(prefix):] if not VERSION_PATTERN.match(version): raise VersionError( f"'{version}' (from tag '{name}') is not a version number.\n" @@ -123,35 +189,39 @@ def version_from_tag(tag: str) -> str: return version -def check_tag(tag: str, path: Path | None = None) -> str: +def check_tag(tag: str, kind: str = "exporter", path: Path | None = None) -> str: """Verify a release tag matches the source version. Returns the agreed version.""" - path = path or WIZARD_PATH - tagged = version_from_tag(tag) - declared, lineno = read_version(path) + spec = _kind(kind) + prefix = spec["prefix"] + path = path or spec["path"] + + tagged = version_from_tag(tag, kind) + declared, lineno = spec["read"](path) relative = _display(path) + field = spec["field"].replace('"..."', f'"{tagged}"') if tagged != declared: raise VersionError( "Release tag and source version disagree.\n" "\n" - f" tag {TAG_PREFIX}{tagged} declares {tagged}\n" + f" tag {prefix}{tagged} declares {tagged}\n" f" {relative}:{lineno} declares {declared}\n" "\n" - f"{relative} is the single source of truth. Its VERSION is shown in the window title\n" - "and stamped into every export as `via: OpenTagViewer.app:`, including exports\n" - "made by running the wizard from source - which no build step can rewrite. Publishing\n" - f"this release would ship a build that calls itself {declared} under a {tagged} tag.\n" + f"{relative} is the single source of truth.\n" + f"{spec['why']}\n" + f"Publishing this release would ship a build that calls itself {declared} under a\n" + f"{tagged} tag.\n" "\n" "To fix:\n" "\n" - f" 1. edit {relative} -> VERSION = \"{tagged}\"\n" + f" 1. edit {relative} -> {field}\n" " 2. commit and push that to main\n" - f" 3. delete the release and its tag, then re-tag the new commit as {TAG_PREFIX}{tagged}\n" + f" 3. delete the release and its tag, then re-tag the new commit as {prefix}{tagged}\n" "\n" - f"Or keep the code as it is and release it as {TAG_PREFIX}{declared} instead.\n" + f"Or keep the code as it is and release it as {prefix}{declared} instead.\n" "\n" - "Releasing the exporter is two steps on purpose: the bump is a commit, and the tag only\n" - f"publishes it. That is what stops the two from drifting apart. See {RELEASE_DOCS}." + "Releasing is two steps on purpose: the bump is a commit, and the tag only publishes\n" + f"it. That is what stops the two from drifting apart. See {spec['docs']}." ) return declared @@ -159,7 +229,13 @@ def check_tag(tag: str, path: Path | None = None) -> str: def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser( - description="Read or verify the macOS exporter version declared in python/main/wizard.py.", + description="Read or verify the version a release tag claims, against the source.", + ) + parser.add_argument( + "--kind", + choices=sorted(KINDS), + default="exporter", + help="which release: the macOS exporter, or the Android app (default: exporter)", ) group = parser.add_mutually_exclusive_group(required=True) group.add_argument( @@ -171,12 +247,13 @@ def main(argv: list[str] | None = None) -> int: group.add_argument( "--tag", metavar="TAG", - help=f"check a release tag ({TAG_PREFIX}, or a full refs/tags/... ref) against the source", + help="check a release tag (, or a full refs/tags/... ref) against the source", ) args = parser.parse_args(argv) try: - version = read_version()[0] if args.print_version else check_tag(args.tag) + spec = _kind(args.kind) + version = spec["read"]()[0] if args.print_version else check_tag(args.tag, args.kind) except VersionError as error: print(f"\nERROR: {error}\n", file=sys.stderr) return 1 diff --git a/scripts/test/test_exporter_version.py b/scripts/test/test_exporter_version.py deleted file mode 100644 index bae3f63c..00000000 --- a/scripts/test/test_exporter_version.py +++ /dev/null @@ -1,139 +0,0 @@ -"""Tests for scripts/exporter_version.py. - -This one gates a release, and the way a gate fails matters: a check that wrongly passes -stops protecting anything while still reporting green. The failure it exists to catch - -tagging `macos-exporter-v1.0.5` against a tree that still says `1.0.4` - is invisible until -someone is holding a zip that lies about which exporter made it. - -The last test is the one that would have caught the original problem, and it reads the real -wizard rather than a fixture, so a rename or a refactor of that constant fails here instead -of at release time. -""" - -from __future__ import annotations - -import sys -from pathlib import Path - -import pytest - -sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) - -import exporter_version # noqa: E402 - - -def wizard(tmp_path: Path, body: str) -> Path: - path = tmp_path / "wizard.py" - path.write_text(body, encoding="utf-8") - return path - - -# --- reading the version out of the source ---------------------------------------------- - -def test_reads_the_version_and_the_line_it_is_on(tmp_path: Path): - path = wizard(tmp_path, 'import os\n\nVERSION = "1.0.5"\n') - assert exporter_version.read_version(path) == ("1.0.5", 3) - - -def test_reads_the_version_without_importing_the_module(tmp_path: Path): - """wizard.py imports tkinter and yaml; the check has to work on a runner with neither.""" - path = wizard(tmp_path, 'import definitely_not_installed_anywhere\n\nVERSION = "1.0.5"\n') - assert exporter_version.read_version(path)[0] == "1.0.5" - - -def test_ignores_a_version_that_is_not_module_level(tmp_path: Path): - path = wizard(tmp_path, 'def f():\n VERSION = "9.9.9"\n return VERSION\n') - with pytest.raises(exporter_version.VersionError, match="No module-level VERSION"): - exporter_version.read_version(path) - - -def test_rejects_a_version_that_is_not_a_plain_literal(tmp_path: Path): - """A computed VERSION cannot be read without importing, so it has to be refused loudly.""" - path = wizard(tmp_path, 'PARTS = (1, 0, 5)\nVERSION = ".".join(str(p) for p in PARTS)\n') - with pytest.raises(exporter_version.VersionError, match="must be a plain string literal"): - exporter_version.read_version(path) - - -def test_reports_a_missing_file_rather_than_raising_oserror(tmp_path: Path): - with pytest.raises(exporter_version.VersionError, match="Could not read"): - exporter_version.read_version(tmp_path / "nope.py") - - -# --- parsing the tag -------------------------------------------------------------------- - -@pytest.mark.parametrize("tag", [ - "macos-exporter-v1.0.5", - "refs/tags/macos-exporter-v1.0.5", -]) -def test_accepts_a_bare_tag_and_a_full_ref(tag: str): - assert exporter_version.version_from_tag(tag) == "1.0.5" - - -def test_accepts_a_four_part_version(): - """1.0.3.1 has shipped, so the pattern must not assume semver.""" - assert exporter_version.version_from_tag("macos-exporter-v1.0.3.1") == "1.0.3.1" - - -@pytest.mark.parametrize("tag", ["v1.0.5", "refs/tags/v1.0.5", "app-v1.0.5"]) -def test_rejects_a_tag_that_is_not_an_exporter_release(tag: str): - """The Android app is tagged in the same repo; its tags must not name an exporter build.""" - with pytest.raises(exporter_version.VersionError, match="not an exporter release tag"): - exporter_version.version_from_tag(tag) - - -@pytest.mark.parametrize("tag", ["macos-exporter-vlatest", "macos-exporter-v1.0.5-rc1", "macos-exporter-v"]) -def test_rejects_a_tag_whose_version_is_not_a_number(tag: str): - with pytest.raises(exporter_version.VersionError, match="not a version number"): - exporter_version.version_from_tag(tag) - - -# --- the check itself ------------------------------------------------------------------- - -def test_passes_when_the_tag_matches_the_source(tmp_path: Path): - path = wizard(tmp_path, 'VERSION = "1.0.5"\n') - assert exporter_version.check_tag("macos-exporter-v1.0.5", path) == "1.0.5" - - -def test_fails_when_the_tag_is_ahead_of_the_source(tmp_path: Path): - path = wizard(tmp_path, 'VERSION = "1.0.4"\n') - with pytest.raises(exporter_version.VersionError) as caught: - exporter_version.check_tag("macos-exporter-v1.0.5", path) - - message = str(caught.value) - # Both numbers have to appear, or the reader cannot tell which end is wrong. - assert "1.0.4" in message and "1.0.5" in message - assert "CONTRIBUTING.md" in message - - -def test_fails_when_the_source_is_ahead_of_the_tag(tmp_path: Path): - path = wizard(tmp_path, 'VERSION = "1.0.6"\n') - with pytest.raises(exporter_version.VersionError, match="disagree"): - exporter_version.check_tag("macos-exporter-v1.0.5", path) - - -# --- the command line ------------------------------------------------------------------- - -def test_print_writes_the_real_version_and_nothing_else(capsys: pytest.CaptureFixture): - """The workflow captures stdout into APP_VERSION, so stray output would end up in a filename.""" - assert exporter_version.main(["--print"]) == 0 - assert capsys.readouterr().out.strip() == exporter_version.read_version()[0] - - -def test_a_failing_check_exits_nonzero_and_reports_on_stderr(tmp_path: Path, capsys: pytest.CaptureFixture, - monkeypatch: pytest.MonkeyPatch): - monkeypatch.setattr(exporter_version, "WIZARD_PATH", wizard(tmp_path, 'VERSION = "1.0.4"\n')) - assert exporter_version.main(["--tag", "macos-exporter-v1.0.5"]) == 1 - - captured = capsys.readouterr() - assert "disagree" in captured.err - # Nothing on stdout, so `APP_VERSION="$(...)"` cannot silently capture an error message. - assert captured.out == "" - - -# --- the real wizard -------------------------------------------------------------------- - -def test_the_real_wizard_still_declares_a_readable_version(): - """Guards the assumption the whole check rests on: VERSION is a module-level literal.""" - version, lineno = exporter_version.read_version() - assert exporter_version.VERSION_PATTERN.match(version), f"{version!r} is not a version number" - assert lineno > 0 diff --git a/scripts/test/test_release_version.py b/scripts/test/test_release_version.py new file mode 100644 index 00000000..d14289bb --- /dev/null +++ b/scripts/test/test_release_version.py @@ -0,0 +1,215 @@ +"""Tests for scripts/release_version.py. + +This one gates a release, and the way a gate fails matters: a check that wrongly passes +stops protecting anything while still reporting green. The failure it exists to catch - +tagging `macos-exporter-v1.0.5` against a tree that still says `1.0.4` - is invisible until +someone is holding a zip that lies about which exporter made it. + +The last test is the one that would have caught the original problem, and it reads the real +wizard rather than a fixture, so a rename or a refactor of that constant fails here instead +of at release time. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import release_version # noqa: E402 + + +def wizard(tmp_path: Path, body: str) -> Path: + path = tmp_path / "wizard.py" + path.write_text(body, encoding="utf-8") + return path + + +# --- reading the version out of the source ---------------------------------------------- + +def test_reads_the_version_and_the_line_it_is_on(tmp_path: Path): + path = wizard(tmp_path, 'import os\n\nVERSION = "1.0.5"\n') + assert release_version.read_version(path) == ("1.0.5", 3) + + +def test_reads_the_version_without_importing_the_module(tmp_path: Path): + """wizard.py imports tkinter and yaml; the check has to work on a runner with neither.""" + path = wizard(tmp_path, 'import definitely_not_installed_anywhere\n\nVERSION = "1.0.5"\n') + assert release_version.read_version(path)[0] == "1.0.5" + + +def test_ignores_a_version_that_is_not_module_level(tmp_path: Path): + path = wizard(tmp_path, 'def f():\n VERSION = "9.9.9"\n return VERSION\n') + with pytest.raises(release_version.VersionError, match="No module-level VERSION"): + release_version.read_version(path) + + +def test_rejects_a_version_that_is_not_a_plain_literal(tmp_path: Path): + """A computed VERSION cannot be read without importing, so it has to be refused loudly.""" + path = wizard(tmp_path, 'PARTS = (1, 0, 5)\nVERSION = ".".join(str(p) for p in PARTS)\n') + with pytest.raises(release_version.VersionError, match="must be a plain string literal"): + release_version.read_version(path) + + +def test_reports_a_missing_file_rather_than_raising_oserror(tmp_path: Path): + with pytest.raises(release_version.VersionError, match="Could not read"): + release_version.read_version(tmp_path / "nope.py") + + +# --- parsing the tag -------------------------------------------------------------------- + +@pytest.mark.parametrize("tag", [ + "macos-exporter-v1.0.5", + "refs/tags/macos-exporter-v1.0.5", +]) +def test_accepts_a_bare_tag_and_a_full_ref(tag: str): + assert release_version.version_from_tag(tag) == "1.0.5" + + +def test_accepts_a_four_part_version(): + """1.0.3.1 has shipped, so the pattern must not assume semver.""" + assert release_version.version_from_tag("macos-exporter-v1.0.3.1") == "1.0.3.1" + + +@pytest.mark.parametrize("tag", ["v1.0.5", "refs/tags/v1.0.5", "app-v1.0.5", "android-app-v1.0.5"]) +def test_rejects_a_tag_that_is_not_an_exporter_release(tag: str): + """The Android app is tagged in the same repo; its tags must not name an exporter build.""" + with pytest.raises(release_version.VersionError, match="not a release tag for the exporter"): + release_version.version_from_tag(tag) + + +@pytest.mark.parametrize("tag", ["macos-exporter-vlatest", "macos-exporter-v1.0.5-rc1", "macos-exporter-v"]) +def test_rejects_a_tag_whose_version_is_not_a_number(tag: str): + with pytest.raises(release_version.VersionError, match="not a version number"): + release_version.version_from_tag(tag) + + +# --- the check itself ------------------------------------------------------------------- + +def test_passes_when_the_tag_matches_the_source(tmp_path: Path): + path = wizard(tmp_path, 'VERSION = "1.0.5"\n') + assert release_version.check_tag("macos-exporter-v1.0.5", path=path) == "1.0.5" + + +def test_fails_when_the_tag_is_ahead_of_the_source(tmp_path: Path): + path = wizard(tmp_path, 'VERSION = "1.0.4"\n') + with pytest.raises(release_version.VersionError) as caught: + release_version.check_tag("macos-exporter-v1.0.5", path=path) + + message = str(caught.value) + # Both numbers have to appear, or the reader cannot tell which end is wrong. + assert "1.0.4" in message and "1.0.5" in message + assert "CONTRIBUTING.md" in message + + +def test_fails_when_the_source_is_ahead_of_the_tag(tmp_path: Path): + path = wizard(tmp_path, 'VERSION = "1.0.6"\n') + with pytest.raises(release_version.VersionError, match="disagree"): + release_version.check_tag("macos-exporter-v1.0.5", path=path) + + +# --- the command line ------------------------------------------------------------------- + +def test_print_writes_the_real_version_and_nothing_else(capsys: pytest.CaptureFixture): + """The workflow captures stdout into APP_VERSION, so stray output would end up in a filename.""" + assert release_version.main(["--print"]) == 0 + assert capsys.readouterr().out.strip() == release_version.read_version()[0] + + +def test_a_failing_check_exits_nonzero_and_reports_on_stderr(tmp_path: Path, capsys: pytest.CaptureFixture, + monkeypatch: pytest.MonkeyPatch): + monkeypatch.setitem(release_version.KINDS["exporter"], "path", + wizard(tmp_path, 'VERSION = "1.0.4"\n')) + assert release_version.main(["--tag", "macos-exporter-v1.0.5"]) == 1 + + captured = capsys.readouterr() + assert "disagree" in captured.err + # Nothing on stdout, so `APP_VERSION="$(...)"` cannot silently capture an error message. + assert captured.out == "" + + +# --- the Android app --------------------------------------------------------------------- + +def gradle(tmp_path: Path, body: str) -> Path: + path = tmp_path / "build.gradle.kts" + path.write_text(body, encoding="utf-8") + return path + + +GRADLE_BLOCK = """\ +android { + defaultConfig { + applicationId = "dev.wander.android.opentagviewer" + versionCode = 3 + versionName = "1.0.5" + } + buildTypes { + debug { + versionNameSuffix = "-debug" + } + } +} +""" + + +def test_reads_the_android_version_and_its_line(tmp_path: Path): + assert release_version.read_gradle_version(gradle(tmp_path, GRADLE_BLOCK)) == ("1.0.5", 5) + + +def test_the_debug_suffix_is_not_mistaken_for_the_version(): + """`versionNameSuffix = "-debug"` sits a few lines below and must not match.""" + assert "-debug" not in release_version.read_gradle_version( + release_version.GRADLE_PATH)[0] + + +def test_an_android_tag_is_checked_against_the_gradle_build(tmp_path: Path): + path = gradle(tmp_path, GRADLE_BLOCK) + assert release_version.check_tag("android-app-v1.0.5", "android", path) == "1.0.5" + + +def test_an_android_tag_that_disagrees_is_refused(tmp_path: Path): + path = gradle(tmp_path, GRADLE_BLOCK) + + with pytest.raises(release_version.VersionError) as caught: + release_version.check_tag("android-app-v1.0.6", "android", path) + + message = str(caught.value) + assert "1.0.5" in message and "1.0.6" in message + # Has to name the file to edit, since it is a different one per kind. + assert "build.gradle.kts" in message + assert "versionName" in message + + +def test_an_exporter_tag_is_not_accepted_as_an_android_release(): + with pytest.raises(release_version.VersionError, match="not a release tag for the android"): + release_version.version_from_tag("macos-exporter-v1.0.5", "android") + + +def test_the_real_gradle_build_still_declares_a_readable_version(): + """Same guard as the wizard's: fail here rather than at release time.""" + version, lineno = release_version.read_gradle_version() + + assert release_version.VERSION_PATTERN.match(version), f"{version!r} is not a version number" + assert lineno > 0 + + +def test_both_kinds_have_a_distinct_tag_prefix(): + """Two releases share this repository; a shared prefix would let either check the other.""" + prefixes = [spec["prefix"] for spec in release_version.KINDS.values()] + + assert len(set(prefixes)) == len(prefixes) + for a in prefixes: + for b in prefixes: + assert a == b or not a.startswith(b), f"{a} and {b} overlap" + + +# --- the real wizard -------------------------------------------------------------------- + +def test_the_real_wizard_still_declares_a_readable_version(): + """Guards the assumption the whole check rests on: VERSION is a module-level literal.""" + version, lineno = release_version.read_version() + assert release_version.VERSION_PATTERN.match(version), f"{version!r} is not a version number" + assert lineno > 0