davidvujic Skill: polylith-migrate-refactor-tests
Goal
Restructure unit tests so they live next to the brick they test, following the workspace's Polylith theme (loose or tdd).
Scope
- Unit tests only. Integration tests typically stay in a shared location (e.g.,
test/integration/ortest/<project>/integration/). Do not redistribute them across bricks. - Structure only. Reorganize test files and update imports/mock patch strings. Do not rewrite test logic or fixtures unless an import/path change makes it strictly necessary.
Shared test helpers & the namespace-merge hazard (read before moving)
Two real traps when moving unit tests next to their bricks:
- Shared test-support packages stop resolving. If tests import shared
helpers/fixtures by package (e.g.
from <svc>_service.helpers import …,from <svc>_service.fixtures import …), those resolve today only because the service test dir sits onsys.path(pytest inserts the topmost non-package dir). Once a test moves totest/components/<TARGET_TOP_NS>/<comp>/(no__init__.py), pytest inserts the leaf test dir instead, and the shared import breaks. - Do NOT "fix" it by adding
test/topythonpath. That makestest/<…>/<TARGET_TOP_NS>/<brick>/directories merge into the real<TARGET_TOP_NS>namespace and collide withcomponents/<TARGET_TOP_NS>/<brick>/bases/<TARGET_TOP_NS>/<brick>— an ambiguous, hard-to-debug namespace package.
Two sanctioned layouts — pick one
- Per-brick (the target table below). Use it after making shared helpers
reachable without putting
test/on the path: convert them toconftest.pyfixtures (pytest injects fixtures with no import), or move pure helper functions to an on-path support location. Unit tests then live undertest/<theme>/<TARGET_TOP_NS>/<brick>/. - Workspace-level service dir. Keep the project's tests under a single
test/<svc>_service/directory (the importable, valid-package name thatpolylith-migrate-prepare-projectcreated). Shared helpers stay co-located and importable. Choose this when converting helpers to fixtures isn't worth it; record the choice instate.md. This still satisfies "tests at workspace level".
If unsure, the workspace-level service dir is the lower-risk default.
Inputs
From migration/<PROJECT>/state.md:
TARGET_TOP_NSRUN_TEST_CMD(will be updated by this skill)RUN_LINT_CMD,RUN_TYPECHECK_CMD(optional)
From migration/<PROJECT>/manifest.md:
- List of all bricks (bases and components) with their module maps.
From workspace.toml:
[tool.polylith.structure].theme(looseortdd).
From test/:
- Current test directory structure.
All inputs from
state.mdare assumed to satisfy the validation rules inpolylith-migrate-discover(### Validation rules). Validate before proceeding.
Target layout
| Theme | Test path for a base | Test path for a component |
|---|---|---|
loose |
test/bases/<TARGET_TOP_NS>/<base>/test_*.py |
test/components/<TARGET_TOP_NS>/<component>/test_*.py |
tdd |
bases/<base>/test/<TARGET_TOP_NS>/<base>/test_*.py |
components/<component>/test/<TARGET_TOP_NS>/<component>/test_*.py |
Steps
1. Classify each unit test file
For each test_*.py under the current test root:
- Read its
importstatements andmock.patch("...")strings. - Identify the primary brick under test — usually the one most imported or the one mock-patched. Tie-break by file name (
test_user_handler.py→<user_handler base or component>). - Record the classification in a table you keep in scratch (do not commit a separate file for this — it's transient):
| Test file | Primary brick | Target path |
|---|
If a test exercises 2+ bricks at integration level, classify it as integration and move it to test/integration/ instead.
2. Move test files
- Create the target directories per the theme matrix above.
- Move each
test_*.pyto its brick's test directory. - For each
conftest.py:- Brick-scoped fixtures (referenced only by tests under one brick) → move to that brick's test directory.
- Workspace-shared fixtures (cross-brick) → keep one
test/<TARGET_TOP_NS>/conftest.pyortest/conftest.py.
3. Update imports and mock patch strings
- Update any
from tests.<x>imports that survived to the new layout. - Update
mock.patch("<old.path>")strings. Brick reshuffling earlier in the migration may have changed where the patched symbol now lives — usegrepto locate the target symbol's new path and align the patch string. Patching a wrong path silently succeeds and the test will pass for the wrong reason — verify by intentionally breaking the target function and checking that the test fails.
4. Update RUN_TEST_CMD
- Set
RUN_TEST_CMDinstate.mdto a command that collects from the new test root (typically<POLY_CMD_PREFIX_RUN> pytest test/forloose, or per-brick collection fortdd). - Verify the test count after the move equals the baseline recorded in
polylith-migrate-discover. A drop in collected tests means files were lost or pytest discovery is misconfigured.
Verify
RUN_TEST_CMDsucceeds and collects the same number of tests as the baseline frompolylith-migrate-discover.- If set,
RUN_LINT_CMDsucceeds. POLY_CMD_PREFIX checkis still green.
Common failure modes
| Symptom | Likely cause | Remediation |
|---|---|---|
pytest collects fewer tests than before |
The old test root is no longer on pytest's path; or duplicate conftest.py files silently shadow each other. |
Update [tool.pytest.ini_options].testpaths (or pass paths explicitly in RUN_TEST_CMD). Run pytest --collect-only and diff against baseline collection. |
fixture '<name>' not found |
The fixture was in a conftest.py you moved into a brick's test dir; tests in another brick can no longer see it. |
Either move the fixture up to a higher-scope conftest.py, or duplicate it (only if cheap). Don't import fixtures across conftest.py files. |
Tests pass but assert nothing useful (mock.patch no longer hits anything) |
Patch string still points at the pre-migration module path. | Re-derive the patch path: <TARGET_TOP_NS>.<brick_name>.<module>.<symbol>. Validate by deliberately breaking the patched function and confirming the test fails. |
ImportError: cannot import name 'fixture_<x>' from conftest.py |
A conftest.py imports a moved test helper module that didn't follow it. |
Move the helper next to the new conftest.py, or import it from its new brick path. |
| Tests for moved code suddenly find themselves under a brick name that doesn't match their content | Misclassification in step 1. | Re-read the test's imports — the brick most imported is the one that owns the test. Move and update. |
ModuleNotFoundError: No module named '<svc>_service' after moving a test to test/<theme>/<TARGET_TOP_NS>/<brick>/ |
Shared test-support package is no longer on sys.path. |
Use a sanctioned layout (above): convert the helpers to conftest fixtures, or keep tests in the workspace-level <svc>_service/ dir. Do not add test/ to pythonpath (namespace merge with the real bricks). |
| Verification fails and you can't quickly diagnose | Phase commit not yet made. | git reset --hard HEAD to roll back to the previous phase's commit and consult the user. |
Commit
After verification passes, commit this phase to the migration branch:
git add -A && git commit -m "migrate(<PROJECT>): phase <N> — refactor-tests"
Substitute <PROJECT>, <N>, and <phase-name> from state.md and the orchestrator's phase table. Do not proceed to the next phase without a clean commit — the per-phase commit is the rollback point for the next phase's failure-mode tables.