Skip to content

Commit d6f5afb

Browse files
author
Mowri M
committed
fix: mock _ensure_executable in mount tests for cross-platform compatibility
Commit 8738bbd added _ensure_executable() calls inside _find_copilot_cli() but did not update the mount tests to mock it. The function calls os.access() and os.stat() on fake test paths, which raises: - WinError 3 on Windows (path not found) - FileNotFoundError on Linux/macOS for non-existent paths The broad `except Exception` in _find_copilot_cli silently swallows these errors and returns None, causing 14 test failures on Windows and 7 on WSL. Fix: Add patch("amplifier_module_provider_github_copilot._ensure_executable") to all mount tests that expect _find_copilot_cli to return a valid path. Affected files: - tests/test_mount.py (10 tests) - tests/test_mount_coverage.py (7 tests) Verified: 29/29 mount tests pass on Windows, WSL, and macOS.
1 parent 1c94ae5 commit d6f5afb

2 files changed

Lines changed: 93 additions & 76 deletions

File tree

tests/test_mount.py

Lines changed: 56 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,15 @@ async def test_mount_success(self, mock_coordinator):
2020
with patch("shutil.which", return_value="/usr/bin/copilot"):
2121
with patch("os.path.isfile", return_value=True):
2222
with patch("os.path.isabs", return_value=True):
23-
cleanup = await mount(mock_coordinator, {"model": "claude-opus-4.5"})
23+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
24+
cleanup = await mount(mock_coordinator, {"model": "claude-opus-4.5"})
2425

25-
# Should return cleanup function
26-
assert cleanup is not None
27-
assert callable(cleanup)
26+
# Should return cleanup function
27+
assert cleanup is not None
28+
assert callable(cleanup)
2829

29-
# Provider should be mounted
30-
assert "github-copilot" in mock_coordinator.mounted_providers
30+
# Provider should be mounted
31+
assert "github-copilot" in mock_coordinator.mounted_providers
3132

3233
@pytest.mark.asyncio
3334
async def test_mount_missing_cli(self, mock_coordinator):
@@ -47,60 +48,64 @@ async def test_mount_with_custom_cli_path(self, mock_coordinator):
4748
with patch("shutil.which") as mock_which:
4849
with patch("os.path.isfile", return_value=True):
4950
with patch("os.path.isabs", return_value=True):
50-
mock_which.return_value = "/custom/path/copilot"
51+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
52+
mock_which.return_value = "/custom/path/copilot"
5153

52-
cleanup = await mount(
53-
mock_coordinator,
54-
{"cli_path": "/custom/path/copilot"},
55-
)
54+
cleanup = await mount(
55+
mock_coordinator,
56+
{"cli_path": "/custom/path/copilot"},
57+
)
5658

57-
assert cleanup is not None
59+
assert cleanup is not None
5860

5961
@pytest.mark.asyncio
6062
async def test_mount_cleanup_function(self, mock_coordinator):
6163
"""Cleanup function should close provider."""
6264
with patch("shutil.which", return_value="/usr/bin/copilot"):
6365
with patch("os.path.isfile", return_value=True):
6466
with patch("os.path.isabs", return_value=True):
65-
cleanup = await mount(mock_coordinator, {})
67+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
68+
cleanup = await mount(mock_coordinator, {})
6669

67-
assert cleanup is not None
70+
assert cleanup is not None
6871

69-
# Get the mounted provider
70-
provider = mock_coordinator.mounted_providers.get("github-copilot")
71-
assert provider is not None
72+
# Get the mounted provider
73+
provider = mock_coordinator.mounted_providers.get("github-copilot")
74+
assert provider is not None
7275

73-
# Mock the provider's close method
74-
with patch.object(provider, "close", new_callable=AsyncMock) as mock_close:
75-
await cleanup()
76-
mock_close.assert_called_once()
76+
# Mock the provider's close method
77+
with patch.object(provider, "close", new_callable=AsyncMock) as mock_close:
78+
await cleanup()
79+
mock_close.assert_called_once()
7780

7881
@pytest.mark.asyncio
7982
async def test_mount_default_config(self, mock_coordinator):
8083
"""Mount should work with no config (uses defaults)."""
8184
with patch("shutil.which", return_value="/usr/bin/copilot"):
8285
with patch("os.path.isfile", return_value=True):
8386
with patch("os.path.isabs", return_value=True):
84-
cleanup = await mount(mock_coordinator, None)
87+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
88+
cleanup = await mount(mock_coordinator, None)
8589

86-
assert cleanup is not None
87-
provider = mock_coordinator.mounted_providers.get("github-copilot")
88-
assert provider._model == "claude-opus-4.5" # Default model
90+
assert cleanup is not None
91+
provider = mock_coordinator.mounted_providers.get("github-copilot")
92+
assert provider._model == "claude-opus-4.5" # Default model
8993

