Run the tests once without the CLI extras - #128
Merged
Conversation
The test job runs `tox -e default`, which installs the cli extra, so nothing on a pull request ever exercised an install without click or rich. release.yml installs plain .[testing]. A test module that imported click above its ImportError guard therefore passed every PR check and failed only in the release workflow - after the v9.3.1 tag had been pushed, with the publish job gated behind it. This adds a job that performs the release workflow's install and runs pytest, so that class of failure surfaces on the pull request. It asserts click is genuinely absent first, so the job cannot quietly stop testing what it claims to if the extras change. Build now waits on it. Also drops a dead step from the test job: it pip-installed .[testing] after tox had already run the tests, so it tested nothing. Verified in a venv built the same way: 689 passed, 5 skipped. Claude-Session: https://claude.ai/code/session_01XVj9BYvuLj7Th3iFUVoeCn
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new job’s documentation and install/test invocation should be aligned with the release workflow to ensure it truly mirrors the intended release-path behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds CI coverage for the “release-style” installation path that omits the cli extra, so PR checks can catch tests (or test imports) that accidentally require click/rich even when they should be optional.
Changes:
- Adds a new
Test without CLI extrasjob that installs.[testing], assertsclickis absent, and runspytest. - Updates the
buildjob to depend on the new job in addition tolintandtest. - Removes a dead post-
toxinstall step from thetestjob (it previously installed after tests had already run).
File summaries
| File | Description |
|---|---|
| .github/workflows/ci.yml | Adds a new CI job to run pytest without CLI extras and gates build on its success. |
Review details
Suppressed comments (1)
.github/workflows/ci.yml:94
- This job is intended to replicate the release workflow install, but it currently does a non-editable install (
pip install '.[testing]') whilerelease.ymluses an editable install. Using the same install mode helps ensure the job catches the same class of failures as the release workflow.
python -m pip install --upgrade pip
python -m pip install '.[testing]'
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+75
to
+79
| # install without click or rich. release.yml installs plain | ||
| # .[testing], and a test module importing click above its | ||
| # ImportError guard failed there and nowhere else, after the release | ||
| # tag was already pushed. This job runs the release workflow's | ||
| # install so that failure surfaces on the pull request instead. |
Owner
Author
There was a problem hiding this comment.
Fixed in 706c03c — the job now runs python -m pytest.
release.yml installs -e ".[testing]"; this job installed it non-editably and described it as a plain install. Match the install mode, correct the comment, and invoke pytest through the interpreter actions/setup-python configured. Claude-Session: https://claude.ai/code/session_01YDSWT2RqH9T5r7Q4ZUsaEK
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.
Why
The
testjob runstox -e default, andtox.inilistsextras = cli, testing— so click and rich are always present on a pull request.release.ymlinstalls-e ".[testing]", which has neither.That gap has already cost a release. A test module in #122 imported
click.testingabove thetry/except ImportErrorguard that lets it skip without the CLI extras. Every PR check passed. The v9.3.1 release workflow then failed atRun testswithModuleNotFoundError: No module named 'click'— after the tag was pushed, with the publish job gated behind it. Recovering meant fixing the import, deleting the tag and re-pushing it.Nothing on a PR can currently catch that.
What
A
Test without CLI extrasjob that performs the release workflow's install and runs pytest.buildnow waits on it alongsidelintandtest.It verifies click is genuinely absent before running, so if the extras are ever restructured such that click arrives anyway, the job fails loudly rather than quietly going back to testing the same thing as
test.Also drops a dead step from
test: it pip-installed.[testing]aftertoxhad already run the tests, so it tested nothing.Verified
In a venv built the same way (
pip install -e '.[testing]', no click): 689 passed, 5 skipped — the CLI modules skip as designed. Against the pre-fix import, the same env produced the collection error the release hit.