Fix pytest 9.1 plugin import: marks on fixtures are now an error#537
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #537 +/- ##
==========================================
+ Coverage 58.51% 60.98% +2.47%
==========================================
Files 40 40
Lines 6523 6531 +8
Branches 1099 1101 +2
==========================================
+ Hits 3817 3983 +166
+ Misses 2176 1951 -225
- Partials 530 597 +67 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
Author
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
1 similar comment
Member
Author
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
why: pytest 9.1 rejects marks applied to fixture functions and raises the error at plugin import, before collection. Because libvcs ships this plugin as an installed pytest11 entry point, the error aborted the entire test session for any downstream project (e.g. vcspull) running pytest 9.1+. The skipif marks stacked on the fixtures were no-ops in every prior pytest version; the real gating must run inside each fixture so a missing binary skips instead of erroring. what: - Add private _skip_if_git_missing / _skip_if_svn_missing / _skip_if_hg_missing helpers that call pytest.skip() when the binary is absent; the svn helper requires both svn and svnadmin - Remove the @skip_if_*_missing decorators from the affected fixtures and call the matching helper as the first body statement - Fold empty_svn_repo's existing inline svn/svnadmin guard into the shared helper - Keep the public skip_if_*_missing marks: they remain valid for decorating test functions and importable by downstream suites
why: Tell downstream users that the plugin now loads on pytest 9.1+, so they can upgrade pytest without the session aborting before collection. what: - Add a Fixes entry under the 0.43.x unreleased block
tony
force-pushed
the
pytest-9.1-deprecations
branch
3 times, most recently
from
dcecb41 to
87b487f
Compare
Member
Author
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
tony
force-pushed
the
pytest-9.1-deprecations
branch
from
87b487f to
cb14f80
Compare
Member
Author
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
why: The entry narrated implementation mechanism (how the checks moved, that pytest 9.1 rejects marks on fixtures) -- diff narration in a shipped artifact per AGENTS.md AI Slop Prevention -- and scoped the impact to fixture users. The plugin loads at pytest startup for any project with libvcs installed, so the abort reaches more than fixture users. what: - Drop the mechanism clauses; keep the user-visible outcome - Broaden the audience to any project with libvcs installed
…inary why: vcs_gitconfig, set_vcs_gitconfig, vcs_hgconfig and set_vcs_hgconfig only write a .gitconfig / .hgrc or set env vars -- they never invoke git or hg. Guarding them on the binary made them skip when it was absent. Because conftest.py's autouse `setup` fixture depends on vcs_gitconfig, a host without git would skip the ENTIRE suite, including svn- and hg-only tests that don't need git at all. The binary-invoking fixtures (empty_git_repo, git_remote_repo, ...) keep their guards, so tests that actually run a binary still skip correctly. what: - Remove _skip_if_git_missing() from vcs_gitconfig and set_vcs_gitconfig - Remove _skip_if_hg_missing() from vcs_hgconfig and set_vcs_hgconfig
why: add_doctest_fixtures declared the per-VCS repo factories (create_git_remote_repo, create_svn_remote_repo, create_hg_remote_repo, git_repo) as parameters, so pytest instantiated all of them before the body's `if shutil.which(...)` guards ran. Each factory pulls in a binary-invoking fixture, so a single missing binary (e.g. hg) skipped the whole fixture and therefore every doctest -- including doctests for the binaries that were present. what: - Drop the per-VCS fixtures from the signature; request them via request.getfixturevalue() inside each `if shutil.which(...)` block - A missing binary now drops only that VCS's doctest helpers; doctests for the present binaries still run
why: The svn fixtures build repositories with `svnadmin`, and _skip_if_svn_missing() treats svn as available only when both the svn client and svnadmin are present. The public skip_if_svn_missing mark and the pytest_ignore_collect hook checked the svn client alone, so on a host with svn but not svnadmin they disagreed with the fixtures: a mark-decorated test would run, and svn doctest modules would be collected while their create_svn_remote_repo namespace helper -- which also requires svnadmin -- was withheld, erroring with NameError instead of skipping. Make every svn-availability check require svnadmin. what: - Add the svnadmin check to the skip_if_svn_missing mark condition - Require svnadmin in pytest_ignore_collect so svn modules are ignored (not collected) when only the svn client is present
why: git 2.54 changed `git remote set-head` output: when origin/HEAD
already points at the branch it prints "'origin/HEAD' is unchanged and
points to 'master'" instead of "origin/HEAD set to master". The doctest
asserted the older literal string, so it failed under the git on CI.
The failure is independent of the pytest 9.1 fixture changes -- it was
latent until the plugin could import and the suite actually ran.
what:
- Assert version-stable invariants instead of the literal message:
`'master' in set_head(auto=True)` and that `set_head('master')`
returns a str
tony
force-pushed
the
pytest-9.1-deprecations
branch
from
cb14f80 to
190dcbb
Compare
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
1 participant
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.
Summary
libvcs.pytest_pluginnow imports cleanly under pytest 9.1, which promoted "marks applied to fixtures" from a deprecation warning to a hard error. Because the plugin ships as apytest11entry point, the error fired at plugin import — aborting the entire test session before collection for any project that has libvcs installed and runs pytest 9.1+ (e.g. vcspull).@skip_if_*_missingmark decorators stacked on 16 Git/Mercurial/Subversion fixtures with in-bodypytest.skip()guards (_skip_if_git_missing/_skip_if_svn_missing/_skip_if_hg_missing). Applying askipifmark to a fixture never had any effect; moving the check into the fixture body makes a missing binary skip dependent tests instead of erroring.skip_if_git_missing/skip_if_svn_missing/skip_if_hg_missingmarks unchanged — they remain valid for decorating test functions and importable by downstream suites.Design decisions
conftest.py), libvcs's fixtures actually invoke the VCS binaries, so deleting the marks alone would turn "skip when missing" into a fixture error. The repo already used this exact in-body pattern inempty_svn_repo; this generalizes it and folds that fixture's inline guard into the shared helper.svnandsvnadmin: the svn fixtures callsvnadmin, matching the pre-existing inline guard's semantics.Verification
No mark decorators remain on fixtures:
$ rg '^\s*@skip_if_(git|svn|hg)_missing' src/libvcs/pytest_plugin.pyThe plugin imports without raising:
$ uv run python -c "import libvcs.pytest_plugin"Fixtures skip (not error) when a binary is absent:
$ PATH="$PWD/.venv/bin:/tmp/nosvn_bin" .venv/bin/python -m pytest tests/sync/test_svn.py -rsTest plan
uv run python -c "import libvcs.pytest_plugin"— plugin imports cleanly under pytest 9.1uv run pytest— full suite greenuv run ruff check .— lint cleanuv run ruff format . --check— formatting cleanuv run mypy— type check cleanSKIPPED ... svn is not available(not an error) whensvn/svnadminare hidden fromPATH