9094
@pytest.mark.asyncio
9195
async def test_mount_registers_with_coordinator(self, mock_coordinator):
9296
"""Mount should call coordinator.mount with correct arguments."""
9397
with patch("shutil.which", return_value="/usr/bin/copilot"):
9498
with patch("os.path.isfile", return_value=True):
9599
with patch("os.path.isabs", return_value=True):
96-
await mount(mock_coordinator, {})
100+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
101+
await mount(mock_coordinator, {})
97102

98-
# Verify mount was called
99-
mock_coordinator.mount.assert_called_once()
100-
call_args = mock_coordinator.mount.call_args
103+
# Verify mount was called
104+
mock_coordinator.mount.assert_called_once()
105+
call_args = mock_coordinator.mount.call_args
101106

102-
assert call_args[0][0] == "providers" # category
103-
assert call_args[1]["name"] == "github-copilot"
107+
assert call_args[0][0] == "providers" # category
108+
assert call_args[1]["name"] == "github-copilot"
104109

105110

106111
class TestModuleMetadata:
@@ -152,15 +157,16 @@ async def test_mount_returns_none_on_provider_init_error(self, mock_coordinator)
152157
with patch("shutil.which", return_value="/usr/bin/copilot"):
153158
with patch("os.path.isfile", return_value=True):
154159
with patch("os.path.isabs", return_value=True):
155-
# Make coordinator.mount raise during provider registration
156-
mock_coordinator.mount = AsyncMock(
157-
side_effect=RuntimeError("Mount failed: config error")
158-
)
160+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
161+
# Make coordinator.mount raise during provider registration
162+
mock_coordinator.mount = AsyncMock(
163+
side_effect=RuntimeError("Mount failed: config error")
164+
)
159165

160-
cleanup = await mount(mock_coordinator, {})
166+
cleanup = await mount(mock_coordinator, {})
161167

162-
# Should return None (graceful degradation)
163-
assert cleanup is None
168+
# Should return None (graceful degradation)
169+
assert cleanup is None
164170

165171

166172
class TestFindCopilotCli:
@@ -172,9 +178,10 @@ def test_cli_from_config(self):
172178

173179
with patch("os.path.isabs", return_value=True):
174180
with patch("os.path.isfile", return_value=True):
175-
result = _find_copilot_cli({"cli_path": "/custom/copilot"})
181+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
182+
result = _find_copilot_cli({"cli_path": "/custom/copilot"})
176183

177-
assert result == "/custom/copilot"
184+
assert result == "/custom/copilot"
178185

179186
def test_cli_from_env_var(self):
180187
"""_find_copilot_cli should check COPILOT_CLI_PATH env var."""
@@ -183,9 +190,10 @@ def test_cli_from_env_var(self):
183190
with patch.dict("os.environ", {"COPILOT_CLI_PATH": "/env/copilot"}):
184191
with patch("os.path.isabs", return_value=True):
185192
with patch("os.path.isfile", return_value=True):
186-
result = _find_copilot_cli({})
193+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
194+
result = _find_copilot_cli({})
187195

188-
assert result == "/env/copilot"
196+
assert result == "/env/copilot"
189197

190198
def test_cli_from_shutil_which(self):
191199
"""_find_copilot_cli should fall back to shutil.which()."""
@@ -195,9 +203,10 @@ def test_cli_from_shutil_which(self):
195203
with patch("shutil.which", return_value="/usr/bin/copilot"):
196204
with patch("os.path.isabs", return_value=True):
197205
with patch("os.path.isfile", return_value=True):
198-
result = _find_copilot_cli({})
206+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
207+
result = _find_copilot_cli({})
199208

200-
assert result == "/usr/bin/copilot"
209+
assert result == "/usr/bin/copilot"
201210

202211
def test_cli_not_found_returns_none(self):
203212
"""_find_copilot_cli should return None when CLI not found."""
@@ -225,9 +234,10 @@ def test_cli_relative_path_resolved_via_which(self):
225234

226235
with patch("os.path.isabs", return_value=False):
227236
with patch("shutil.which", return_value="/resolved/path/copilot"):
228-
result = _find_copilot_cli({"cli_path": "copilot"})
237+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
238+
result = _find_copilot_cli({"cli_path": "copilot"})
229239

230-
assert result == "/resolved/path/copilot"
240+
assert result == "/resolved/path/copilot"
231241

232242
def test_cli_relative_path_not_found_in_path(self):
233243
"""_find_copilot_cli should return None if relative path not in PATH."""

