test: reduce registry manifest test repetition by PascalThuet · Pull Request #3146 · github/spec-kit

PascalThuet · 2026-06-24T08:04:29Z

What

Reduce the expensive multi-install manifest contract from one CLI setup per safe-integration pair to one aggregate setup per multi-install-safe rotation.

Each rotation places a different safe integration first, so every safe integration still exercises the specify init --integration ... path at least once. The test then reuses the resulting manifests for the pairwise manifest-disjoint assertions instead of repeating equivalent setup for every pair.

Stack

Stacked on #3144. Because this fork cannot push the stack base branch to github/spec-kit (403), this PR targets main for upstream visibility and will include the #3144 diff until #3144 lands. After #3144 is merged, this PR diff should shrink to only tests/integrations/test_registry.py.

Validation

git diff --check
.venv/bin/python -m pytest tests/integrations/test_registry.py -q (443 passed in 15.18s)

Earlier validation before the final merge from origin/main:

uv sync --extra test
uvx ruff check tests/integrations/test_registry.py
.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 -q before the final naming/comment cleanup (2500 passed, 1 skipped in 236.69s)

Disclosure

Prepared by Codex (model: GPT-5) on behalf of @PascalThuet.

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.

PascalThuet · 2026-06-24T08:37:02Z

Pushed ca07e0e for the review feedback on test_safe_integrations_have_disjoint_manifests. The assertion is unchanged; this is clarity plus a guard.

Documented why both orders run. Manifest contents don't depend on install order: each safe integration writes only to its own disjoint directories and always records what it writes, so installing the whole set into one project gives the same per-integration manifests either way. forward/reverse doesn't vary the manifests; it just routes a different integration through init vs integration install (forward inits the first key, reverse the last). The old comment didn't say this and read as if order mattered.
Added a len(ordered_keys) >= 2 precondition. With fewer than two safe integrations the contract is vacuous and ordered_keys[0] would fail on an empty set, so a shrunken registry now fails here instead of passing silently.
Built the manifest map with a comprehension and pulled out integrations_dir. Cosmetic.

Validation: ruff check clean; pytest tests/integrations/test_registry.py -> 1035 passed in 6.48s.

(Disclosure: drafted and verified with an AI coding agent; reviewed by me.)

Copilot

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/USERPROFILE and XDG directories into each test’s tmp_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

Please address Copilot feedback. If not applicable, please explain why

Assisted-by: Codex (model: GPT-5, autonomous)

Copilot

Review details

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Low

+def _multi_install_safe_orders() -> list[list[str]]:
+    safe_keys = _multi_install_safe_keys()
+    if len(safe_keys) < 2:
+        return [safe_keys]
+    return [safe_keys[index:] + safe_keys[:index] for index in range(len(safe_keys))]


mnriem · 2026-06-29T14:38:50Z

Please address Copilot feedback

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

Copilot

Review details

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Low

+        integrations_dir = project_root / ".specify" / "integrations"
+        manifests = {
+            key: set(
+                json.loads(
+                    (integrations_dir / f"{key}.manifest.json").read_text(encoding="utf-8")
+                ).get("files", {})
            )
-
-            initial_files = set(initial_manifest.get("files", {}))
-            additional_files = set(additional_manifest.get("files", {}))
-
-            assert initial_files.isdisjoint(additional_files), (
-                f"{initial} and {additional} are declared multi-install safe but both manage "
-                f"these files: {sorted(initial_files & additional_files)}"
+            for key in ordered_keys
+        }


mnriem · 2026-06-30T13:38:09Z

Please address Copilot feedback

PascalThuet added 2 commits June 24, 2026 09:26

test: isolate integration test home

7acd7d2

Assisted-by: Codex (model: GPT-5, autonomous)

test: reduce registry manifest test repetition

0167a0f

Assisted-by: Codex (model: GPT-5, autonomous)

PascalThuet marked this pull request as ready for review June 24, 2026 08:05

PascalThuet requested a review from mnriem as a code owner June 24, 2026 08:05

test: clarify disjoint-manifest order rationale and guard safe set

ca07e0e

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.

mnriem requested a review from Copilot June 24, 2026 17:21

Copilot started reviewing on behalf of mnriem June 24, 2026 17:22 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

mnriem requested changes Jun 24, 2026

View reviewed changes

test: rotate init coverage for manifest isolation

5375034

Assisted-by: Codex (model: GPT-5, autonomous)

mnriem requested a review from Copilot June 29, 2026 12:19

Copilot started reviewing on behalf of mnriem June 29, 2026 12:19 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

Comment thread tests/integrations/test_registry.py

Comment on lines +51 to +55

def _multi_install_safe_orders() -> list[list[str]]:

safe_keys = _multi_install_safe_keys()

if len(safe_keys) < 2:

return [safe_keys]

return [safe_keys[index:] + safe_keys[:index] for index in range(len(safe_keys))]

Merge origin/main into chore/parallelize-integration-tests

fbbb455

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

mnriem requested a review from Copilot June 30, 2026 13:25

Copilot started reviewing on behalf of mnriem June 30, 2026 13:25 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: reduce registry manifest test repetition#3146

test: reduce registry manifest test repetition#3146
PascalThuet wants to merge 5 commits into
github:mainfrom
PascalThuet:chore/parallelize-integration-tests

PascalThuet commented Jun 24, 2026 •

edited

Loading

Uh oh!

PascalThuet commented Jun 24, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

mnriem left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

mnriem commented Jun 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mnriem commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

PascalThuet commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Stack

Validation

Disclosure

Uh oh!

PascalThuet commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Review details

Uh oh!

mnriem commented Jun 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Review details

Uh oh!

mnriem commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PascalThuet commented Jun 24, 2026 •

edited

Loading

PascalThuet commented Jun 24, 2026 •

edited

Loading