ci: enforce one canonical demo version - #38
codeforester merged 3 commits into
Conversation
| source_version = Path("VERSION").read_text(encoding="utf-8").strip() | ||
| representations = {"VERSION": source_version} | ||
| if raw_tag.startswith("v"): | ||
| representations["tag"] = raw_tag.removeprefix("v") |
There was a problem hiding this comment.
Tautological tag verification — this check can never fail on the actual tag-push release path, so it verifies nothing beyond the original one-line check it replaced.
On a tag push, expected (line 35) is ${GITHUB_REF_NAME#v}, and representations["tag"] (line 48) is raw_tag.removeprefix("v") where raw_tag is the same GITHUB_REF_NAME. Both sides are the identical transformation of the identical source, so expected == representations["tag"] by construction. A tag/VERSION mismatch is still only caught via the VERSION key — i.e. no more protection than before, even though this reads as an independent tag-vs-source cross-check.
|
|
||
| wheel_path = Path(sys.argv[1]) | ||
| sdist_path = Path(sys.argv[2]) | ||
| expected_version = Path("VERSION").read_text(encoding="utf-8").strip() |
There was a problem hiding this comment.
Cleanup: VERSION-file read duplicated verbatim in three places.
Path("VERSION").read_text(encoding="utf-8").strip() appears here, in .github/workflows/release-package.yml (the heredoc), and in tests/test_version_identity.py:17 — even though all three already import the shared tests/version_identity.py module for assert_versions_match. That module centralized the comparison logic but not the 'read the expected version' step common to every call site. A one-line expected_version() helper in version_identity.py would remove the triplication and the risk of the three drifting (e.g. if the trim/encoding rule ever changes).
| assert "--dry-run" in result.stdout | ||
|
|
||
|
|
||
| def test_cli_version_matches_the_installed_package_metadata() -> None: |
There was a problem hiding this comment.
Cleanup: near-duplicate of tests/test_version_identity.py::test_source_version_matches_installed_metadata_and_cli.
This new test asserts essentially the same property (module __version__ == importlib.metadata.version("base-cli-demo") == CLI --version output) with a weaker, ad-hoc assert __version__ in result.stdout" substring check, duplicating what test_version_identity.py's test already verifies more rigorously via the shared assert_versions_match" helper (which also checks against the VERSION file). Two near-identical tests now need to be kept in sync for any change to the --version output format.
| fi | ||
| test -n "$expected" | ||
| test "$(tr -d '[:space:]' < VERSION)" = "$expected" | ||
| PYTHONPATH=tests EXPECTED_VERSION="$expected" RELEASE_TAG="$GITHUB_REF_NAME" python - <<'PY' |
There was a problem hiding this comment.
Altitude: inlined YAML heredoc deviates from this PR's own established pattern.
This same PR keeps release-validation logic in checked-in, executable scripts under tests/ (package.sh, validate.sh), each invoked as its own workflow step. This new 'Verify release version' step instead inlines a fresh, non-trivial Python heredoc directly in the workflow YAML. It can't be run or linted locally the way tests/package.sh can, isn't covered by pytest, and edits require hand-editing YAML-embedded, indentation-sensitive heredoc syntax. A tests/verify_release_version.sh alongside package.sh/validate.sh would match the rest of the PR.
|
|
||
| with zipfile.ZipFile(wheel_path) as wheel: | ||
| wheel_names = set(wheel.namelist()) | ||
| metadata_name = next( |
There was a problem hiding this comment.
Cleanup: wheel/sdist metadata extraction copy-pasted with only the archive API swapped.
Lines 31-36 (zipfile) and lines 46-53 (tarfile) both: find the member ending in the metadata filename via next(name for name in names if name.endswith(...)), then Parser().parsestr(...) on its decoded bytes. A future fix to this parsing step (e.g. handling multiple matches, decode errors, or a clearer missing-metadata message) has to be applied twice by hand with nothing tying the two copies together.
| __all__ = ["__version__"] | ||
|
|
||
| __version__ = "0.1.0" | ||
| __version__ = version("base-cli-demo") |
There was a problem hiding this comment.
Stale version in a long-lived editable install — __version__ now reads installed package metadata instead of VERSION, so it silently goes stale after a VERSION bump until the package is reinstalled.
Reproduced directly: with pip install -e . already done, editing VERSION from 0.1.0 to 9.9.9 and running python -m pytest tests/test_version_identity.py still reports __version__/CLI/distribution as 0.1.0 while Path("VERSION").read_text() reads 9.9.9 — so test_source_version_matches_installed_metadata_and_cli fails with a confusing 'Release version mismatch' error even though nothing is actually broken. This is exactly the workflow the repo's own release-process doc prescribes ('At release preparation time, update VERSION ... and run the release checks'), so a contributor bumping VERSION locally before committing will hit this every time until they reinstall.
Separately, this also hardcodes the distribution name "base-cli-demo" as a second literal duplicating name in pyproject.toml — importlib.metadata.version(__package__) would avoid that duplication (and still resolves correctly since importlib.metadata normalizes '-'/'_').
| with zipfile.ZipFile(wheel_path) as wheel: | ||
| wheel_names = set(wheel.namelist()) | ||
| metadata_name = next( | ||
| name for name in wheel_names if name.endswith(".dist-info/METADATA") |
There was a problem hiding this comment.
Unhandled StopIteration on missing metadata entry — the wheel/sdist metadata-file lookups use next(...) with no default, so an unexpectedly missing METADATA/PKG-INFO entry fails with a raw StopIteration traceback instead of a clear validation error.
If a future packaging change alters the dist-info naming and no file ends with .dist-info/METADATA (or /PKG-INFO for the sdist, checked a few lines below), next() raises StopIteration with no message — unlike the clear SystemExit(f"...is missing: ...") messages this same script uses for other missing-file checks.
1 participant
Fixes #26