fix(install,tests): repair 63 integration tests in merge queue#1257
Merged
danielmeppiel merged 3 commits intomainfrom May 10, 2026
Merged
fix(install,tests): repair 63 integration tests in merge queue#1257danielmeppiel merged 3 commits intomainfrom
danielmeppiel merged 3 commits intomainfrom
Conversation
The merge-queue Integration Tests job for #1238 surfaced 63 failures that had accumulated in the suite since several integration tests were finally wired into CI. Fixes group into seven clusters: 1. Copilot harness marker (post-#1154): bare `.github/` no longer counts; fixtures must drop `.github/copilot-instructions.md`. 2. `apm marketplace build` was removed in favor of `apm pack`; tests now assert the deprecation message + exit 2. 3. `tests/fixtures/azure-skills/.claude-plugin/marketplace.json` regenerated and SHA constant refreshed. 4. `apm.yml` invalid-deps assertion: tolerate Rich line-wrapping in `dependencies` literal echo. 5. Schema fix: stop using removed top-level `repo:` key; use `git:`. 6. ADO virtual collections (`collections/<name>`) now use the SUBDIRECTORY layout per #1094; orphan-detection and dependency- declaration-order tests updated to slash form. 7. `test_auto_bootstrap_creates_user_manifest` exposed a real regression introduced by #1149: `user_scope_rejection_reason()` rejected ALL local paths at user scope, contradicting the post-#937 contract that only relative local paths are ambiguous. Restored absolute-vs-relative gating. The two unit failures in `test_mcp_registry_module` and `test_registry_client` reproduce on `origin/main` without these changes and are pre-existing, out of scope for this PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR repairs a large set of integration tests that began failing once more of the integration suite was wired into CI, and it also fixes a real product regression around --global installs of local packages.
Changes:
- Updated many integration tests to match current target-detection and dependency/schema contracts (notably
.github/copilot-instructions.mdas the Copilot marker, ADO virtual collection subdirectory layout, andgit:replacingrepo:inapm.yml). - Adjusted marketplace-build integration coverage to reflect the removal/stub behavior of
apm marketplace buildin favor ofapm pack. - Restored the intended install-time validation behavior for user-scope local packages by rejecting only relative local paths at
--globalscope.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/test_virtual_package_orphan_detection.py | Updates expected on-disk and lockfile identity paths for ADO virtual collection subdirectory layout. |
| tests/integration/test_update_e2e.py | Switches structured dependency key from repo to git in generated apm.yml. |
| tests/integration/test_uninstall_multi_e2e.py | Adds Copilot marker file so target detection passes. |
| tests/integration/test_uninstall_dry_run_e2e.py | Adds Copilot marker file so target detection passes. |
| tests/integration/test_selective_install_mcp.py | Adds helper to mark projects as Copilot harnesses; updates multiple tests to use it. |
| tests/integration/test_policy_install_e2e.py | Adds Copilot marker file so target detection passes. |
| tests/integration/test_plugin_e2e.py | Adds Copilot marker file so target detection passes. |
| tests/integration/test_pack_unpack_e2e.py | Adds Copilot marker file so target detection passes. |
| tests/integration/test_local_install.py | Adds Copilot marker file in the consumer project fixture. |
| tests/integration/test_local_content_audit.py | Adds Copilot marker file during local-content seeding. |
| tests/integration/test_intra_package_cleanup.py | Adds Copilot marker file so target detection passes. |
| tests/integration/test_install_invalid_deps_format_e2e.py | Adds Copilot marker file; makes error-text assertions more robust to wrapping. |
| tests/integration/test_install_dry_run_e2e.py | Adds Copilot marker file so target detection passes. |
| tests/integration/test_global_scope_e2e.py | Adds Copilot marker file in fake home to satisfy --global target detection. |
| tests/integration/test_global_install_e2e.py | Adds Copilot marker file so target detection passes. |
| tests/integration/test_deployed_files_e2e.py | Updates fixture setup/commentary to reflect Copilot marker requirements. |
| tests/integration/test_azure_skills_marketplace.py | Updates expected SHA256 snapshot for azure-skills fixture build output. |
| tests/integration/marketplace/test_build_integration.py | Updates expectations for the removed apm marketplace build command (stub exit code + messaging). |
| tests/fixtures/azure-skills/.claude-plugin/marketplace.json | Snapshot normalization (key ordering). |
| src/apm_cli/install/package_resolution.py | Fixes user-scope local-path validation to reject only relative paths (restoring intended behavior). |
| CHANGELOG.md | Adds Unreleased entries describing the regression fix and the integration suite realignment. |
Copilot's findings
- Files reviewed: 21/21 changed files
- Comments generated: 3
Comment on lines
+83
to
+93
| from pathlib import Path | ||
|
|
||
| from apm_cli.core.scope import InstallScope | ||
|
|
||
| if dep_ref.is_local and scope is InstallScope.USER: | ||
| return ( | ||
| "local packages are not supported at user scope (--global). " | ||
| "Use a remote reference (owner/repo) instead" | ||
| ) | ||
| local_path = dep_ref.local_path or "" | ||
| if not Path(local_path).is_absolute(): | ||
| return ( | ||
| "relative local paths are not supported at user scope (--global). " | ||
| "Use an absolute path or a remote reference (owner/repo) instead" | ||
| ) |
Comment on lines
34
to
41
| @pytest.fixture | ||
| def temp_project(tmp_path): | ||
| """Temp APM project with .github/ for VSCode target detection.""" | ||
| project_dir = tmp_path / "dry-run-test" | ||
| project_dir.mkdir() | ||
| (project_dir / ".github").mkdir() | ||
| (project_dir / ".github" / "copilot-instructions.md").write_text("# test\n") | ||
| return project_dir |
Comment on lines
67
to
68
| # .github/ already-exists triggers copilot target detection (and the | ||
| # default fallback is also copilot, so this is belt-and-suspenders). |
Adds two fast unit-level test files that pin the contracts whose absence let the corresponding regressions ship to release: 1. tests/unit/install/test_user_scope_rejection_reason.py Pins the post-#937 / post-#1149 contract for local-path policy at user scope: relative -> reject; absolute -> accept; remote -> accept; git: parent -> reject; project scope -> never reject. Verified to fail when the predicate is reverted to the broken #1149 form (5 of 15 tests fail). 2. tests/unit/test_install_path_declaration_invariant.py Asserts that DependencyReference.get_install_path() and primitives.discovery.get_dependency_declaration_order() agree on the on-disk path for every dependency flavor: regular GitHub, regular ADO, virtual subdirectory GitHub, and -- critically -- virtual subdirectory ADO (collections/<name>) which was the cluster-6 drift in the merge-queue run. Verified to fail when the declaration-order branch is mutated to emit the deleted-pre-#1094 flattened form (2 of 6 tests fail). Both files run in <1s combined, so the next refactor that touches either contract fails one assertion in CI instead of waiting for a slow E2E that may not even be wired in yet. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1. user_scope_rejection_reason now expanduser()s the local path before the absolute check. The rest of the install pipeline (sources.py, phases/resolve.py) already expanduser()s local paths, so without this alignment 'apm install --global ~/pkg' was incorrectly rejected as a relative path even though it expands to an absolute one. Trap added: tests/unit/install/test_user_scope_rejection_reason.py gains test_user_scope_accepts_tilde_local_path (parametrized x2). 2. tests/integration/test_install_dry_run_e2e.py: the temp_project fixture pre-created .github/copilot-instructions.md but _assert_no_install_artifacts asserted that same file is absent after the dry-run -- a structural contradiction that masked real dry-run regressions. Fix: drop the fixture file and set targets: [copilot] explicitly in the generated apm.yml so target detection no longer depends on the marker existing pre-install. 3. tests/integration/test_local_content_audit.py: refresh stale comment to reflect the post-#1154 detection contract (the marker is .github/copilot-instructions.md, not a bare .github/ directory). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
pushed a commit
that referenced
this pull request
May 11, 2026
Two integration tests were green in the merge queue (because the suite was not yet wired in) and surfaced as failures only after #1257 enabled the runner. 1. test_deployed_files_are_under_github_or_claude The lockfile correctly records '.agents/skills/<name>' for skill primitives (AGENTS.md is the cross-tool common skills format used by Codex / Cursor / OpenCode). The assertion was stale: it forbade '.agents/' even though deployment there is by design when a plugin-style package contains skills. Renamed the test to ..._under_known_target_roots and widened the allowlist to ('.github/', '.claude/', '.agents/'). 2. test_pack_unpack_e2e::test_full_round_trip 'apm unpack' is deprecated (the CLI itself prints '"apm unpack" is deprecated and will be removed in v0.14. Use "apm install <bundle-path>" instead'). Its bundle verification compares lockfile '.github/...' paths against on-disk plugin-bundle 'agents/' / 'commands/' / 'instructions/' paths and always fails -- the round-trip is structurally broken on the supported plugin bundle format. Switched the test to use the supported 'apm install <archive>' migration path and seeded a copilot signal in the consumer dir for target detection. Both failures verified locally before and after the fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
added a commit
that referenced
this pull request
May 11, 2026
Two integration tests were green in the merge queue (because the suite was not yet wired in) and surfaced as failures only after #1257 enabled the runner. 1. test_deployed_files_are_under_github_or_claude The lockfile correctly records '.agents/skills/<name>' for skill primitives (AGENTS.md is the cross-tool common skills format used by Codex / Cursor / OpenCode). The assertion was stale: it forbade '.agents/' even though deployment there is by design when a plugin-style package contains skills. Renamed the test to ..._under_known_target_roots and widened the allowlist to ('.github/', '.claude/', '.agents/'). 2. test_pack_unpack_e2e::test_full_round_trip 'apm unpack' is deprecated (the CLI itself prints '"apm unpack" is deprecated and will be removed in v0.14. Use "apm install <bundle-path>" instead'). Its bundle verification compares lockfile '.github/...' paths against on-disk plugin-bundle 'agents/' / 'commands/' / 'instructions/' paths and always fails -- the round-trip is structurally broken on the supported plugin bundle format. Switched the test to use the supported 'apm install <archive>' migration path and seeded a copilot signal in the consumer dir for target detection. Both failures verified locally before and after the fix. Co-authored-by: Daniel Meppiel <copilot-rework@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
TL;DR
The merge-queue
Integration Testsjob for #1238 surfaced 63 failing tests that had accumulated in the suite since several integration tests were finally wired into CI. This PR fixes all of them, plus one real regression discovered along the way.Verified locally with
APM_RUN_INTEGRATION_TESTS=1 uv run --extra dev pytest tests/integration/: 473 passed, 209 skipped, 16 deselected, 2 xfailed (0 failures). Lint silent.Failure clusters
.github/directory is no longer a copilot harness signal --.github/copilot-instructions.mdis.apm marketplace buildwas removed in favor ofapm pack(exit 2 + recovery message).tests/fixtures/azure-skills/.claude-plugin/marketplace.jsonsnapshot drift (key ordering, semantically identical).dependenciesliteral echo on narrow terminals; substring assertion was brittle.repo:key instead ofgit:.collections/<name>) now use the SUBDIRECTORY layout per #1094; install-path and orphan-detection use the natural slash path, not the old flattened name.user_scope_rejection_reason()rejected ALL local paths at user scope, contradicting the post-#937 contract that only relative local paths are ambiguous.Cluster 7 in detail (the real bug)
PR #937 (Apr 2026, "allow local packages at --global scope") explicitly removed the rejection of absolute local paths at user scope. PR #1149 ("GitLab marketplace host support") refactored validation into
src/apm_cli/install/package_resolution.py:user_scope_rejection_reason()and re-introduced the unconditionaldep_ref.is_localcheck, regressing the contract. This PR restores absolute-vs-relative gating and updates the error message accordingly.Pre-existing failures (NOT fixed here)
Two unit tests fail on
origin/mainwithout these changes (network-mocking flakes), out of scope:tests/unit/install/test_mcp_registry_module.py::TestRegistryClientTimeout::test_session_get_called_with_timeouttests/unit/test_registry_client.py::TestSimpleRegistryClient::test_list_serversHow to test