chore: remove redundant baseapp_core pytest plugin - #450
Conversation
The single-test auto-serialize hook is now applied per-project in each consuming repo's conftest.py, so pytest-xdist adoption is self-contained and works on repos pinning older baseapp-backend. The baseapp_core pytest11 plugin was a no-op for baseapp-backend itself (its suite runs serial) and redundant/shadowed in consuming repos — drop it + the entry point, and point the skill wording at the project conftest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WalkthroughThe change removes the ChangesTargeted pytest execution
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR removes the redundant baseapp_core pytest auto-loaded plugin from the monorepo and updates internal agent documentation to reflect that the “targeted test runs go serial” behavior is now handled per consuming project instead of via a shared pytest11 entry point.
Changes:
- Dropped the
pytest11entry point forbaseapp_corefrompyproject.toml. - Removed the now-unused
baseapp_core/pytest_plugin.pyimplementation. - Updated the
run-development-commandsagent skill wording to reference project-levelconftest.pybehavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pyproject.toml | Removes the pytest11 entry point that previously auto-loaded the baseapp_core pytest plugin. |
| baseapp_core/pytest_plugin.py | Deletes the shared pytest hook that auto-serialized targeted node-id runs. |
| .agents/skills/run-development-commands/SKILL.md | Updates guidance text to match the new per-project conftest.py approach. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| baseapp_reactions = "baseapp_reactions.plugin:ReactionsPlugin" | ||
| baseapp_blocks = "baseapp_blocks.plugin:BlocksPlugin" | ||
| baseapp_content_feed = "baseapp.content_feed.plugin:ContentFeedPlugin" | ||
| baseapp_chats = "baseapp_chats.plugin:ChatsPlugin" | ||
|
|
||
| [project.entry-points.pytest11] | ||
| baseapp_core = "baseapp_core.pytest_plugin" | ||
|
|
||
| [tool.black] |
| **Parallel-aware.** When a project enables xdist (`-n <N> --dist loadscope` in its pytest `addopts` — `pytest.ini` or `setup.cfg`), `pytest` runs in parallel by default, matching CI so parallel-only failures reproduce locally; each worker uses its own test DB. Tests must be hermetic (see the `ensure-test-coverage` skill). | ||
| - Debug one test serially (pdb works): `docker compose <run> web pytest -n 0 apps/<app>/tests/test_file.py::test_function` — or just pass a `path::test` node id; `baseapp_core`'s pytest plugin auto-serializes targeted runs. | ||
| - Debug one test serially (pdb works): `docker compose <run> web pytest -n 0 apps/<app>/tests/test_file.py::test_function` — or just pass a `path::test` node id; the project's `conftest.py` auto-serializes targeted runs. | ||
| - Use all cores on a big machine: `docker compose <run> web pytest -n auto`. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/run-development-commands/SKILL.md:
- Line 68: Update the debug-test command in the development skill guidance to
explicitly require pytest’s “-n 0” option for serial execution, and remove the
claim that targeted runs are automatically serialized by conftest.py. Keep the
existing Docker Compose and test-node examples while ensuring the documented
command cannot inherit parallel xdist settings.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1083e3da-bb96-4ead-9e85-7b7aa34c0153
📒 Files selected for processing (3)
.agents/skills/run-development-commands/SKILL.mdbaseapp_core/pytest_plugin.pypyproject.toml
💤 Files with no reviewable changes (2)
- pyproject.toml
- baseapp_core/pytest_plugin.py
|
|
||
| **Parallel-aware.** When a project enables xdist (`-n <N> --dist loadscope` in its pytest `addopts` — `pytest.ini` or `setup.cfg`), `pytest` runs in parallel by default, matching CI so parallel-only failures reproduce locally; each worker uses its own test DB. Tests must be hermetic (see the `ensure-test-coverage` skill). | ||
| - Debug one test serially (pdb works): `docker compose <run> web pytest -n 0 apps/<app>/tests/test_file.py::test_function` — or just pass a `path::test` node id; `baseapp_core`'s pytest plugin auto-serializes targeted runs. | ||
| - Debug one test serially (pdb works): `docker compose <run> web pytest -n 0 apps/<app>/tests/test_file.py::test_function` — or just pass a `path::test` node id; the project's `conftest.py` auto-serializes targeted runs. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify that every consuming conftest.py exposes the targeted-run serialization hook.
fd -t f '^conftest\.py$' . | while IFS= read -r file; do
rg -n -C 4 'pytest_(collection_modifyitems|configure)|serialize|serial|xdist|baseapp_core\.tests\.fixtures' "$file" || true
done
# Verify whether the imported shared module defines the hook.
rg -n -C 5 'pytest_(collection_modifyitems|configure)|serialize|serial' \
baseapp_core testproject --glob '*.py' || trueRepository: silverlogic/baseapp-backend
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the shared test fixtures module and inspect pytest hook exports.
fd -t f '^fixtures\.py$|^__init__\.py$' baseapp_core | rg 'baseapp_core/tests/(fixtures|__init__|plugins/tests/fixtures|plugins/tests/__init__|plugins/tests)/' || true
# Inspect the relevant test fixtures files with line counts and targeted content.
for f in $(fd -t f -e py 'fixtures\.py$|__init__\.py$' baseapp_core/testproject | rg 'baseapp_core/(tests|plugins/tests)/'); do
echo "===== $f ====="
wc -l "$f"
rg -n -C 8 '__all__|pytest_|xdist|serial|serialize|plugin' "$f" || true
done
# List pytest_hookimpl registrations in baseapp_core/tests and baseapp_core/plugins/tests modules.
python3 - <<'PY'
import ast, pathlib
for p in pathlib.Path('baseapp_core').rglob('*.py'):
if 'tests' not in p.parts and 'plugins' not in p.parts:
continue
text = p.read_text()
if 'pytest_' in text or 'xdist' in text or 'serial' in text:
try:
tree = ast.parse(text)
except Exception:
continue
hits = []
for node in ast.walk(tree):
s = ast.get_source_segment(text, node) or ''
if any(tok in s for tok in ['pytest_configure', 'pytest_collection_modifyitems', 'pytest_collection', 'pytest_itemcollected', 'hookimpl', 'xdist', 'serial', 'serialize']):
hits.append(node.lineno)
if hits and ('pytest' in str(p) or 'tests' in str(p)):
print(f'{p}: {hits}')
PYRepository: silverlogic/baseapp-backend
Length of output: 487
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## candidate fixtures files"
fd -t f 'fixtures\.py$|__init__\.py$' baseapp_core/testproject | sort
echo "## content of fixtures under baseapp_core and testproject"
while IFS= read -r f; do
case "$f" in
*/fixtures.py|*/__init__.py)
if rg -q 'pytest_|xdist|serial|serialize|hookimpl' "$f" || [[ "$(basename "$f")" == "fixtures.py" || "$(basename "$(dirname "$f")")" == "tests" ]]; then
echo "===== $f ====="
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
fi
;;
esac
done < <(fd -t f 'fixtures\.py$|__init__\.py$' baseapp_core/testproject | sort)
echo "## focused pytest hook text"
rg -n -C 6 'pytest_(configure|collection_modifyitems|collection|itemcollected)|hookimpl|autouse|xdist|serial|serialize|skip' baseapp_core/baseapp_core/testproject --glob '*.py' | head -n 240 || true
echo "## all pytest_hookimpl decorators"
rg -n -C 3 '@.*hookimpl|pytest_configure|pytest_collection_modifyitems|pytest_collection' baseapp_core testproject --glob '*.py' || trueRepository: silverlogic/baseapp-backend
Length of output: 305
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## root files matching fixtures"
find . -path './.git' -prune -o \( -path '*/baseapp_core/tests/*' -o -path '*/baseapp_core/plugins/tests/*' -o -path '*/testproject/*' \) \( -name 'fixtures.py' -o -name '__init__.py' \) -print | sort
echo "## fixtures.py files inspected"
while IFS= read -r f; do
if [[ "$(basename "$f")" == "fixtures.py" ]]; then
echo "===== $f ====="
wc -l "$f"
sed -n '1,260p' "$f" | cat -n
fi
done < <(find . -path './.git' -prune -o \( -path '*/baseapp_core/tests/*' -o -path '*/baseapp_core/plugins/tests/*' -o -path '*/testproject/*' \) -name 'fixtures.py' -print | sort)
echo "## hook registrations and xdist/serial keywords"
rg -n -C 6 'pytest_(configure|collection_modifyitems|collection|itemcollected)|hookimpl|autouse|xdist|serial|serialize|skip|mark' . --glob '*.py' --glob '!*/migrations/*.py' | head -n 400 || trueRepository: silverlogic/baseapp-backend
Length of output: 2347
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## fixtures.py contents"
for f in ./baseapp_core/tests/fixtures.py ./baseapp_core/plugins/tests/fixtures.py; do
if [[ -f "$f" ]]; then
echo "===== $f ====="
wc -l "$f"
sed -n '1,260p' "$f" | cat -n
fi
done
echo "## hook registrations and xdist/serial keywords in Python"
rg -n -C 6 'pytest_(configure|collection_modifyitems|collection|itemcollected)|hookimpl|autouse|xdist|serial|serialize|skip' --glob '*.py' --glob '!*/migrations/*.py' . | head -n 400 || true
echo "## deterministic import/export probe"
python3 - <<'PY'
import ast, pathlib
for p in [pathlib.Path('baseapp_core/tests/fixtures.py'), pathlib.Path('baseapp_core/plugins/tests/fixtures.py')]:
text = p.read_text()
tree = ast.parse(text)
all_ = getattr(next((n for n in tree.body if isinstance(n, ast.Assign) and
any(isinstance(t, ast.Name) and t.id == '__all__' for t in n.targets)), None), 'value', None)
names = []
if isinstance(all_, ast.List):
names = [elt.s for elt in all_.elts if isinstance(elt, ast.Str)]
print(p)
print(' __all__:', names)
print(' pytest hooks:')
for n in tree.body:
src = ast.get_source_segment(text, n) or ''
if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) and n.name.startswith('pytest_'):
print(f' {n.lineno}: {n.name}')
print(f' all names: {names}')
print(f' hooked by module import: {n.name in names}')
PYRepository: silverlogic/baseapp-backend
Length of output: 43414
Remove the implied pytest-serial setup from the debug guidance.
The shared fixtures do not expose a pytest hook that disables xdist for targeted runs, so a standalone pytest tests/foo.py::test_bar can still inherit parallel flags. Require -n 0 in this command, or document the actual pytest-serial dependency and hook separately.
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 1: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'run tests' conflicts with built-in command 'run'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
[warning] 2: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'run server' conflicts with built-in command 'run'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
[warning] 5: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'run lint' conflicts with built-in command 'run'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
[warning] 6: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'format code' conflicts with built-in command 'format'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
[warning] 7: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'open shell' conflicts with built-in command 'open'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
[warning] 12: [TR2] Shadow Command Trigger: Shadow Command Trigger: 'install package' conflicts with built-in command 'install'
Remediation: Choose triggers that do not conflict with built-in commands or other skills. Prefix with a unique namespace if necessary.
(Trigger Abuse (TR2))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/run-development-commands/SKILL.md at line 68, Update the
debug-test command in the development skill guidance to explicitly require
pytest’s “-n 0” option for serial execution, and remove the claim that targeted
runs are automatically serialized by conftest.py. Keep the existing Docker
Compose and test-node examples while ensuring the documented command cannot
inherit parallel xdist settings.
The single-test auto-serialize hook is now applied per-project in each consuming repo's conftest.py, so pytest-xdist adoption is self-contained and works on repos pinning older baseapp-backend. The baseapp_core pytest11 plugin was a no-op for baseapp-backend itself (its suite runs serial) and redundant/shadowed in consuming repos — drop it + the entry point, and point the skill wording at the project conftest.
Summary by CodeRabbit
Bug Fixes
Documentation