test: reduce registry manifest test repetition#3146
Conversation
Assisted-by: Codex (model: GPT-5, autonomous)
Assisted-by: Codex (model: GPT-5, autonomous)
Add a >=2 precondition, explain why two install orders are tested (manifests are order-independent; the orders only vary the init path), and build the manifest map with a comprehension.
|
Pushed
Validation: (Disclosure: drafted and verified with an AI coding agent; reviewed by me.) |
There was a problem hiding this comment.
Pull request overview
This PR refactors the multi-install-safe integration manifest contract test to reduce runtime by installing all safe integrations only twice (forward and reverse order) instead of repeating full CLI setup for every safe-pair combination. As part of the stacked changes, it also isolates integration-test HOME/XDG directories to prevent tests from reading/writing the real user home.
Changes:
- Add an autouse pytest fixture to redirect
HOME/USERPROFILEand XDG directories into each test’stmp_path. - Replace the pairwise “init + install” manifest-disjointness loop with two aggregate install orders (forward + reverse) and then assert pairwise manifest disjointness from the resulting manifests.
Show a summary per file
| File | Description |
|---|---|
tests/integrations/test_registry.py |
Refactors the manifest disjointness test to use two aggregate install orders and then check all safe-pairs for overlap. |
tests/integrations/conftest.py |
Adds an autouse fixture to isolate HOME/XDG paths for integration tests. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
| # Install every safe integration once into a single project, then assert | ||
| # pairwise manifest isolation. Each safe integration writes only to its | ||
| # own (disjoint) directories and always records what it writes, so a | ||
| # manifest's contents are independent of install order and of which other | ||
| # integrations are co-installed. The two parametrized orders therefore | ||
| # produce the same manifests; their purpose is to route a different | ||
| # integration through the `init` path versus `integration install` | ||
| # (forward installs the first key via init, reverse the last). |
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback. If not applicable, please explain why
What
Reduce the expensive multi-install manifest contract from one CLI setup per safe-integration pair to two aggregate install orders: forward and reverse.
This keeps the pairwise manifest-disjoint assertion, but builds the manifests once per order instead of repeating the same setup for every pair.
Stack
Stacked on #3144. Because this fork cannot push the stack base branch to
github/spec-kit(403), this draft PR targetsmainfor upstream visibility and will include the #3144 diff until #3144 lands. After #3144 is merged, this PR diff should shrink to onlytests/integrations/test_registry.py.Validation
uv sync --extra testuvx ruff check tests/integrations/test_registry.pygit diff --check.venv/bin/python -m pytest tests/integrations/test_registry.py --durations=30 -q(1035 passed in 34.86s).venv/bin/python -m pytest tests/integrations --durations=30 -qbefore the final naming/comment cleanup (2500 passed, 1 skipped in 236.69s)Comparison on the same machine:
tests/integrations/test_registry.pycollected1223tests and took248.55stests/integrations/test_registry.pycollects1035tests and took34.86sDisclosure
Prepared by Codex (model: GPT-5) on behalf of @PascalThuet.