From 2cfff9c471a0c0584a8b99ae597f55e1a1075118 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Mon, 27 Apr 2026 11:50:38 +0100 Subject: [PATCH] fix(test): patch sys.platform in cowork tests for Windows CI macOS auto-detection tests did not mock sys.platform, so on Windows CI the code entered the win32 branch first and returned None without ever reaching the CloudStorage glob logic. 5 tests failed; 2 more passed for the wrong reason (win32 short-circuit returning None matched the expected result). Fix: every platform-specific test now patches apm_cli.integration.copilot_cowork_paths.sys.platform to the intended OS ("darwin" for macOS tests, "linux" for the Linux test). Also normalise the path in set_copilot_cowork_skills_dir with os.path.normpath to avoid mixed separators on Windows (forward slash from tilde expansion vs backslash from os.path.join). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/config.py | 2 +- .../integration/test_copilot_cowork_paths.py | 66 +++++++++++++------ 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/apm_cli/config.py b/src/apm_cli/config.py index af2dd8aae..0e8409b56 100644 --- a/src/apm_cli/config.py +++ b/src/apm_cli/config.py @@ -169,7 +169,7 @@ def set_copilot_cowork_skills_dir(path: str) -> None: """ if not path or not path.strip(): raise ValueError("Path cannot be empty") - expanded = os.path.expanduser(path) + expanded = os.path.normpath(os.path.expanduser(path)) if not os.path.isabs(expanded): raise ValueError(f"Path must be absolute: {expanded}") update_config({"copilot_cowork_skills_dir": expanded}) diff --git a/tests/unit/integration/test_copilot_cowork_paths.py b/tests/unit/integration/test_copilot_cowork_paths.py index dd685ec75..fb44c57f9 100644 --- a/tests/unit/integration/test_copilot_cowork_paths.py +++ b/tests/unit/integration/test_copilot_cowork_paths.py @@ -72,9 +72,12 @@ def test_macos_single_tenant_returns_skills_dir( cloud_dir = tmp_path / "Library" / "CloudStorage" tenant_dir = cloud_dir / "OneDrive - Tenant" tenant_dir.mkdir(parents=True) - with patch( - "apm_cli.integration.copilot_cowork_paths.Path.home", - return_value=tmp_path, + with ( + patch("apm_cli.integration.copilot_cowork_paths.sys.platform", "darwin"), + patch( + "apm_cli.integration.copilot_cowork_paths.Path.home", + return_value=tmp_path, + ), ): result = resolve_copilot_cowork_skills_dir() expected = tenant_dir / "Documents" / "Cowork" / "skills" @@ -87,9 +90,12 @@ def test_macos_zero_tenant_returns_none( cloud_dir = tmp_path / "Library" / "CloudStorage" cloud_dir.mkdir(parents=True) # No OneDrive dirs - with patch( - "apm_cli.integration.copilot_cowork_paths.Path.home", - return_value=tmp_path, + with ( + patch("apm_cli.integration.copilot_cowork_paths.sys.platform", "darwin"), + patch( + "apm_cli.integration.copilot_cowork_paths.Path.home", + return_value=tmp_path, + ), ): result = resolve_copilot_cowork_skills_dir() assert result is None @@ -99,9 +105,12 @@ def test_macos_no_cloud_storage_dir_returns_none( ) -> None: monkeypatch.delenv("APM_COPILOT_COWORK_SKILLS_DIR", raising=False) # No Library/CloudStorage at all - with patch( - "apm_cli.integration.copilot_cowork_paths.Path.home", - return_value=tmp_path, + with ( + patch("apm_cli.integration.copilot_cowork_paths.sys.platform", "darwin"), + patch( + "apm_cli.integration.copilot_cowork_paths.Path.home", + return_value=tmp_path, + ), ): result = resolve_copilot_cowork_skills_dir() assert result is None @@ -113,9 +122,12 @@ def test_macos_multi_tenant_raises_cowork_resolution_error( cloud_dir = tmp_path / "Library" / "CloudStorage" (cloud_dir / "OneDrive - TenantA").mkdir(parents=True) (cloud_dir / "OneDrive - TenantB").mkdir(parents=True) - with patch( - "apm_cli.integration.copilot_cowork_paths.Path.home", - return_value=tmp_path, + with ( + patch("apm_cli.integration.copilot_cowork_paths.sys.platform", "darwin"), + patch( + "apm_cli.integration.copilot_cowork_paths.Path.home", + return_value=tmp_path, + ), ): with pytest.raises(CoworkResolutionError): resolve_copilot_cowork_skills_dir() @@ -127,9 +139,12 @@ def test_multi_tenant_error_message_lists_candidates( cloud_dir = tmp_path / "Library" / "CloudStorage" (cloud_dir / "OneDrive - TenantA").mkdir(parents=True) (cloud_dir / "OneDrive - TenantB").mkdir(parents=True) - with patch( - "apm_cli.integration.copilot_cowork_paths.Path.home", - return_value=tmp_path, + with ( + patch("apm_cli.integration.copilot_cowork_paths.sys.platform", "darwin"), + patch( + "apm_cli.integration.copilot_cowork_paths.Path.home", + return_value=tmp_path, + ), ): with pytest.raises(CoworkResolutionError) as exc_info: resolve_copilot_cowork_skills_dir() @@ -144,9 +159,12 @@ def test_multi_tenant_error_message_hint_contains_env_var_name( cloud_dir = tmp_path / "Library" / "CloudStorage" (cloud_dir / "OneDrive - TenantA").mkdir(parents=True) (cloud_dir / "OneDrive - TenantB").mkdir(parents=True) - with patch( - "apm_cli.integration.copilot_cowork_paths.Path.home", - return_value=tmp_path, + with ( + patch("apm_cli.integration.copilot_cowork_paths.sys.platform", "darwin"), + patch( + "apm_cli.integration.copilot_cowork_paths.Path.home", + return_value=tmp_path, + ), ): with pytest.raises(CoworkResolutionError) as exc_info: resolve_copilot_cowork_skills_dir() @@ -165,9 +183,12 @@ def test_linux_no_env_returns_none( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: monkeypatch.delenv("APM_COPILOT_COWORK_SKILLS_DIR", raising=False) - with patch( - "apm_cli.integration.copilot_cowork_paths.Path.home", - return_value=tmp_path, + with ( + patch("apm_cli.integration.copilot_cowork_paths.sys.platform", "linux"), + patch( + "apm_cli.integration.copilot_cowork_paths.Path.home", + return_value=tmp_path, + ), ): result = resolve_copilot_cowork_skills_dir() assert result is None @@ -186,6 +207,7 @@ def test_config_beats_macos_auto_detect( (cloud / "OneDrive - Tenant").mkdir(parents=True) with ( patch("apm_cli.config.get_copilot_cowork_skills_dir", return_value="/config/skills"), + patch("apm_cli.integration.copilot_cowork_paths.sys.platform", "darwin"), patch( "apm_cli.integration.copilot_cowork_paths.Path.home", return_value=tmp_path, @@ -215,6 +237,7 @@ def test_auto_detect_used_when_both_env_and_config_absent( tenant.mkdir(parents=True) with ( patch("apm_cli.config.get_copilot_cowork_skills_dir", return_value=None), + patch("apm_cli.integration.copilot_cowork_paths.sys.platform", "darwin"), patch( "apm_cli.integration.copilot_cowork_paths.Path.home", return_value=tmp_path, @@ -243,6 +266,7 @@ def test_config_none_falls_through_cleanly_to_next_branch( # No CloudStorage directory -- auto-detect returns None. with ( patch("apm_cli.config.get_copilot_cowork_skills_dir", return_value=None), + patch("apm_cli.integration.copilot_cowork_paths.sys.platform", "darwin"), patch( "apm_cli.integration.copilot_cowork_paths.Path.home", return_value=tmp_path,