diff --git a/.github/workflows/python-cli-wheels.yml b/.github/workflows/python-cli-wheels.yml index 47bb408c..6255c18c 100644 --- a/.github/workflows/python-cli-wheels.yml +++ b/.github/workflows/python-cli-wheels.yml @@ -308,6 +308,16 @@ jobs: runs-on: ${{ matrix.runs-on }} timeout-minutes: 15 steps: + # Sparse, submodule-free checkout of just the smoke harness so the + # step below can run `scripts/smoke/cli_wheel_smoke.sh` against the + # downloaded wheel (#995 — extracted from the former inline `run:` + # block so the assertions are shellcheck-lintable and also run + # per-PR via smoke-dryrun.yml). + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + sparse-checkout: scripts/smoke + submodules: false + - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 with: python-version: ${{ env.PYTHON_VERSION_HOST }} @@ -334,33 +344,13 @@ jobs: # rather than silently installing a stale published version. python -m pip install --no-index --find-links=dist big-code-analysis-cli # The console script lands in the setup-python interpreter's - # scripts dir, which is on PATH. Capture the version rather than - # piping to `head` (a `| head -n1` would SIGPIPE the producer - # under pipefail — see release.yml's note). - ver_out=$(bca --version) - echo "$ver_out" - # On a tag build, prove the binary reports the lockstep release - # version (maturin reads `version.workspace = true`, so the - # wheel version must equal the tag minus its leading `v`). - if [[ -n "$EXPECTED_TAG" ]]; then - want="${EXPECTED_TAG#v}" - case "$ver_out" in - *"$want"*) : ;; - *) echo "::error::bca --version '$ver_out' does not contain tag version $want"; exit 1 ;; - esac - fi - bca list-metrics names >/dev/null - # Parse two unrelated languages to prove the all-languages - # grammar set is compiled in, not just the host language. - printf 'def add(a, b):\n if a > b:\n return a\n return b\n' > smoke.py - printf 'fn main() { if true { println!("x"); } }\n' > smoke.rs - py_cc=$(bca metrics --paths smoke.py -O json | python -c "import sys,json; print(json.load(sys.stdin)['metrics']['cyclomatic']['sum'])") - rs_cc=$(bca metrics --paths smoke.rs -O json | python -c "import sys,json; print(json.load(sys.stdin)['metrics']['cyclomatic']['sum'])") - # 2.0 serializes integer-valued metrics as integers (#530), - # so cyclomatic.sum prints as `3`, not the pre-2.0 `3.0`. - test "$py_cc" = "3" || { echo "::error::python cyclomatic.sum=$py_cc, expected 3"; exit 1; } - test "$rs_cc" = "3" || { echo "::error::rust cyclomatic.sum=$rs_cc, expected 3"; exit 1; } - echo "smoke OK" + # scripts dir, which is on PATH. The assertions (version + # lockstep against EXPECTED_TAG, the all-languages grammar set, + # and the #530 integer-metric serialization) live in the + # checked-in smoke harness so they also run per-PR via + # smoke-dryrun.yml (#995). Invoke via `bash` so the Windows + # matrix leg (Git Bash, no executable bit) runs it too. + bash scripts/smoke/cli_wheel_smoke.sh # Aggregate gate so branch protection can require a single check name. # Every dependency is label-gated at PR time, so on an unlabelled PR all diff --git a/.github/workflows/python-wheels.yml b/.github/workflows/python-wheels.yml index a34721ca..2f88efe3 100644 --- a/.github/workflows/python-wheels.yml +++ b/.github/workflows/python-wheels.yml @@ -256,12 +256,18 @@ jobs: runs-on: ${{ matrix.runs-on }} timeout-minutes: 15 steps: - # No `actions/checkout` — the smoke step reads only the - # downloaded wheel artefact and runs an inline Python heredoc; - # no repo file (fixture, helper, doc) is touched. Skipping the - # checkout saves ~10-20 s per matrix leg on aarch64 (the - # native-ARM runner's clone over LFS-adjacent submodules takes - # the wall-clock penalty even when we skip submodules). + # Sparse, submodule-free checkout of just the smoke harness. The + # smoke step runs `scripts/smoke/lib_wheel_smoke.py` against the + # downloaded wheel (#995 — extracted from the former inline heredoc + # so the assertions are lintable and also run per-PR via + # smoke-dryrun.yml). `sparse-checkout` + `submodules: false` keep + # the aarch64 native-ARM clone cheap: the wall-clock cost this job + # historically skipped was the full clone's LFS-adjacent submodules, + # not the handful of files under scripts/smoke. + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + sparse-checkout: scripts/smoke + submodules: false - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 with: python-version: ${{ matrix.python }} @@ -286,103 +292,19 @@ jobs: # analyze_source returns a dict, flatten_spaces yields function # records, to_sarif emits SARIF JSON, language_for_file resolves # extensions. If any of these regress the smoke fails before - # the wheel can publish. - # - # Assertions are deliberately VALUE-bearing, not just shape- - # bearing — a binding regression that returned `{"spaces": []}` - # would satisfy a bare `isinstance(result, dict)` check but - # fail the `records[*]["name"] == "add"` check below. + # the wheel can publish. The assertions are deliberately + # VALUE-bearing, not just shape-bearing — see the per-API + # `_check_*` helpers in scripts/smoke/lib_wheel_smoke.py. - name: Import + analyse smoke env: # Force unbuffered stdout so failure assertions show up # immediately in the action log instead of after the python # process has been torn down. PYTHONUNBUFFERED: "1" - run: | - python - <<'PY' - import json - import tempfile - from pathlib import Path - - import big_code_analysis as bca - - # Sanity: package + extension loaded. - assert bca.__version__, repr(bca.__version__) - assert "python" in bca.supported_languages() - # Extensions are bare names (no leading dot) — match the - # `language_extensions` contract in the stub. The CLI used - # to expose dotted forms; the binding does not. - assert "py" in bca.language_extensions("python"), bca.language_extensions("python") - - # `language_for_file` reads the file (parity with `analyze`, - # #318), so a stub path that does not exist raises - # FileNotFoundError. Materialise an empty fixture in a - # tempdir so the assertion exercises the extension table - # path rather than crashing on I/O. - with tempfile.TemporaryDirectory() as td: - fixture = Path(td) / "foo.py" - fixture.write_bytes(b"") - resolved = bca.language_for_file(fixture) - assert resolved == "python", resolved - - # `language` is a positional-only parameter on - # `analyze_source`; passing it as a kwarg raises TypeError. - src = "def add(a, b):\n if a > b:\n return a\n return b\n" - result = bca.analyze_source(src, "python") - assert isinstance(result, dict), type(result) - assert result.get("kind") == "unit", result.get("kind") - # The unit-level cyclomatic is 3 for this fixture (one - # function spanning the file + one `if` branch). A binding - # regression that produced null / zero metrics would slip - # past `isinstance(result, dict)` but trip this assertion. - cyclomatic_sum = result["metrics"]["cyclomatic"]["sum"] - assert cyclomatic_sum == 3.0, f"expected unit cyclomatic.sum == 3.0, got {cyclomatic_sum}" - - # `flatten_spaces` yields one record per space; for this - # fixture that is the unit + the `add` function. The - # function record's cyclomatic.sum is 2 (1 entry + 1 if). - records = list(bca.flatten_spaces(result)) - assert records, "flatten_spaces returned no records" - add_records = [r for r in records if r.get("name") == "add"] - assert add_records, f"no 'add' function in records: {[r.get('name') for r in records]}" - assert add_records[0].get("kind") == "function", add_records[0].get("kind") - assert add_records[0].get("cyclomatic.sum") == 2.0, add_records[0].get("cyclomatic.sum") - - # SARIF must be a well-formed 2.1.0 document with at least - # one run carrying the bca tool driver. A stub - # `{"version": "2.1.0"}` (the most plausible regression of - # an empty-run SARIF writer) would fail the runs check. - sarif_obj = json.loads(bca.to_sarif(result)) - assert sarif_obj.get("version") == "2.1.0", sarif_obj.get("version") - runs = sarif_obj.get("runs") or [] - assert len(runs) == 1, f"expected 1 SARIF run, got {len(runs)}" - driver = runs[0].get("tool", {}).get("driver", {}) - assert driver.get("name"), f"SARIF run missing tool.driver.name: {driver}" - - # analyze_batch's never-raise contract: a missing path in - # the batch must be returned as an AnalysisFailure instance - # interleaved with the successful dicts, never as an - # exception. A binding regression that switched batch back - # to raising would crash here; one that dropped error - # records entirely would shrink `len(batch)` below the - # input count and trip the assertion. - with tempfile.TemporaryDirectory() as td: - good = Path(td) / "good.py" - good.write_text("def f():\n pass\n") - missing = Path(td) / "does-not-exist.py" - batch = bca.analyze_batch([good, missing]) - assert len(batch) == 2, f"expected 2 batch results, got {len(batch)}" - # Good path → dict. - assert isinstance(batch[0], dict), type(batch[0]) - # Missing path → AnalysisFailure with `error_kind == "IoError"` - # (the type was renamed from AnalysisError at 2.0, #614). - err = batch[1] - assert isinstance(err, bca.AnalysisFailure), type(err) - assert err.error_kind == "IoError", err.error_kind - assert str(missing) in err.path, (err.path, str(missing)) - - print("smoke OK:", bca.__version__) - PY + # The assertions live in a checked-in, mypy-strict, ruff-linted + # script (#995) so a binding rename/serialization change reds a + # PR via smoke-dryrun.yml instead of only this tag-gated job. + run: python scripts/smoke/lib_wheel_smoke.py # pip-audit deliberately NOT run here. The pre-publish wheel # has zero Python runtime deps (the binary surface is the diff --git a/.github/workflows/smoke-dryrun.yml b/.github/workflows/smoke-dryrun.yml new file mode 100644 index 00000000..5071e8ce --- /dev/null +++ b/.github/workflows/smoke-dryrun.yml @@ -0,0 +1,108 @@ +name: Smoke dry-run + +# Per-PR drift gate for the release/wheel smoke harnesses (#995). +# +# The wheel + release smoke scripts only ran on a `v*` tag push (or via an +# opt-in `python-*-wheels` PR label), so an assertion that fell out of step +# with a breaking change rotted silently until release day. Three did, +# blocking the `v2.0.0` cut: the #530 integer-metric serialization (CLI +# smoke still expected `"3.0"`) and the #614 `AnalysisError` -> +# `AnalysisFailure` rename (library smoke still referenced the old name). +# +# This job runs the *same* checked-in smoke scripts the wheel workflows now +# invoke (scripts/smoke/*), but against a cheap local dev build instead of a +# packaged wheel — `cargo build` for the CLI and `maturin develop` for the +# bindings. It fires only when the wheel-smoke plumbing or the smoke scripts +# themselves change, so the common case pays nothing. +# +# What this gate catches: drift between the extracted smoke scripts and the +# code they assert against, on a PR that edits the scripts or the wheel +# workflows that invoke them. It is the script/plumbing half of #995's fix. +# The *other* half — a src-side metric rename or serialization change (the +# #530 / #614 class) — is caught on every PR by the per-PR unit/integration +# tests (`cli_metrics_json_serializes_integer_metrics_as_integers` in +# big-code-analysis-cli/tests/format_smoke.rs and the `AnalysisFailure` +# assertions in big-code-analysis-py/tests/test_batch.py), which run under +# normal CI whenever src/** changes. This job does not trigger on src/**, +# by design: those tests already guard that surface, and widening the filter +# would rebuild a wheel on nearly every PR. +# +# Scope note: this validates smoke-script-vs-code drift, not OS-package +# *packaging* (the `.apk` basename collision of bug #1). `release.yml` is +# deliberately NOT a trigger here — its deb/rpm/apk smoke is package-manager- +# specific, not extracted into scripts/smoke/*, so a dev-build run would +# validate nothing in it and produce a hollow green check. Building a signed +# multi-arch `.apk` needs the Alpine/abuild toolchain release.yml carries and +# is too heavy for a per-PR job; that path stays tag-gated, with the arch- +# qualified asset name (c53e504b) guarding the specific regression. + +on: + pull_request: + paths: + - '.github/workflows/python-wheels.yml' + - '.github/workflows/python-cli-wheels.yml' + - '.github/workflows/smoke-dryrun.yml' + - 'scripts/smoke/**' + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + CARGO_TERM_COLOR: always + CARGO_INCREMENTAL: 0 + CARGO_NET_RETRY: 10 + RUSTUP_MAX_RETRIES: 10 + PYTHON_VERSION_HOST: "3.12" + +jobs: + smoke: + name: smoke (dev build) + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + # `submodules: false` — the smoke fixtures are synthesised in-process + # (scripts/smoke/*); the test-repository submodules are not needed and + # are the slow part of a full clone. + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + submodules: false + + - uses: dtolnay/rust-toolchain@67ef31d5b988238dd797d409d6f9574278e20537 # stable tip + with: + toolchain: stable + + - uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1 + + - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 + with: + python-version: ${{ env.PYTHON_VERSION_HOST }} + + # CLI smoke against a debug `bca` (the wheel workflow runs the same + # script against the packaged console binary). Runs before the venv + # below so `python` resolves to the setup-python interpreter. + - name: CLI smoke (cargo build) + env: + PYTHONUNBUFFERED: "1" + run: | + set -euo pipefail + cargo build -p big-code-analysis-cli + BCA="$PWD/target/debug/bca" bash scripts/smoke/cli_wheel_smoke.sh + + # Library smoke against a `maturin develop` build of the bindings (the + # wheel workflow runs the same script against the packaged abi3 wheel). + - name: Library smoke (maturin develop) + env: + PYTHONUNBUFFERED: "1" + run: | + set -euo pipefail + python -m venv .venv + # shellcheck disable=SC1091 # venv activate script is created above + . .venv/bin/activate + python -m pip install --upgrade pip maturin + maturin develop --manifest-path big-code-analysis-py/Cargo.toml + python scripts/smoke/lib_wheel_smoke.py diff --git a/CHANGELOG.md b/CHANGELOG.md index bb4d1516..90c5afff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,25 @@ for historical reference. ## [Unreleased] +### Added + +- Per-PR coverage for the release/wheel smoke harnesses, closing the + drift gap that let three stale assertions block the `v2.0.0` cut + (#995). The integer-valued-metric JSON serialization (#530) is now + pinned by a CLI integration test + (`cli_metrics_json_serializes_integer_metrics_as_integers`) that + asserts `cyclomatic.sum` serializes as a JSON integer (`is_u64()`), + which the existing `as_f64()`-coercing round-trip tests could not + catch. The previously-inline library and CLI wheel smokes were + extracted into checked-in, lint-gated scripts under + [`scripts/smoke/`](scripts/smoke/) (mypy `--strict` / ruff for the + Python script, shellcheck for the shell script), referenced by both + wheel workflows and runnable locally via `make smoke`. A new + path-filtered [`smoke-dryrun.yml`](.github/workflows/smoke-dryrun.yml) + workflow runs those scripts against a cheap dev build on any PR that + touches the release/wheel plumbing, so a future metric rename or + serialization change reds a PR check instead of a release. + ## [2.0.0] - 2026-06-29 The project's first major version bump since `1.0`, and the first diff --git a/Makefile b/Makefile index 28d1cf76..9746d9d3 100644 --- a/Makefile +++ b/Makefile @@ -23,6 +23,15 @@ BASE_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST)))) # read it). BCA_PY_DIR := $(BASE_DIR)big-code-analysis-py +# The release/wheel smoke harness lives outside the bindings package, so the +# package-scoped py-* gates below pull it in explicitly (#995). Linting it +# with the bindings' ruff/mypy config — rather than letting it drift outside +# every per-PR gate — is the whole point of extracting it from workflow YAML +# (lesson #80). `--config` forces the bindings config since there is no +# config to discover from scripts/smoke upward. +SMOKE_PY_DIR := $(BASE_DIR)scripts/smoke +SMOKE_LIB_PY := $(SMOKE_PY_DIR)/lib_wheel_smoke.py + # Directories excluded from linting and file-search operations. # `tests/repositories` holds vendored fixtures (incl. the # big-code-analysis-output submodule); `tree-sitter-*` are vendored @@ -50,7 +59,7 @@ FIND_EXCLUDE := $(foreach dir,$(EXCLUDE_DIRS),! -path './$(dir)/*') # warnings on `$(2)`, e.g. $(call find-by-ext,md,). find-by-ext = $(if $(FD),$(FD) --extension $(1) $(FD_EXCLUDE) $(2),find . -name "*.$(1)" -type f $(FIND_EXCLUDE)) -.PHONY: help check-tools build build-release check test test-doc fmt fmt-check markdown-fmt markdown-lint shellcheck sh-fmt sh-fmt-check toml-fmt toml-fmt-check toml-lint makefile-check actionlint snapshot-anchors grammar-marker-sync grammar-marker-sync-test check-versions check-manpage-assets enums-check enums-codegen-drift enums-codegen-drift-test self-scan self-scan-headroom self-scan-write-baseline self-scan-write-baseline-headroom vcs lint clippy udeps insta-review insta-accept clean distclean install install-cli install-web doc doc-open doc-check book book-serve book-deploy all pre-commit ci release-check verify-changelog pkg-deb-local pkg-rpm-local py-bootstrap py-sync py-relock py-clean py-fmt py-fmt-check py-lint py-typecheck py-test py-stubtest _check-find _pc-fmt _pc-clippy _pc-test _pc-doc-check _pc-udeps _pc-shellcheck _pc-markdown-lint _pc-toml-lint _pc-makefile-check _pc-actionlint _pc-snapshot-anchors _pc-grammar-marker-sync _pc-grammar-marker-sync-test _pc-check-versions _pc-check-versions-test _pc-check-grammar-crate-test _pc-check-manpage-assets _pc-enums-check _pc-enums-codegen-drift _pc-enums-codegen-drift-test _pc-self-scan _pc-self-scan-headroom _pc-py-fmt _pc-py-typecheck _pc-py-test _pc-py-stubtest _ci-fmt-check _ci-clippy _ci-test _ci-doc-check _ci-build _ci-udeps _ci-shellcheck _ci-markdown-lint _ci-toml-lint _ci-makefile-check _ci-actionlint _ci-snapshot-anchors _ci-grammar-marker-sync _ci-grammar-marker-sync-test _ci-check-versions _ci-check-versions-test _ci-check-grammar-crate-test _ci-check-manpage-assets _ci-enums-check _ci-enums-codegen-drift _ci-enums-codegen-drift-test _ci-enums-codegen-drift-test _ci-self-scan _ci-self-scan-headroom _ci-cargo-pipeline _ci-py-fmt-check _ci-py-lint _ci-py-typecheck _ci-py-test _ci-py-stubtest +.PHONY: help check-tools build build-release check test test-doc fmt fmt-check markdown-fmt markdown-lint shellcheck sh-fmt sh-fmt-check toml-fmt toml-fmt-check toml-lint makefile-check actionlint snapshot-anchors grammar-marker-sync grammar-marker-sync-test check-versions check-manpage-assets enums-check enums-codegen-drift enums-codegen-drift-test self-scan self-scan-headroom self-scan-write-baseline self-scan-write-baseline-headroom vcs lint clippy udeps insta-review insta-accept clean distclean install install-cli install-web doc doc-open doc-check book book-serve book-deploy all pre-commit ci release-check verify-changelog pkg-deb-local pkg-rpm-local py-bootstrap py-sync py-relock py-clean py-fmt py-fmt-check py-lint py-typecheck py-test py-stubtest smoke smoke-cli smoke-lib _check-find _pc-fmt _pc-clippy _pc-test _pc-doc-check _pc-udeps _pc-shellcheck _pc-markdown-lint _pc-toml-lint _pc-makefile-check _pc-actionlint _pc-snapshot-anchors _pc-grammar-marker-sync _pc-grammar-marker-sync-test _pc-check-versions _pc-check-versions-test _pc-check-grammar-crate-test _pc-check-manpage-assets _pc-enums-check _pc-enums-codegen-drift _pc-enums-codegen-drift-test _pc-self-scan _pc-self-scan-headroom _pc-py-fmt _pc-py-typecheck _pc-py-test _pc-py-stubtest _ci-fmt-check _ci-clippy _ci-test _ci-doc-check _ci-build _ci-udeps _ci-shellcheck _ci-markdown-lint _ci-toml-lint _ci-makefile-check _ci-actionlint _ci-snapshot-anchors _ci-grammar-marker-sync _ci-grammar-marker-sync-test _ci-check-versions _ci-check-versions-test _ci-check-grammar-crate-test _ci-check-manpage-assets _ci-enums-check _ci-enums-codegen-drift _ci-enums-codegen-drift-test _ci-enums-codegen-drift-test _ci-self-scan _ci-self-scan-headroom _ci-cargo-pipeline _ci-py-fmt-check _ci-py-lint _ci-py-typecheck _ci-py-test _ci-py-stubtest # Default target help: @@ -114,6 +123,9 @@ help: @echo " py-typecheck Type-check with mypy --strict + pyright" @echo " py-test maturin develop + pytest (needs active venv)" @echo " py-stubtest maturin develop + mypy stubtest of _native.pyi (needs venv)" + @echo " smoke Run the release/wheel smoke scripts locally (smoke-cli + smoke-lib)" + @echo " smoke-cli CLI smoke (scripts/smoke/cli_wheel_smoke.sh) against a debug bca" + @echo " smoke-lib Library smoke (scripts/smoke/lib_wheel_smoke.py) via maturin develop" @echo " (first-time setup: 'make py-bootstrap' — installs uv-managed venv from uv.lock)" @echo "" @echo "Maintenance:" @@ -554,23 +566,33 @@ py-relock: exit 1; } @cd "$(BCA_PY_DIR)" && uv lock +# `scripts/smoke` is a second, separate ruff invocation rather than an extra +# path on the bindings call: `--config` would otherwise re-root the bindings +# dir's own `extend-exclude` (e.g. `_native.pyi`) against the wrong base and +# spuriously flag excluded files. The smoke dir has nothing to exclude, so +# pointing `--config` at the bindings pyproject just gives it the same +# ruleset (line-length 100, the extend-select set) (#995). py-fmt: @if command -v ruff >/dev/null 2>&1; then \ echo "Formatting Python sources..."; \ ruff format "$(BCA_PY_DIR)"; \ + ruff format --config "$(BCA_PY_DIR)/pyproject.toml" "$(SMOKE_PY_DIR)"; \ else echo "ruff not found; skipping py-fmt"; fi py-fmt-check: @if command -v ruff >/dev/null 2>&1; then \ echo "Checking Python formatting..."; \ - ruff format --check "$(BCA_PY_DIR)" || \ + ruff format --check "$(BCA_PY_DIR)" && \ + ruff format --check --config "$(BCA_PY_DIR)/pyproject.toml" "$(SMOKE_PY_DIR)" || \ { echo "Python files not formatted (run 'make py-fmt')"; exit 1; }; \ else echo "ruff not found; skipping py-fmt-check"; fi py-lint: @if command -v ruff >/dev/null 2>&1; then \ echo "Linting Python sources..."; \ - ruff check "$(BCA_PY_DIR)" || { echo "ruff lint found issues"; exit 1; }; \ + ruff check "$(BCA_PY_DIR)" && \ + ruff check --config "$(BCA_PY_DIR)/pyproject.toml" "$(SMOKE_PY_DIR)" || \ + { echo "ruff lint found issues"; exit 1; }; \ else echo "ruff not found; skipping py-lint"; fi py-typecheck: @@ -581,13 +603,17 @@ py-typecheck: @# host's PATH when the venv hasn't been provisioned (CI sets @# `VIRTUAL_ENV` and uses PATH-resolved binaries). A pipx-isolated @# system `mypy` can't see the bindings dir's pytest stubs. + @# The smoke harness (`$(SMOKE_LIB_PY)`) is appended to the checked + @# paths so it is type-gated under the same `--strict` posture (#995); + @# mypy with cwd=$(BCA_PY_DIR) discovers the bindings `[tool.mypy]` + @# config and resolves `import big_code_analysis` from the venv. @if [ -x "$(BCA_PY_DIR)/.venv/bin/mypy" ]; then \ echo "Type-checking with mypy --strict (venv)..."; \ - (cd "$(BCA_PY_DIR)" && .venv/bin/mypy --strict python tests examples) || \ + (cd "$(BCA_PY_DIR)" && .venv/bin/mypy --strict python tests examples "$(SMOKE_LIB_PY)") || \ { echo "mypy --strict found issues"; exit 1; }; \ elif command -v mypy >/dev/null 2>&1; then \ echo "Type-checking with mypy --strict..."; \ - (cd "$(BCA_PY_DIR)" && mypy --strict python tests examples) || \ + (cd "$(BCA_PY_DIR)" && mypy --strict python tests examples "$(SMOKE_LIB_PY)") || \ { echo "mypy --strict found issues"; exit 1; }; \ else echo "mypy not found; skipping mypy stage of py-typecheck"; fi @if [ -x "$(BCA_PY_DIR)/.venv/bin/pyright" ]; then \ @@ -625,12 +651,16 @@ py-typecheck: # checkout (target/ is restored from cache but the .so is rebuilt on # every job invocation, not repeated within a single job), and the # editable install dir is never populated before the build step. +# +# Both guards live in one variable, invoked as `@$(PY_EXT_CLEAN)` by every +# maturin-develop target (py-test, py-stubtest, smoke-lib), so a fix really +# does land in one place — the #995 review caught a third copy drifting from +# the "hoisted to one place" promise this comment once made per-target. +PY_EXT_CLEAN := find "$(BASE_DIR)target" -name 'libbig_code_analysis_py*' -delete 2>/dev/null || true; rm -f "$(BCA_PY_DIR)/python/big_code_analysis/"_native*.so + py-test: - @# Pre-build cleanups (see header comment) are hoisted before the - @# if/elif so a future fix to either guard lands in one place; the - @# rm/find are safe no-ops on missing files. - @find "$(BASE_DIR)target" -name 'libbig_code_analysis_py*' -delete 2>/dev/null || true - @rm -f "$(BCA_PY_DIR)/python/big_code_analysis/"_native*.so + @# Pre-build cleanups (see header comment); safe no-ops on missing files. + @$(PY_EXT_CLEAN) @# Prefer the bindings dir's `.venv/bin/{maturin,python}` over the @# host's PATH for the same reason `py-typecheck` does: the venv @# has pytest (declared as a dev-dependency in @@ -660,8 +690,7 @@ py-test: # deliberate facade differences (the `vcs` submodule, runtime # `__all__`); see `big-code-analysis-py/stubtest-allowlist.txt`. py-stubtest: - @find "$(BASE_DIR)target" -name 'libbig_code_analysis_py*' -delete 2>/dev/null || true - @rm -f "$(BCA_PY_DIR)/python/big_code_analysis/"_native*.so + @$(PY_EXT_CLEAN) @if [ -x "$(BCA_PY_DIR)/.venv/bin/maturin" ] && [ -x "$(BCA_PY_DIR)/.venv/bin/python" ]; then \ if "$(BCA_PY_DIR)/.venv/bin/python" -c "import mypy.stubtest" >/dev/null 2>&1; then \ echo "Building extension + running mypy stubtest (venv)..."; \ @@ -678,6 +707,41 @@ py-stubtest: { echo "stubtest found stub/runtime drift"; exit 1; }; \ else echo "maturin or mypy stubtest not found; skipping py-stubtest"; fi +# --------------------------------------------------------------------------- +# Release/wheel smoke harnesses (#995) +# --------------------------------------------------------------------------- +# The wheel workflows (python-wheels.yml, python-cli-wheels.yml) and the +# per-PR smoke-dryrun.yml all invoke the *same* checked-in scripts under +# scripts/smoke/. These targets run them locally against a dev build so a +# stale assertion (the #530 integer-metric / #614 AnalysisFailure drift that +# blocked v2.0.0) surfaces before a tag, not at release time. +smoke: smoke-cli smoke-lib + +# Build a debug `bca` and run the CLI smoke against it — mirrors the +# python-cli-wheels.yml smoke step, which runs the identical script against +# the packaged console binary. +smoke-cli: + @echo "Building bca + running CLI smoke..." + @cargo build -q -p big-code-analysis-cli + @BCA="$(BASE_DIR)target/debug/bca" bash "$(BASE_DIR)scripts/smoke/cli_wheel_smoke.sh" + +# Build the bindings via `maturin develop` and run the library smoke — +# mirrors the python-wheels.yml smoke step (packaged abi3 wheel) and carries +# the same 0-byte-.so pre-build cleanups as py-test. +smoke-lib: + @$(PY_EXT_CLEAN) + @if [ -x "$(BCA_PY_DIR)/.venv/bin/maturin" ] && [ -x "$(BCA_PY_DIR)/.venv/bin/python" ]; then \ + echo "Building extension + running library smoke (venv)..."; \ + (cd "$(BCA_PY_DIR)" && .venv/bin/maturin develop --quiet) && \ + "$(BCA_PY_DIR)/.venv/bin/python" "$(BASE_DIR)scripts/smoke/lib_wheel_smoke.py" || \ + { echo "smoke-lib failed"; exit 1; }; \ + elif command -v maturin >/dev/null 2>&1; then \ + echo "Building extension + running library smoke..."; \ + (cd "$(BCA_PY_DIR)" && maturin develop --quiet) && \ + python "$(BASE_DIR)scripts/smoke/lib_wheel_smoke.py" || \ + { echo "smoke-lib failed"; exit 1; }; \ + else echo "maturin not found; skipping smoke-lib"; fi + # --------------------------------------------------------------------------- # Lint aggregate # --------------------------------------------------------------------------- diff --git a/big-code-analysis-cli/tests/format_smoke.rs b/big-code-analysis-cli/tests/format_smoke.rs index 7e103236..7f840618 100644 --- a/big-code-analysis-cli/tests/format_smoke.rs +++ b/big-code-analysis-cli/tests/format_smoke.rs @@ -310,6 +310,56 @@ fn cli_check_code_climate_output_matches_gitlab_shape() { ); } +/// #995 / #530: integer-valued metrics (counts, sums, min/max) must +/// serialize as JSON *integers* (`3`), never floats (`3.0`). This is the +/// exact invariant the `python-cli-wheels.yml` wheel smoke asserts against +/// the packaged binary — and the one that silently rotted (the smoke still +/// expected the pre-2.0 `"3.0"`) until the `v2.0.0` tag forced the smoke to +/// run (#995). Pinning it in a per-PR test means a regression of the `u64` +/// wire fields (a serde rename, a field-type flip back to `f64`) reds a PR +/// check instead of a release. +/// +/// The round-trip tests below coerce every metric through `as_f64()`, so +/// they pass for both `3` and `3.0` and cannot catch this. The +/// distinguishing check is `is_u64()`: `serde_json` parses `3` as an integer +/// (`is_u64() == true`) but `3.0` as a float (`is_u64() == false`). +#[test] +fn cli_metrics_json_serializes_integer_metrics_as_integers() { + let dir = TempDir::new().unwrap(); + let fixture = write_rust_fixture(&dir); + let out = run_metrics(&dir, "json", &fixture); + let doc: serde_json::Value = serde_json::from_str(&out).expect("metrics JSON parses"); + + // expected: the fixture's unit-level cyclomatic.sum is 3 — the unit's + // own base (1) plus function `f`'s value (entry + `if` = 2); the `else` + // adds nothing under McCabe. loc.sloc is 1. Both are integral, so both + // must serialize as JSON integers, not floats. + let cyclomatic_sum = &doc["metrics"]["cyclomatic"]["sum"]; + assert!( + cyclomatic_sum.is_u64(), + "cyclomatic.sum must serialize as a JSON integer (#530), got {cyclomatic_sum} in:\n{out}", + ); + assert_eq!(cyclomatic_sum.as_u64(), Some(3), "cyclomatic.sum value"); + + let sloc = &doc["metrics"]["loc"]["sloc"]; + assert!( + sloc.is_u64(), + "loc.sloc must serialize as a JSON integer (#530), got {sloc} in:\n{out}", + ); + + // Negative control: a genuinely fractional metric (`halstead.volume`, + // ~58.81 for this fixture) is an `f64` wire field and MUST stay a JSON + // float. This proves `is_u64()` above actually discriminates integers + // from floats rather than passing vacuously — if every metric + // serialized as a float (the pre-#530 regression), this assertion would + // still hold while the integer checks above failed, and vice versa. + let volume = &doc["metrics"]["halstead"]["volume"]; + assert!( + volume.is_f64() && !volume.is_u64(), + "halstead.volume must serialize as a JSON float, got {volume} in:\n{out}", + ); +} + // --- TOML / YAML / CBOR round-trip smoke tests (issue #543) --------------- // // These three formats had no validity / round-trip coverage, so a shape diff --git a/docs/development/lessons_learned.md b/docs/development/lessons_learned.md index 3793e366..e47615cf 100644 --- a/docs/development/lessons_learned.md +++ b/docs/development/lessons_learned.md @@ -4105,3 +4105,58 @@ the cache stays key-agnostic, while the outer digest *includes* it so the emitted value is hardened. --- + +## 80. An assertion that only runs at release time rots silently — mirror it into a per-PR test that actually discriminates + +CI smoke / packaging checks that fire only on a rare trigger (a `v*` tag +push, an opt-in PR label, a scheduled cron) are invisible to the per-PR +`cargo test` / `pytest` suites and to any refactor that updates the real +API. A breaking change lands, its per-PR tests stay green, and the +rarely-run assertion silently rots until the trigger finally fires — at +which point it blocks the very release it was meant to protect. Mirroring +the invariant into the per-PR suite closes the gap only if the mirror +actually *discriminates* the regression: a test that coerces both the old +and the new representation to a common type guards nothing. + +**Three `v2.0.0` smoke assertions rotted between rc1 and the stable tag** +(#995, `6e23d46e`; hot-fixed in `c53e504b`). The wheel / release workflows +(`python-wheels.yml`, `python-cli-wheels.yml`, `release.yml`) only run their +build / smoke matrices on a `v*` tag or an opt-in label, and the assertions +lived as inline shell / Python heredocs inside the workflow YAML — invisible +to `cargo test` / `pytest`. So the #530 integer-serialization change +(`cyclomatic.sum` now emits `3`, not `3.0`) and the #614 +`AnalysisError` → `AnalysisFailure` rename left the smokes asserting the +pre-2.0 strings; both stayed green on every PR and only failed when the +`v2.0.0` tag forced the matrix to run, blocking publication (recovery needed +a force-moved tag + `gh run rerun --failed`). The fix extracts the smokes +into checked-in, lint-gated `scripts/smoke/*` referenced by the workflows, +mirrors the invariants into per-PR tests, and adds a path-filtered +`smoke-dryrun.yml` that runs those scripts against a cheap dev build whenever +the plumbing changes. + +**The per-PR test that should have caught #530 coerced the regression +away** (#995). `format_smoke.rs` already round-tripped `cyclomatic.sum` +across JSON / YAML / TOML / CBOR — but every extractor read the value via +`as_f64()`, which yields `3.0` for both the integer `3` and the float `3.0`. +The round-trip passed identically before and after #530, so it could never +have flagged a `u64` → `f64` wire regression. The discriminating check is +`serde_json::Value::is_u64()` (true for `3`, false for `3.0`), now pinned in +`cli_metrics_json_serializes_integer_metrics_as_integers` with +`halstead.volume` (genuinely fractional) as a negative control proving the +assertion distinguishes integers from floats rather than passing vacuously. + +**Lesson:** A check that runs only on a rare trigger is not a per-PR gate — +treat its load-bearing assertions as *untested* until they are mirrored into +the suite that runs on every PR, and extract embedded-in-YAML scripts so they +are lintable and locally runnable (`make smoke`). The mirror counts only if +it discriminates the regression: when an invariant is about a value's +*representation* (integer vs float, one error type vs another), assert the +property that distinguishes them and prove the discriminating power with a +negative control — coercing both sides to a common type guards nothing. +Related to lesson #15 (code outside the workspace-scoped gates drifts +silently; here it is assertions outside the per-PR-scoped gates) and to +lesson #75 (an assertion that passes under the wrong condition verifies +nothing; here the masking mechanism is type coercion, not grammar +error-recovery). + +--- diff --git a/scripts/smoke/cli_wheel_smoke.sh b/scripts/smoke/cli_wheel_smoke.sh new file mode 100755 index 00000000..3a2c66fb --- /dev/null +++ b/scripts/smoke/cli_wheel_smoke.sh @@ -0,0 +1,99 @@ +#!/usr/bin/env bash +# +# CLI-wheel smoke for the `bca` command-line tool. +# +# Extracted from the inline `run:` block in +# `.github/workflows/python-cli-wheels.yml` so the load-bearing assertions +# are visible to reviewers, lintable (shellcheck), and runnable per-PR / +# locally rather than only on a `v*` tag push (#995). The +# `cyclomatic.sum == "3"` integer-metric assertion silently rotted from the +# pre-2.0 `"3.0"` (#530) precisely because it was buried in workflow YAML +# and only ran when the `v2.0.0` tag forced it. +# +# The binary under test is resolved from, in order: +# 1. $BCA — an explicit path (used by the dev-build dry-run). +# 2. `bca` on PATH — the console script installed from the wheel (the +# workflow's case). +# +# Optional env: +# EXPECTED_TAG — e.g. `v2.1.0`. When set, assert `bca --version` reports +# the tag version (minus the leading `v`). Empty on PR / +# dev runs, where there is no tag to compare against. +# +# Usage: +# bca on PATH: scripts/smoke/cli_wheel_smoke.sh +# explicit binary: BCA=target/release/bca scripts/smoke/cli_wheel_smoke.sh +set -euo pipefail + +bca="${BCA:-bca}" + +ver_out="$("$bca" --version)" +echo "$ver_out" + +# On a tag build, prove the binary reports the lockstep release version +# (maturin reads `version.workspace = true`, so the wheel version must equal +# the tag minus its leading `v`). +# +# The match is a deliberate SUBSTRING, not an exact compare: a git tag and +# the Cargo version can differ in pre-release punctuation (tag `v2.0.0-rc1` +# vs Cargo `2.0.0-rc.1`), and tightening this to `=` would red a legitimate +# rc cut. The lockstep contract only needs the tag's version to *appear* in +# `bca --version` output (`bca `). Keep it lenient. +if [[ -n "${EXPECTED_TAG:-}" ]]; then + want="${EXPECTED_TAG#v}" + case "$ver_out" in + *"$want"*) : ;; + *) + echo "::error::bca --version '$ver_out' does not contain tag version $want" + exit 1 + ;; + esac +fi + +"$bca" list-metrics names >/dev/null + +# Work in a scratch dir so the fixtures never collide with the caller's cwd. +# The template is *relative* (`./...`) on purpose: the python-cli-wheels.yml +# smoke runs on the windows-latest leg under Git Bash, where `bca.exe` is a +# native Windows binary. An absolute MSYS path (`mktemp -d` with no template, +# e.g. `/tmp/tmp.XXXX`) would be handed to that binary unmangled and fail to +# open; a relative path under cwd needs no POSIX->Windows translation. macOS +# and GNU `mktemp` both accept a positional template. +workdir="$(mktemp -d ./bca-smoke.XXXXXX)" +trap 'rm -rf "$workdir"' EXIT + +# Parse two unrelated languages to prove the all-languages grammar set is +# compiled in, not just the host language. +printf 'def add(a, b):\n if a > b:\n return a\n return b\n' >"$workdir/smoke.py" +printf 'fn main() { if true { println!("x"); } }\n' >"$workdir/smoke.rs" + +# `python` is the wheel workflow's interpreter; fall back to `python3` for +# local shells where only the versioned name is on PATH. +py="python" +command -v "$py" >/dev/null 2>&1 || py="python3" + +# `--no-config` keeps the smoke hermetic: the wheel job's sparse checkout +# (and a local `make smoke-cli` from the repo root) would otherwise let bca +# auto-discover the repo's own bca.toml and apply its self-scan config. The +# smoke asserts the binary's *default* behaviour, independent of repo state. +extract_cyclomatic_sum() { + "$bca" metrics --no-config --paths "$1" -O json \ + | "$py" -c "import sys,json; print(json.load(sys.stdin)['metrics']['cyclomatic']['sum'])" +} + +py_cc="$(extract_cyclomatic_sum "$workdir/smoke.py")" +rs_cc="$(extract_cyclomatic_sum "$workdir/smoke.rs")" + +# 2.0 serializes integer-valued metrics as integers (#530), so `json.load` +# yields a Python int and `print` emits `3`, not the pre-2.0 `3.0`. A +# regression that flipped the wire field back to f64 would print `3.0` here +# (and red `cli_metrics_json_serializes_integer_metrics_as_integers`). +test "$py_cc" = "3" || { + echo "::error::python cyclomatic.sum=$py_cc, expected 3" + exit 1 +} +test "$rs_cc" = "3" || { + echo "::error::rust cyclomatic.sum=$rs_cc, expected 3" + exit 1 +} +echo "smoke OK" diff --git a/scripts/smoke/lib_wheel_smoke.py b/scripts/smoke/lib_wheel_smoke.py new file mode 100755 index 00000000..082285c7 --- /dev/null +++ b/scripts/smoke/lib_wheel_smoke.py @@ -0,0 +1,131 @@ +#!/usr/bin/env python3 +"""Library-wheel import + analyse smoke for ``big_code_analysis``. + +Extracted from the inline heredoc in ``.github/workflows/python-wheels.yml`` +so the load-bearing assertions are visible to reviewers, lintable, and +runnable per-PR / locally rather than only on a ``v*`` tag push (#995). +Three of these checks rotted into a release blocker exactly because the +script was buried in workflow YAML: the #614 ``AnalysisError`` -> +``AnalysisFailure`` rename was mirrored in ``tests/test_batch.py`` but not +here, so the smoke only failed when the ``v2.0.0`` tag forced it to run. + +Run it against any importable build of the bindings:: + + maturin develop -m big-code-analysis-py/Cargo.toml + python scripts/smoke/lib_wheel_smoke.py + +The workflow runs the identical file against the freshly built wheel, and +``make smoke-lib`` / the ``smoke-dryrun.yml`` PR job run it against a +``maturin develop`` build — a single source of truth for all three. + +Assertions are deliberately VALUE-bearing, not just shape-bearing: a +binding regression that returned ``{"spaces": []}`` would satisfy a bare +``isinstance(result, dict)`` check but fail the ``records[*]["name"] == +"add"`` check below. Each ``_check_*`` helper pins one public API surface +that #103 enumerates. +""" + +import json +import tempfile +from pathlib import Path + +import big_code_analysis as bca +from big_code_analysis import FuncSpaceDict + +# Shared fixture: one function spanning the file plus one `if` branch. +_ADD_SRC = "def add(a, b):\n if a > b:\n return a\n return b\n" + + +def _check_package_loaded() -> None: + """Package metadata + the language registry survived packaging.""" + assert bca.__version__, repr(bca.__version__) + assert "python" in bca.supported_languages() + # Extensions are bare names (no leading dot) — match the + # `language_extensions` contract in the stub. The CLI used to expose + # dotted forms; the binding does not. + assert "py" in bca.language_extensions("python"), bca.language_extensions("python") + + +def _check_language_resolution() -> None: + """`language_for_file` reads the file (parity with `analyze`, #318).""" + # A stub path that does not exist raises FileNotFoundError, so + # materialise an empty fixture in a tempdir to exercise the extension + # table path rather than crashing on I/O. + with tempfile.TemporaryDirectory() as td: + fixture = Path(td) / "foo.py" + fixture.write_bytes(b"") + resolved = bca.language_for_file(fixture) + assert resolved == "python", resolved + + +def _check_analyze_source() -> FuncSpaceDict: + """`analyze_source` returns a unit dict with non-zero metrics.""" + result = bca.analyze_source(_ADD_SRC, "python") + assert isinstance(result, dict), type(result) + assert result.get("kind") == "unit", result.get("kind") + # The unit-level cyclomatic is 3 for this fixture. A binding regression + # that produced null / zero metrics would slip past `isinstance(result, + # dict)` but trip this assertion. The binding returns Python floats (the + # integer-as-JSON serialization of #530 applies to the CLI's text + # output, asserted in cli_wheel_smoke.sh), so 3.0 is correct. + cyclomatic_sum = result["metrics"]["cyclomatic"]["sum"] + assert cyclomatic_sum == 3.0, f"expected unit cyclomatic.sum == 3.0, got {cyclomatic_sum}" + return result + + +def _check_flatten_spaces(result: FuncSpaceDict) -> None: + """`flatten_spaces` yields one record per space (unit + `add`).""" + records = list(bca.flatten_spaces(result)) + assert records, "flatten_spaces returned no records" + add_records = [r for r in records if r.get("name") == "add"] + assert add_records, f"no 'add' function in records: {[r.get('name') for r in records]}" + assert add_records[0].get("kind") == "function", add_records[0].get("kind") + # The `add` function record's cyclomatic.sum is 2 (1 entry + 1 if). + assert add_records[0].get("cyclomatic.sum") == 2.0, add_records[0].get("cyclomatic.sum") + + +def _check_sarif(result: FuncSpaceDict) -> None: + """`to_sarif` emits a well-formed 2.1.0 document with one tool run.""" + # A stub `{"version": "2.1.0"}` (the most plausible regression of an + # empty-run SARIF writer) would fail the runs check. + sarif_obj = json.loads(bca.to_sarif(result)) + assert sarif_obj.get("version") == "2.1.0", sarif_obj.get("version") + runs = sarif_obj.get("runs") or [] + assert len(runs) == 1, f"expected 1 SARIF run, got {len(runs)}" + driver = runs[0].get("tool", {}).get("driver", {}) + assert driver.get("name"), f"SARIF run missing tool.driver.name: {driver}" + + +def _check_batch_never_raises() -> None: + """`analyze_batch` returns an AnalysisFailure for a bad path, never raises.""" + # A binding regression that switched batch back to raising would crash + # here; one that dropped error records entirely would shrink `len(batch)` + # below the input count and trip the length assertion. + with tempfile.TemporaryDirectory() as td: + good = Path(td) / "good.py" + good.write_text("def f():\n pass\n") + missing = Path(td) / "does-not-exist.py" + batch = bca.analyze_batch([good, missing]) + assert len(batch) == 2, f"expected 2 batch results, got {len(batch)}" + # Good path → dict. + assert isinstance(batch[0], dict), type(batch[0]) + # Missing path → AnalysisFailure with `error_kind == "IoError"` (the + # type was renamed from AnalysisError at 2.0, #614). + err = batch[1] + assert isinstance(err, bca.AnalysisFailure), type(err) + assert err.error_kind == "IoError", err.error_kind + assert str(missing) in err.path, (err.path, str(missing)) + + +def main() -> None: + _check_package_loaded() + _check_language_resolution() + result = _check_analyze_source() + _check_flatten_spaces(result) + _check_sarif(result) + _check_batch_never_raises() + print("smoke OK:", bca.__version__) + + +if __name__ == "__main__": + main()