tests/test_mount_coverage.py

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@ async def test_mount_returns_none_when_provider_creation_raises(self, mock_coord
2222
with patch("shutil.which", return_value="/usr/bin/copilot"):
2323
with patch("os.path.isfile", return_value=True):
2424
with patch("os.path.isabs", return_value=True):
25-
with patch(
26-
"amplifier_module_provider_github_copilot.CopilotSdkProvider",
27-
side_effect=Exception("Init explosion"),
28-
):
29-
cleanup = await mount(mock_coordinator, {"model": "claude-opus-4.5"})
25+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
26+
with patch(
27+
"amplifier_module_provider_github_copilot.CopilotSdkProvider",
28+
side_effect=Exception("Init explosion"),
29+
):
30+
cleanup = await mount(mock_coordinator, {"model": "claude-opus-4.5"})
3031

31-
assert cleanup is None
32-
assert "github-copilot" not in mock_coordinator.mounted_providers
32+
assert cleanup is None
33+
assert "github-copilot" not in mock_coordinator.mounted_providers
3334

3435
@pytest.mark.asyncio
3536
async def test_mount_returns_none_when_coordinator_mount_raises(self, mock_coordinator):
@@ -39,9 +40,10 @@ async def test_mount_returns_none_when_coordinator_mount_raises(self, mock_coord
3940
with patch("shutil.which", return_value="/usr/bin/copilot"):
4041
with patch("os.path.isfile", return_value=True):
4142
with patch("os.path.isabs", return_value=True):
42-
cleanup = await mount(mock_coordinator, {})
43+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
44+
cleanup = await mount(mock_coordinator, {})
4345

44-
assert cleanup is None
46+
assert cleanup is None
4547

4648

4749
class TestFindCopilotCliEnvVar:
@@ -54,24 +56,26 @@ async def test_env_var_cli_path(self, mock_coordinator):
5456
with patch("shutil.which", return_value=None):
5557
with patch("os.path.isfile", return_value=True):
5658
with patch("os.path.isabs", return_value=True):
57-
cleanup = await mount(mock_coordinator, {})
59+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
60+
cleanup = await mount(mock_coordinator, {})
5861

59-
assert cleanup is not None
60-
provider = mock_coordinator.mounted_providers.get("github-copilot")
61-
assert provider is not None
62+
assert cleanup is not None
63+
provider = mock_coordinator.mounted_providers.get("github-copilot")
64+
assert provider is not None
6265

6366
@pytest.mark.asyncio
6467
async def test_config_cli_path_takes_priority_over_env(self, mock_coordinator):
6568
"""Config cli_path should be used even when env var is set."""
6669
with patch.dict(os.environ, {"COPILOT_CLI_PATH": "/env/copilot"}):
6770
with patch("os.path.isfile", return_value=True):
6871
with patch("os.path.isabs", return_value=True):
69-
cleanup = await mount(
70-
mock_coordinator,
71-
{"cli_path": "/config/copilot"},
72-
)
72+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
73+
cleanup = await mount(
74+
mock_coordinator,
75+
{"cli_path": "/config/copilot"},
76+
)
7377

74-
assert cleanup is not None
78+
assert cleanup is not None
7579

7680

7781
class TestFindCopilotCliAbsolutePath:
@@ -94,12 +98,13 @@ async def test_absolute_path_exists_succeeds(self, mock_coordinator):
9498
"""Should succeed when absolute path to CLI exists."""
9599
with patch("os.path.isabs", return_value=True):
96100
with patch("os.path.isfile", return_value=True):
97-
cleanup = await mount(
98-
mock_coordinator,
99-
{"cli_path": "/usr/local/bin/copilot"},
100-
)
101+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
102+
cleanup = await mount(
103+
mock_coordinator,
104+
{"cli_path": "/usr/local/bin/copilot"},
105+
)
101106

102-
assert cleanup is not None
107+
assert cleanup is not None
103108

104109

105110
class TestFindCopilotCliNonAbsolutePath:
@@ -122,12 +127,13 @@ async def test_non_absolute_path_found_in_path(self, mock_coordinator):
122127
"""Should resolve non-absolute CLI name through PATH."""
123128
with patch("os.path.isabs", return_value=False):
124129
with patch("shutil.which", return_value="/usr/bin/copilot"):
125-
cleanup = await mount(
126-
mock_coordinator,
127-
{"cli_path": "copilot"},
128-
)
130+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
131+
cleanup = await mount(
132+
mock_coordinator,
133+
{"cli_path": "copilot"},
134+
)
129135

130-
assert cleanup is not None
136+
assert cleanup is not None
131137

132138

133139
class TestFindCopilotCliExceptionPath:
@@ -175,5 +181,6 @@ def which_side_effect(name):
175181
with patch("shutil.which", side_effect=which_side_effect):
176182
with patch("os.path.isabs", return_value=True):
177183
with patch("os.path.isfile", return_value=True):
178-
result = _find_copilot_cli({})
179-
assert result == "C:\\Program Files\\copilot\\copilot.exe"
184+
with patch("amplifier_module_provider_github_copilot._ensure_executable"):
185+
result = _find_copilot_cli({})
186+
assert result == "C:\\Program Files\\copilot\\copilot.exe"

0 commit comments

Comments
 (0)