diff --git a/.github/workflows/test-python.yml b/.github/workflows/test-python.yml index 59e0951..3a7cec2 100644 --- a/.github/workflows/test-python.yml +++ b/.github/workflows/test-python.yml @@ -7,11 +7,13 @@ on: pull_request: paths: - "sdk-python/**" + - "scripts/ga/cleanroom_python_assert.py" - ".github/workflows/test-python.yml" push: branches: [main] paths: - "sdk-python/**" + - "scripts/ga/cleanroom_python_assert.py" concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -37,3 +39,48 @@ jobs: run: python -m pip install --upgrade pip && python -m pip install -e '.[dev]' - name: Test (offline) run: python -m pytest -q + + smoke-install: + # Regression guard for the fresh-install class of bug: builds the actual wheel from + # this checkout, installs it (no `-e`, no repo on sys.path) into a throwaway venv, + # and runs the SAME probe `scripts/ga/registry-cleanroom.mjs` runs against the live + # PyPI artifact after a publish. `python -m pytest` from the repo checkout does NOT + # catch this class of bug (the checkout dir is first on sys.path, which is exactly + # how wave-av-sdk 2.0.0's `wave` -> stdlib collision hid through review and CI) — this + # job is the one gate that runs it the way a real `pip install wave-av-sdk` user does, + # and it runs on every PR, before any registry publish is even possible. + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ["3.10", "3.12"] + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: ${{ matrix.python-version }} + - name: Build wheel + working-directory: sdk-python + run: python -m pip install --upgrade pip && pip install build && python -m build --wheel + - name: Create fresh venv (no repo on sys.path) + run: python -m venv "$RUNNER_TEMP/smoke" + - name: Install the built wheel + run: | + WHEEL=$(ls sdk-python/dist/*.whl) + "$RUNNER_TEMP/smoke/bin/pip" install --upgrade pip + "$RUNNER_TEMP/smoke/bin/pip" install "$WHEEL" + - name: Copy the clean-room probe into the smoke venv + run: cp scripts/ga/cleanroom_python_assert.py "$RUNNER_TEMP/smoke/cleanroom_python_assert.py" + - name: Assert the installed wheel imports and does not shadow the stdlib + working-directory: ${{ runner.temp }}/smoke + run: | + set -euo pipefail + out=$(bin/python cleanroom_python_assert.py --dist wave-av-sdk --module wave_sdk --symbol Wave) + echo "$out" + echo "$out" | bin/python -c ' + import json, sys + report = json.load(sys.stdin) + failed = [c["name"] for c in report["checks"] if not c["ok"]] + if failed: + print("FAILED:", failed) + sys.exit(1) + ' diff --git a/scripts/ga/cleanroom-targets.mjs b/scripts/ga/cleanroom-targets.mjs index 39b83f9..8b4d125 100644 --- a/scripts/ga/cleanroom-targets.mjs +++ b/scripts/ga/cleanroom-targets.mjs @@ -1,7 +1,7 @@ // Per-ecosystem target runners: stand up the clean room, install the published artifact, then // hand a context to the checks. Nothing here reads the repository checkout. -import { mkdtempSync } from 'node:fs'; +import { copyFileSync, mkdtempSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -244,7 +244,16 @@ export async function runPypiTarget(target, args) { // cwd is the throwaway room, never the repo: a checkout on sys.path could satisfy an import the // published wheel is supposed to satisfy — exactly the illusion this suite exists to destroy. // cleanroom_python_assert.py re-verifies that independently and reports it as its own check. - const argv = [join(HERE, 'cleanroom_python_assert.py'), '--dist', name, '--module', target.import_module]; + // + // The probe is COPIED into the room before it runs. Python unconditionally prepends the + // executed script's own directory to sys.path, so running it in place put `/scripts/ga` + // — a repository directory — first on the path of every clean-room probe, which is the one + // thing this whole suite promises never happens. (`-P`/PYTHONSAFEPATH would also fix it but + // is 3.11+; the venv's interpreter version is not ours to assume.) `.github/workflows/ + // test-python.yml`'s smoke-install job already copies the probe for the same reason. + const probePath = join(room, 'cleanroom_python_assert.py'); + copyFileSync(join(HERE, 'cleanroom_python_assert.py'), probePath); + const argv = [probePath, '--dist', name, '--module', target.import_module]; if (target.import_symbol) argv.push('--symbol', target.import_symbol); const probe = run(venv.py, argv, { cwd: room, timeout: 180000 }); let parsed; diff --git a/scripts/ga/cleanroom_python_assert.py b/scripts/ga/cleanroom_python_assert.py index de0345b..bee7769 100644 --- a/scripts/ga/cleanroom_python_assert.py +++ b/scripts/ga/cleanroom_python_assert.py @@ -39,6 +39,69 @@ def check(name: str, ok: bool, detail: str) -> dict: return {"name": name, "ok": bool(ok), "detail": detail} +# Source-root layouts a checkout can present for the module under test. The clean room must +# not be able to satisfy `import ` from ANY of them, and the guard must not be keyed +# on one repository's directory names — that is precisely how it went blind before: +# "" wave-av/sdk-python -> `wave_sdk/` at the repository root +# "sdk-python" wave-av/sdks -> `sdk-python/wave_sdk/` +# "src"/"python" the two other conventional source roots, so a future re-layout of either +# repo does not silently disarm this check again +CHECKOUT_SUBROOTS: tuple[str, ...] = ("", "sdk-python", "src", "python") + + +def interpreter_owned(path: str) -> bool: + """True when a sys.path entry belongs to the interpreter/venv under test. + + site-packages (where the published wheel legitimately installs), the stdlib, and + everything else under the venv's own prefix are not leaks. They must be excluded + explicitly, because the leak test below asks "can this entry supply the module?" and + the correct answer for site-packages is yes. The install roots are read from sysconfig + and `site` rather than assumed to live under `sys.prefix`, so a `--user` install (which + does not) cannot be mistaken for a checkout. + """ + rp = realpath(path) + if not rp: + return False + roots = [realpath(sys.prefix), realpath(getattr(sys, "base_prefix", sys.prefix))] + paths = sysconfig.get_paths() + roots += [realpath(paths[k]) for k in ("purelib", "platlib", "stdlib", "platstdlib") if k in paths] + try: + import site + + roots += [realpath(p) for p in (getattr(site, "USER_SITE", None), getattr(site, "USER_BASE", None)) if p] + except ImportError: # pragma: no cover - `site` is unimportable only under -S + pass + return any(r and (rp == r or rp.startswith(r + os.sep)) for r in roots) + + +def checkout_paths_providing(paths: list[str], module: str) -> list[str]: + """sys.path entries from which a SOURCE CHECKOUT could satisfy `import `. + + Layout-independent on purpose. The previous implementation looked for one hardcoded + path (`/sdk-python/wave`); when `wave` was renamed to `wave_sdk` it matched + nothing anywhere and reported "no repo checkout on sys.path" unconditionally — a guard + that cannot fail, which is worse than no guard, because the whole clean-room result + rests on it. Keying on the module actually under test, across the layouts a checkout + can have, is what makes it self-maintaining. + """ + hits: list[str] = [] + for p in paths: + entry = p or os.getcwd() # '' means cwd, which can absolutely be a checkout + if interpreter_owned(entry): + continue + for sub in CHECKOUT_SUBROOTS: + base = os.path.join(entry, sub) if sub else entry + pkg_dir = os.path.join(base, module) + mod_file = os.path.join(base, module + ".py") + if os.path.isdir(pkg_dir): + hits.append(pkg_dir) + break + if os.path.isfile(mod_file): + hits.append(mod_file) + break + return hits + + def dist_top_level(dist_name: str) -> list[str]: """Top-level import names the installed distribution claims. @@ -84,10 +147,13 @@ def main() -> int: checks: list[dict] = [] # Guard: a repo checkout on sys.path would make this whole run meaningless. - repo_marker_on_path = [ - p for p in sys.path - if p and os.path.isdir(os.path.join(p, "sdk-python", "wave")) - ] + # Keyed on `args.module` (the same name the import check below uses) and on every + # source-root layout a checkout can present, not on a literal `sdk-python/wave`: + # that literal was the pre-rename package directory of ONE of the two repositories + # this probe runs against, so after the `wave` -> `wave_sdk` rename (ART-001) it + # matched nothing at all and this guard reported a pass unconditionally — blind to + # the exact repo-on-sys.path leak it exists to catch. See checkout_paths_providing(). + repo_marker_on_path = checkout_paths_providing(list(sys.path), args.module) checks.append(check( "cleanroom-isolation", not repo_marker_on_path, diff --git a/sdk-python/pyproject.toml b/sdk-python/pyproject.toml index b81f9eb..ec3aad1 100644 --- a/sdk-python/pyproject.toml +++ b/sdk-python/pyproject.toml @@ -71,6 +71,9 @@ dev = [ "mypy>=1.0.0", "ruff>=0.1.0", "black>=23.0.0", + # tests/test_packaging.py reads this file back to assert the shipped metadata + # matches the repo (name/version). tomllib is stdlib from 3.11 only. + "tomli>=2.0.0; python_version < '3.11'", ] [project.urls] diff --git a/sdk-python/tests/test_cleanroom_probe.py b/sdk-python/tests/test_cleanroom_probe.py new file mode 100644 index 0000000..bdb10c5 --- /dev/null +++ b/sdk-python/tests/test_cleanroom_probe.py @@ -0,0 +1,144 @@ +"""Unit guards for the clean-room isolation check in `scripts/ga/cleanroom_python_assert.py`. + +WHY THESE EXIST +--------------- +`cleanroom-isolation` is the load-bearing assertion of the entire registry clean-room gate: +every other check in that probe (`py-import-module`, `py-no-stdlib-shadow`) is only +meaningful if no source checkout can satisfy the import that the PUBLISHED WHEEL is +supposed to satisfy. If the isolation check is wrong, the gate can report a confident PASS +on a package that is completely broken for real users. + +It WAS wrong. The check was keyed on one hardcoded path — `/sdk-python/wave` +— which was the pre-rename layout of exactly one of the two repositories the probe runs +against. After `wave` -> `wave_sdk` (ART-001) it matched nothing on any machine, so it +reported "no repo checkout on sys.path" unconditionally: a check that could not fail. That +is strictly worse than no check, because the rest of the gate's verdict rests on it. + +A hardcoded-path guard has no test that would have caught this, so the fix is not just a +better path list — it is `checkout_paths_providing()`, a pure function over an explicit +sys.path, exercised below against real directory trees for BOTH repository layouts plus the +false-positive case (site-packages, where finding the module is the correct outcome). These +run offline in the normal `pytest` pass, so the guard can never again silently disarm +itself between releases. +""" + +from __future__ import annotations + +import importlib.util +import os +import sys +from pathlib import Path + +import pytest + +PROBE_PATH = Path(__file__).resolve().parents[2] / "scripts" / "ga" / "cleanroom_python_assert.py" + + +def _load_probe(): + """Import the GA probe by path — it is a script, not an installed module.""" + assert PROBE_PATH.is_file(), ( + f"clean-room probe not found at {PROBE_PATH}. If it moved, this test file and " + f".github/workflows/test-python.yml's path filter must move with it." + ) + spec = importlib.util.spec_from_file_location("_ga_cleanroom_probe", PROBE_PATH) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +probe = _load_probe() + + +def _make_checkout(root: Path, subroot: str, module: str) -> Path: + """Create a source-checkout-shaped tree: ///__init__.py.""" + pkg = (root / subroot / module) if subroot else (root / module) + pkg.mkdir(parents=True) + (pkg / "__init__.py").write_text("") + return pkg + + +# --- control: the helper can see anything at all ------------------------------------------ + + +def test_flags_root_level_checkout_layout(tmp_path: Path): + """`wave-av/sdk-python` layout: `wave_sdk/` sits at the repository root. + + The pre-fix guard looked only under `/sdk-python/`, so this — a checkout of the + OTHER repository the probe runs against — was invisible to it. + """ + _make_checkout(tmp_path, "", "wave_sdk") + hits = probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") + assert hits, "a root-level `wave_sdk/` on sys.path must be reported as a clean-room leak" + + +def test_flags_nested_sdk_python_checkout_layout(tmp_path: Path): + """`wave-av/sdks` layout: `sdk-python/wave_sdk/`. This is this repository.""" + _make_checkout(tmp_path, "sdk-python", "wave_sdk") + hits = probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") + assert hits, "a `sdk-python/wave_sdk/` checkout on sys.path must be reported as a leak" + + +def test_flags_single_file_module(tmp_path: Path): + """A leak does not have to be a package — `wave_sdk.py` shadows just as effectively.""" + (tmp_path / "wave_sdk.py").write_text("") + hits = probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") + assert hits, "a top-level `wave_sdk.py` on sys.path must be reported as a leak" + + +def test_flags_the_empty_sys_path_entry_meaning_cwd(tmp_path: Path, monkeypatch): + """`''` on sys.path means the current directory, which can absolutely be a checkout. + + The pre-fix guard skipped falsy entries outright, so `cd` into a checkout and run the + probe and it saw nothing. + """ + _make_checkout(tmp_path, "", "wave_sdk") + monkeypatch.chdir(tmp_path) + hits = probe.checkout_paths_providing([""], "wave_sdk") + assert hits, "the '' sys.path entry (cwd) must be resolved and checked, not skipped" + + +# --- false-positive controls: the guard must stay usable ---------------------------------- + + +def test_does_not_flag_the_interpreters_own_site_packages(): + """Finding the module in site-packages is the CORRECT outcome, not a leak. + + Without this exclusion the guard would fire on every legitimate run, and a guard that + fails when everything is fine gets deleted — which is how gates die. + """ + site_dirs = [p for p in sys.path if "site-packages" in p] + if not site_dirs: + pytest.skip("no site-packages on sys.path in this environment") + for entry in site_dirs: + assert probe.interpreter_owned(entry), ( + f"{entry} is inside the interpreter prefix and must never be treated as a " + f"repository checkout" + ) + assert probe.checkout_paths_providing(site_dirs, "wave_sdk") == [] + + +def test_does_not_flag_an_unrelated_directory(tmp_path: Path): + """A sys.path entry that cannot supply the module under test is not a leak.""" + (tmp_path / "docs").mkdir() + assert probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") == [] + + +def test_guard_is_keyed_on_the_module_under_test(tmp_path: Path): + """The self-maintenance property: rename the package, the guard follows it. + + A checkout of the PRE-rename layout (`wave/`) is not a leak for `import wave_sdk`, and + a checkout of whatever the module is called today always is. This is the assertion the + hardcoded `"wave"` literal could not make, and its absence is why the rename disarmed + the check without a single test going red. + """ + _make_checkout(tmp_path, "sdk-python", "wave") + assert probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") == [] + assert probe.checkout_paths_providing([str(tmp_path)], "wave") != [] + + +def test_reported_hit_points_at_the_offending_path(tmp_path: Path): + """The failure message has to name the leaking path, not just say something is wrong.""" + pkg = _make_checkout(tmp_path, "sdk-python", "wave_sdk") + hits = probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") + assert [os.path.realpath(h) for h in hits] == [os.path.realpath(str(pkg))] diff --git a/sdk-python/tests/test_packaging.py b/sdk-python/tests/test_packaging.py new file mode 100644 index 0000000..17bea32 --- /dev/null +++ b/sdk-python/tests/test_packaging.py @@ -0,0 +1,149 @@ +""" +Packaging / distribution-metadata guards. + +These tests exist because of a class of defect that NO other gate in this repo caught +offline, and that only became visible once the package was on PyPI — where it is +unfixable, since PyPI refuses a re-upload of an already-published version. + +Every published `wave-av-sdk` release through `2.0.1` shipped a top-level package named +`wave`. CPython's standard library ships `Lib/wave.py` (WAV audio I/O), and the stdlib +directory sits AHEAD of `site-packages` on `sys.path`. So `import wave` in a fresh +`pip install wave-av-sdk` resolved to the stdlib module and the entire SDK was +unreachable — the artifact was 100% unimportable via its own documented entry point, on +every Python version (`scripts/ga/registry-cleanroom.mjs` in this repo proved this +against the live PyPI artifact: `py-import-module` / `py-no-stdlib-shadow`, both FAIL on +`wave-av-sdk@2.0.0`). The repo checkout hid it during development: the checkout +directory is first on `sys.path`, so the local `wave/` package won `import wave` under +pytest, and `pip install -e .` (an editable install, which also just points back at the +checkout) hid it in CI too — neither ever exercised a real built-and-installed wheel. + +`.github/workflows/test-python.yml` (the `smoke-install` job) guards the same class at +the wheel level: build -> fresh venv -> install -> import using this repo's own +`scripts/ga/cleanroom_python_assert.py`, the exact probe the GA gate runs against the +live registry. These tests are the cheap, always-on half: they fail at PR time, in the +normal unit run, before a wheel is ever built. + +Guarded here: + 1. No top-level package this repo ships may shadow a stdlib module name. + 2. `import wave` (bare) must still resolve to the stdlib, from inside the checkout. + 3. `wave_sdk.__version__` must equal `[project] version` in pyproject.toml. + 4. `[project] name` must still be the distribution name README/CHANGELOG promise. +""" + +from __future__ import annotations + +import sys +import sysconfig +from pathlib import Path + +try: # tomllib is stdlib from 3.11; tomli is the dev-extra fallback below that. + import tomllib +except ModuleNotFoundError: # pragma: no cover - exercised on 3.10 only + import tomli as tomllib + +REPO_ROOT = Path(__file__).resolve().parent.parent +PYPROJECT = REPO_ROOT / "pyproject.toml" + + +def _pyproject() -> dict: + with PYPROJECT.open("rb") as fh: + return tomllib.load(fh) + + +def _stdlib_top_level_names() -> set[str]: + """Every name `import ` could resolve to from the standard library. + + `sys.stdlib_module_names` is 3.10+ (this package's floor), so that alone would + suffice, but the on-disk scan is unioned in too: it costs nothing and it means this + guard does not silently get weaker if it is ever backported below 3.10. + """ + names = set(sys.builtin_module_names) + names |= set(getattr(sys, "stdlib_module_names", ())) + stdlib_dir = Path(sysconfig.get_paths()["stdlib"]) + if stdlib_dir.is_dir(): + for entry in stdlib_dir.iterdir(): + if entry.suffix == ".py": + names.add(entry.stem) + elif entry.is_dir() and (entry / "__init__.py").exists(): + names.add(entry.name) + return names + + +def _shipped_top_level_packages() -> list[str]: + """Top-level importable packages in the checkout that setuptools will ship. + + Derived from the filesystem (any root-level directory with an `__init__.py`) rather + than from the pyproject include-glob (`wave_sdk*`), because the failure mode being + guarded is exactly someone re-adding a directory the glob would sweep up — reading + the glob back would make this test agree with the very config that could regress. + `tests`, `scripts` and `examples` are excluded: none is in + `[tool.setuptools.packages.find] include`, so none is ever part of the distribution. + """ + skip = {"tests", "scripts", "examples"} + return sorted( + p.name + for p in REPO_ROOT.iterdir() + if p.is_dir() + and not p.name.startswith((".", "_")) + and p.name not in skip + and (p / "__init__.py").exists() + ) + + +def test_repo_ships_the_wave_sdk_package(): + """Control for the shadow test below: prove the scan sees anything at all. + + Without this, a bug that made `_shipped_top_level_packages()` return `[]` would turn + the shadow guard into a test that can never fail. + """ + assert "wave_sdk" in _shipped_top_level_packages() + + +def test_no_shipped_package_shadows_a_stdlib_module(): + """A distribution package named after a stdlib module is permanently unimportable. + + site-packages comes AFTER the stdlib on sys.path, so the stdlib always wins. + """ + stdlib = _stdlib_top_level_names() + collisions = [name for name in _shipped_top_level_packages() if name in stdlib] + assert collisions == [], ( + f"top-level package(s) {collisions} collide with a Python standard-library " + f"module name. The stdlib precedes site-packages on sys.path, so a user who " + f"runs `pip install wave-av-sdk` could never import them. Rename the package " + f"(this is exactly the defect that shipped as wave-av-sdk 2.0.0's `wave`)." + ) + + +def test_import_wave_still_resolves_to_the_standard_library(): + """The specific regression: re-adding a top-level `wave/` here would break users. + + Run from the repo checkout, the checkout is first on sys.path — so if a `wave/` + package reappears, this assertion fails HERE, which is the one place the old bug + was invisible (both under pytest and under `pip install -e .`). + """ + import wave # noqa: F401 - imported for its resolved location, not its API + + stdlib_dir = Path(sysconfig.get_paths()["stdlib"]).resolve() + resolved = Path(wave.__file__).resolve() + assert stdlib_dir in resolved.parents, ( + f"`import wave` resolved to {resolved}, not the standard library at " + f"{stdlib_dir}. A top-level `wave` package has been reintroduced." + ) + assert REPO_ROOT not in resolved.parents, f"`import wave` resolved into this repo: {resolved}" + + +def test_dunder_version_matches_pyproject_version(): + """`wave_sdk.__version__` is what users print; pyproject is what PyPI records. + + A hardcoded literal in a test (or in `__init__.py` itself) can drift from + `pyproject.toml` with no build-time signal — this reads pyproject back rather than + encoding the expected value, so it fails the moment the two disagree either way. + """ + import wave_sdk + + assert wave_sdk.__version__ == _pyproject()["project"]["version"] + + +def test_distribution_name_is_wave_av_sdk(): + """`pip install wave-av-sdk` is what README/CHANGELOG currently promise.""" + assert _pyproject()["project"]["name"] == "wave-av-sdk"