diff --git a/docs/src/content/docs/contributing/integration-testing.md b/docs/src/content/docs/contributing/integration-testing.md index ed63fa4a41..77d051a7a9 100644 --- a/docs/src/content/docs/contributing/integration-testing.md +++ b/docs/src/content/docs/contributing/integration-testing.md @@ -391,9 +391,19 @@ Every module under those roots must import on Windows before marker deselection. Import Unix-only modules such as `pwd` or `fcntl` inside the helpers that need them, not at module scope. The collection guard checks this with Unix-only standard modules unavailable, even on Linux and macOS. +The nested collection subprocess has a 300-second hard timeout: pytest must +import and collect both roots before applying the marker, including on release +runners sharing CPU with other test workers. Plugin autoload remains disabled, +and a collection timeout fails the check rather than skipping it. Collection proves a test is selected; a successful Windows job provides Windows execution evidence. The existing job timeout bounds runtime. +Plugin sequential-install coverage checks deployed-file removal and lockfile +ownership after uninstall, plus unchanged files and ownership for the retained +skill. The local fixture covers both an independent skill and a separately +installed skill from the plugin; the network-backed case uses the same assertions. +Cleanup wording is not the lifecycle contract. + Linux Lifecycle Smoke runs the required marker subset with `-n 2 --dist loadgroup`. Grouped tests stay on one worker, and the six-minute job limit remains unchanged. Its `lifecycle_smoke and not lifecycle_merge_group` selection is not all lifecycle coverage: also run affected generated state machines, deployment diff --git a/tests/integration/test_plugin_e2e.py b/tests/integration/test_plugin_e2e.py index 7a5e7bfb06..d14ad917f9 100644 --- a/tests/integration/test_plugin_e2e.py +++ b/tests/integration/test_plugin_e2e.py @@ -77,6 +77,38 @@ def _existing_deployed_file_paths(project_root: Path, deployed_files: list[str]) return existing +def _deployed_skill_contents(project_root: Path, dependency: dict) -> dict[Path, bytes]: + """Snapshot real skill files so retained deployment checks cannot pass vacuously.""" + paths = _existing_deployed_file_paths(project_root, dependency["deployed_files"]) + contents = {path: path.read_bytes() for path in paths if path.is_file()} + assert any(path.name == "SKILL.md" for path in contents), "Skill must be deployed" + return contents + + +def _assert_plugin_removed_skill_retained( + project_root: Path, + plugin_key: str, + plugin_deployed_paths: list[Path], + skill_dependency: dict, + skill_contents: dict[Path, bytes], +) -> None: + """Check filesystem cleanup and surviving lock ownership, not diagnostic wording.""" + import yaml + + assert plugin_deployed_paths, "Plugin must have deployed files before uninstall" + remaining = [path.as_posix() for path in plugin_deployed_paths if path.exists()] + assert not remaining, f"Plugin deployed files remained after uninstall: {remaining}" + lockfile = yaml.safe_load((project_root / "apm.lock.yaml").read_text()) + dep_keys = {_locked_dependency_key(entry) for entry in lockfile["dependencies"]} + skill_key = _locked_dependency_key(skill_dependency) + assert plugin_key not in dep_keys, "Plugin ownership must be removed from the lockfile" + assert dep_keys == {skill_key}, f"Expected only the retained skill, got {dep_keys}" + assert _locked_dependency(lockfile, skill_key) == skill_dependency + for path, content in skill_contents.items(): + assert path.is_file(), f"Retained skill file removed: {path}" + assert path.read_bytes() == content, f"Retained skill file changed: {path}" + + def _make_package_info( package: APMPackage, install_path: Path, package_type: PackageType ) -> PackageInfo: @@ -422,10 +454,11 @@ def test_generated_apm_yml_type_is_hybrid(self, tmp_path): assert revalidated.package_type is PackageType.SKILL_BUNDLE @pytest.mark.requires_apm_binary + @pytest.mark.parametrize("plugin_subskill", [False, True], ids=["standalone", "plugin-skill"]) def test_local_plugin_uninstall_after_sequential_skill_install_cleans_deployed_files( - self, apm_binary_path, tmp_path - ): - """Sequential CLI installs must retain plugin deployed_files for uninstall cleanup.""" + self, apm_binary_path: Path, tmp_path: Path, plugin_subskill: bool + ) -> None: + """Removing a plugin cleans its files without removing a separately installed skill.""" import yaml as yaml_lib if not FIXTURE_DIR.exists(): @@ -435,8 +468,11 @@ def test_local_plugin_uninstall_after_sequential_skill_install_cleans_deployed_f workspace.mkdir() plugin_dir = workspace / "plugin-src" shutil.copytree(FIXTURE_DIR, plugin_dir) - skill_dir = workspace / "skill-src" - _write_local_skill_package(skill_dir) + if plugin_subskill: + skill_dir = plugin_dir / "skills" / "test-skill" + else: + skill_dir = workspace / "skill-src" + _write_local_skill_package(skill_dir) project = workspace / "project" project.mkdir() @@ -466,6 +502,8 @@ def test_local_plugin_uninstall_after_sequential_skill_install_cleans_deployed_f deployed_paths = _existing_deployed_file_paths(project, deployed_files) assert project / ".github" / "agents" / "test-agent.agent.md" in deployed_paths + skill_dep = _locked_dependency(lockfile, f"_local/{skill_dir.name}") + skill_contents = _deployed_skill_contents(project, skill_dep) uninstall = _run_apm_command( apm_binary_path, ["uninstall", str(plugin_dir)], project, timeout=60 @@ -473,9 +511,9 @@ def test_local_plugin_uninstall_after_sequential_skill_install_cleans_deployed_f assert uninstall.returncode == 0, ( f"Uninstall failed:\n{uninstall.stdout}\n{uninstall.stderr}" ) - combined = uninstall.stdout + uninstall.stderr - assert "Uninstall complete" in combined - assert all(not path.exists() for path in deployed_paths) + _assert_plugin_removed_skill_retained( + project, "_local/plugin-src", deployed_paths, skill_dep, skill_contents + ) @pytest.mark.requires_apm_binary def test_copilot_dialect_plugin_lsp_survives_install_reinstall_and_audit( @@ -998,8 +1036,10 @@ def test_uninstall_plugin(self, apm_binary_path, temp_project): # ---- Test 6: lockfile preserved on sequential installs --------------- - def test_lockfile_preserved_on_sequential_install(self, apm_binary_path, temp_project): - """Installing packages one at a time must preserve previous lockfile entries.""" + def test_lockfile_preserved_on_sequential_install( + self, apm_binary_path: Path, temp_project: Path + ) -> None: + """Sequential installs preserve ownership through scoped plugin removal.""" skill_ref = self.SKILL_REF # Install plugin @@ -1037,6 +1077,8 @@ def test_lockfile_preserved_on_sequential_install(self, apm_binary_path, temp_pr plugin_dep = _locked_dependency(lockfile, plugin_key) plugin_deployed_files = plugin_dep.get("deployed_files", []) plugin_deployed_paths = _existing_deployed_file_paths(temp_project, plugin_deployed_files) + skill_dep = _locked_dependency(lockfile, self.SKILL_PATH) + skill_contents = _deployed_skill_contents(temp_project, skill_dep) # deps tree should show both tree = subprocess.run( @@ -1046,6 +1088,7 @@ def test_lockfile_preserved_on_sequential_install(self, apm_binary_path, temp_pr cwd=str(temp_project), timeout=60, ) + assert tree.returncode == 0, f"Deps tree failed:\n{tree.stdout}\n{tree.stderr}" combined = tree.stdout + tree.stderr assert "mock-marketplace-plugin" in combined, "Plugin missing from deps tree" assert "test-skill" in combined, "Skill missing from deps tree" @@ -1059,13 +1102,9 @@ def test_lockfile_preserved_on_sequential_install(self, apm_binary_path, temp_pr timeout=60, ) assert r3.returncode == 0, f"Uninstall failed:\n{r3.stderr}" - combined = r3.stdout + r3.stderr - if plugin_deployed_paths: - assert "Cleaned up" in combined and "integrated" in combined.lower(), ( - f"Uninstall should report integration cleanup:\n{combined}" - ) - remaining = [path.as_posix() for path in plugin_deployed_paths if path.exists()] - assert not remaining, f"Plugin deployed files remained after uninstall: {remaining}" + _assert_plugin_removed_skill_retained( + temp_project, plugin_key, plugin_deployed_paths, skill_dep, skill_contents + ) # ---- Test 7: compile includes plugin primitives --------------------- diff --git a/tests/unit/test_windows_compat_gate_workflow.py b/tests/unit/test_windows_compat_gate_workflow.py index 12c0cee2f0..7602247463 100644 --- a/tests/unit/test_windows_compat_gate_workflow.py +++ b/tests/unit/test_windows_compat_gate_workflow.py @@ -136,7 +136,9 @@ def _collect_gate_family(args: list[str]) -> subprocess.CompletedProcess[str]: env=collection_env, capture_output=True, text=True, - timeout=120, + # Marker deselection follows full-root collection; allow for slow, + # contended release runners, as the repository taxonomy guard does. + timeout=300, check=False, ) @@ -281,10 +283,10 @@ def test_gate_collection_rejects_invalid_selections( _assert_gate_family_collection(subprocess.CompletedProcess([], returncode, summary, "")) -def test_nested_collection_disables_plugin_autoload( +def test_nested_collection_preserves_environment_and_bounded_timeout( monkeypatch: pytest.MonkeyPatch, ) -> None: - """Nested collection must not import unrelated third-party plugins.""" + """Keep baseline environment isolation and a finite full-root collection budget.""" expected_path = os.environ.get("PATH") captured_env: dict[str, str] | None = None @@ -295,6 +297,12 @@ def fake_run( nonlocal captured_env env = kwargs.get("env") captured_env = env if isinstance(env, dict) else None + assert kwargs["timeout"] == 300 + assert kwargs["cwd"] == ROOT + assert kwargs["capture_output"] is True + assert kwargs["check"] is False + assert command[0] == sys.executable + assert command[-3:] == ["-m", GATE_MARKER, "tests/unit"] return subprocess.CompletedProcess( command, returncode=0, @@ -304,6 +312,7 @@ def fake_run( monkeypatch.setattr(subprocess, "run", fake_run) monkeypatch.delenv("PATH", raising=False) + monkeypatch.setenv("PYTEST_DISABLE_PLUGIN_AUTOLOAD", "0") _collect_gate_family(["-m", GATE_MARKER, "tests/unit"]) @@ -312,6 +321,19 @@ def fake_run( assert captured_env.get("PATH") == expected_path +def test_nested_collection_timeout_fails_closed(monkeypatch: pytest.MonkeyPatch) -> None: + """A hung collector must still fail rather than skip or synthesize a passing result.""" + failure = subprocess.TimeoutExpired("pytest", 300, output="partial collection") + + def timed_out(command: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + raise failure + + monkeypatch.setattr(subprocess, "run", timed_out) + with pytest.raises(subprocess.TimeoutExpired) as exc: + _collect_gate_family(["-m", GATE_MARKER, "tests/unit", "tests/integration"]) + assert exc.value is failure + + @pytest.mark.parametrize( "expected_checks_key", (