Make CI able to catch a missing runtime dependency - #22
Merged
Conversation
Closes #21. PR #20 fixed a crash on first run: cli.py imports click, typer 0.26.8 dropped click from its dependencies, and a runtime-only install had no supplier for it. Every CI job stayed green throughout, because every job installs .[dev] and black depends on click>=8.0.0. A user found it instead, on the first command they typed. Three changes so that class of bug fails here rather than there. packaging/check_deps.py compares third-party imports under src/ against project.dependencies. It checks the declaration rather than the installed environment, so it catches imports that a dev tool happens to satisfy, and imports that are unreachable at module scope. Verified by reproducing the pre-#20 state: with click removed from pyproject it exits 1 and names cli.py:13; restored, it exits 0. A runtime-deps CI job installs with a plain `pip install .`, imports every module in the package, and runs the entry point. That is the environment a user actually gets, which no existing job reproduced. `import click` moves from inside _run_setup_wizard to module scope. As a function-local import it failed halfway through the wizard, after the menu had printed, and no import sweep could reach it. At module scope it fails at import time, which is both louder and detectable by the job above. check_deps.py reads pyproject as utf-8-sig: with plain utf-8, a BOM left by an editor makes it fail with a TOML parse error instead of answering the question it was asked. Found while testing the failure path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #21.
#20 fixed a crash on first run:
cli.pyimports click, typer 0.26.8 dropped click from its dependencies, and a runtime-only install had no supplier for it.Every CI job stayed green the whole time, because every job installs
.[dev]andblackdepends onclick>=8.0.0. The test suite, ruff, mypy and three adversarial review passes all missed it. A user found it on the first command they typed.Three changes so this class of bug fails here instead.
packaging/check_deps.pyCompares third-party imports under
src/againstproject.dependencies. It checks the declaration, not the installed environment, so it catches:Verified against the real bug by reproducing the pre-#20 state:
Guard code that has never been shown to fire is just decoration, so this one has.
runtime-depsCI jobInstalls with a plain
pip install .(no[dev]), imports every module in the package, and runs the entry point. That is the environment a user actually gets, and no existing job reproduced it.import clickmoves to module scopeAs a function-local import it failed halfway through the wizard, after the menu had already printed, and no import sweep could reach it. At module scope it fails at import time: louder, and detectable by the job above.
Also
check_deps.pyreads pyproject asutf-8-sig. With plainutf-8, a BOM left by an editor makes it die with a TOML parse error instead of answering the question. Found while testing the failure path, which is the sort of thing you only hit by actually running the negative case.Testing
ruff clean, mypy clean, 173 tests pass,
import game_save_genie.cliresolves click at module scope.Not addressed here, from #21: fork PRs sit at
action_requiredand get no CI signal. #20 was merged after local verification. Still worth a policy decision.