|
4 | 4 | This module tests the mount() entry point and prerequisite checking. |
5 | 5 | """ |
6 | 6 |
|
7 | | -from unittest.mock import AsyncMock, patch |
| 7 | +from unittest.mock import AsyncMock, Mock, patch |
8 | 8 |
|
9 | 9 | import pytest |
10 | 10 |
|
@@ -44,23 +44,23 @@ async def test_mount_missing_cli(self, mock_coordinator): |
44 | 44 |
|
45 | 45 | @pytest.mark.asyncio |
46 | 46 | async def test_mount_cleanup_function(self, mock_coordinator): |
47 | | - """Cleanup function should close provider.""" |
| 47 | + """Cleanup function should release the shared client reference.""" |
48 | 48 | with patch("shutil.which", return_value="/usr/bin/copilot"): |
49 | | - with patch("os.path.isfile", return_value=True): |
50 | | - with patch("os.path.isabs", return_value=True): |
51 | | - with patch("amplifier_module_provider_github_copilot._ensure_executable"): |
52 | | - cleanup = await mount(mock_coordinator, {}) |
| 49 | + with patch("amplifier_module_provider_github_copilot._ensure_executable"): |
| 50 | + with patch( |
| 51 | + "amplifier_module_provider_github_copilot.CopilotClientWrapper" |
| 52 | + ) as mock_wrapper_cls: |
| 53 | + mock_wrapper_cls.return_value = Mock() |
53 | 54 |
|
54 | | - assert cleanup is not None |
| 55 | + cleanup = await mount(mock_coordinator, {}) |
| 56 | + assert cleanup is not None |
55 | 57 |
|
56 | | - # Get the mounted provider |
57 | | - provider = mock_coordinator.mounted_providers.get("github-copilot") |
58 | | - assert provider is not None |
| 58 | + import amplifier_module_provider_github_copilot as mod |
| 59 | + |
| 60 | + assert mod._shared_client_refcount == 1 |
59 | 61 |
|
60 | | - # Mock the provider's close method |
61 | | - with patch.object(provider, "close", new_callable=AsyncMock) as mock_close: |
62 | | - await cleanup() |
63 | | - mock_close.assert_called_once() |
| 62 | + await cleanup() |
| 63 | + assert mod._shared_client_refcount == 0 |
64 | 64 |
|
65 | 65 | @pytest.mark.asyncio |
66 | 66 | async def test_mount_default_config(self, mock_coordinator): |
@@ -203,3 +203,153 @@ def which_side_effect(name): |
203 | 203 | with patch("amplifier_module_provider_github_copilot._ensure_executable"): |
204 | 204 | result = _find_copilot_cli({}) |
205 | 205 | assert result == "C:\\Program Files\\copilot\\copilot.exe" |
| 206 | + |
| 207 | + |
| 208 | +class TestSingleton: |
| 209 | + """Tests for the process-level singleton CopilotClientWrapper.""" |
| 210 | + |
| 211 | + @pytest.mark.asyncio |
| 212 | + async def test_singleton_creates_one_wrapper(self, mock_coordinator): |
| 213 | + """First mount should create exactly one CopilotClientWrapper.""" |
| 214 | + with patch("shutil.which", return_value="/usr/bin/copilot"): |
| 215 | + with patch("amplifier_module_provider_github_copilot._ensure_executable"): |
| 216 | + with patch( |
| 217 | + "amplifier_module_provider_github_copilot.CopilotClientWrapper" |
| 218 | + ) as mock_wrapper_cls: |
| 219 | + mock_wrapper_cls.return_value = Mock() |
| 220 | + |
| 221 | + await mount(mock_coordinator, {}) |
| 222 | + |
| 223 | + mock_wrapper_cls.assert_called_once() |
| 224 | + |
| 225 | + @pytest.mark.asyncio |
| 226 | + async def test_singleton_reuses_wrapper_across_mounts(self): |
| 227 | + """Multiple mounts should reuse the same CopilotClientWrapper instance.""" |
| 228 | + with patch("shutil.which", return_value="/usr/bin/copilot"): |
| 229 | + with patch("amplifier_module_provider_github_copilot._ensure_executable"): |
| 230 | + with patch( |
| 231 | + "amplifier_module_provider_github_copilot.CopilotClientWrapper" |
| 232 | + ) as mock_wrapper_cls: |
| 233 | + mock_wrapper_cls.return_value = Mock() |
| 234 | + |
| 235 | + coordinator_a = Mock() |
| 236 | + coordinator_a.mount = AsyncMock() |
| 237 | + coordinator_a.hooks = Mock() |
| 238 | + coordinator_a.hooks.emit = AsyncMock() |
| 239 | + |
| 240 | + coordinator_b = Mock() |
| 241 | + coordinator_b.mount = AsyncMock() |
| 242 | + coordinator_b.hooks = Mock() |
| 243 | + coordinator_b.hooks.emit = AsyncMock() |
| 244 | + |
| 245 | + coordinator_c = Mock() |
| 246 | + coordinator_c.mount = AsyncMock() |
| 247 | + coordinator_c.hooks = Mock() |
| 248 | + coordinator_c.hooks.emit = AsyncMock() |
| 249 | + |
| 250 | + await mount(coordinator_a, {}) |
| 251 | + await mount(coordinator_b, {}) |
| 252 | + await mount(coordinator_c, {}) |
| 253 | + |
| 254 | + # Only ONE wrapper should ever be created |
| 255 | + assert mock_wrapper_cls.call_count == 1 |
| 256 | + |
| 257 | + import amplifier_module_provider_github_copilot as mod |
| 258 | + |
| 259 | + assert mod._shared_client_refcount == 3 |
| 260 | + |
| 261 | + @pytest.mark.asyncio |
| 262 | + async def test_singleton_close_only_on_last_cleanup(self): |
| 263 | + """close() should be called only when the last session's cleanup runs.""" |
| 264 | + with patch("shutil.which", return_value="/usr/bin/copilot"): |
| 265 | + with patch("amplifier_module_provider_github_copilot._ensure_executable"): |
| 266 | + with patch( |
| 267 | + "amplifier_module_provider_github_copilot.CopilotClientWrapper" |
| 268 | + ) as mock_wrapper_cls: |
| 269 | + mock_client_instance = AsyncMock() |
| 270 | + mock_client_instance.close = AsyncMock() |
| 271 | + mock_wrapper_cls.return_value = mock_client_instance |
| 272 | + |
| 273 | + coordinator_a = Mock() |
| 274 | + coordinator_a.mount = AsyncMock() |
| 275 | + coordinator_a.hooks = Mock() |
| 276 | + coordinator_a.hooks.emit = AsyncMock() |
| 277 | + |
| 278 | + coordinator_b = Mock() |
| 279 | + coordinator_b.mount = AsyncMock() |
| 280 | + coordinator_b.hooks = Mock() |
| 281 | + coordinator_b.hooks.emit = AsyncMock() |
| 282 | + |
| 283 | + cleanup_a = await mount(coordinator_a, {}) |
| 284 | + cleanup_b = await mount(coordinator_b, {}) |
| 285 | + |
| 286 | + assert cleanup_a is not None |
| 287 | + assert cleanup_b is not None |
| 288 | + |
| 289 | + await cleanup_a() |
| 290 | + # close() must NOT have been called yet — b is still mounted |
| 291 | + mock_client_instance.close.assert_not_called() |
| 292 | + |
| 293 | + await cleanup_b() |
| 294 | + # Now the last reference is gone — close() must have been called |
| 295 | + mock_client_instance.close.assert_called_once() |
| 296 | + |
| 297 | + @pytest.mark.asyncio |
| 298 | + async def test_singleton_concurrent_mounts_create_one_wrapper(self): |
| 299 | + """Concurrent mount() calls must not create more than one CopilotClientWrapper.""" |
| 300 | + import asyncio |
| 301 | + |
| 302 | + with patch("shutil.which", return_value="/usr/bin/copilot"): |
| 303 | + with patch("amplifier_module_provider_github_copilot._ensure_executable"): |
| 304 | + with patch( |
| 305 | + "amplifier_module_provider_github_copilot.CopilotClientWrapper" |
| 306 | + ) as mock_wrapper_cls: |
| 307 | + mock_wrapper_cls.return_value = Mock() |
| 308 | + |
| 309 | + def make_coordinator(): |
| 310 | + c = Mock() |
| 311 | + c.mount = AsyncMock() |
| 312 | + c.hooks = Mock() |
| 313 | + c.hooks.emit = AsyncMock() |
| 314 | + return c |
| 315 | + |
| 316 | + coordinators = [make_coordinator() for _ in range(5)] |
| 317 | + await asyncio.gather(*[mount(c, {}) for c in coordinators]) |
| 318 | + |
| 319 | + # All five concurrent mounts must share ONE wrapper |
| 320 | + assert mock_wrapper_cls.call_count == 1 |
| 321 | + |
| 322 | + import amplifier_module_provider_github_copilot as mod |
| 323 | + |
| 324 | + assert mod._shared_client_refcount == 5 |
| 325 | + |
| 326 | + @pytest.mark.asyncio |
| 327 | + async def test_singleton_logs_debug_on_timeout_mismatch(self, caplog): |
| 328 | + """Mismatched timeout on second mount emits DEBUG log, does not raise.""" |
| 329 | + import logging |
| 330 | + |
| 331 | + with patch("shutil.which", return_value="/usr/bin/copilot"): |
| 332 | + with patch("amplifier_module_provider_github_copilot._ensure_executable"): |
| 333 | + with patch( |
| 334 | + "amplifier_module_provider_github_copilot.CopilotClientWrapper" |
| 335 | + ) as mock_wrapper_cls: |
| 336 | + mock_wrapper_cls.return_value = Mock(_timeout=300.0) |
| 337 | + |
| 338 | + coordinator_a = Mock() |
| 339 | + coordinator_a.mount = AsyncMock() |
| 340 | + coordinator_a.hooks = Mock() |
| 341 | + coordinator_a.hooks.emit = AsyncMock() |
| 342 | + |
| 343 | + coordinator_b = Mock() |
| 344 | + coordinator_b.mount = AsyncMock() |
| 345 | + coordinator_b.hooks = Mock() |
| 346 | + coordinator_b.hooks.emit = AsyncMock() |
| 347 | + |
| 348 | + await mount(coordinator_a, {"timeout": 300.0}) |
| 349 | + |
| 350 | + with caplog.at_level(logging.DEBUG): |
| 351 | + cleanup = await mount(coordinator_b, {"timeout": 600.0}) |
| 352 | + |
| 353 | + assert cleanup is not None # No exception raised |
| 354 | + assert "Ignoring timeout" in caplog.text |
| 355 | + assert mock_wrapper_cls.call_count == 1 # Still only one wrapper |
0 commit comments