test_ensure_emulator_uses_windows_creation_flags can only pass on Windows
- Dominant language
- Python
- Stars
- 5.9k
- Forks
- 516
- Avg merge
- 22m
- Merged PRs (30d)
- 5
Description
## Problem
`tests/unit/mcp/test_device_utils.py::test_ensure_emulator_uses_windows_creation_flags` fails on every non-Windows host, including CI's own `ubuntu-latest` runner. `CONTRIBUTING.md` describes `make test` as the deterministic suite that needs no device, credentials or private services, and as "the same suite used by pull-request CI", so this test cannot currently hold up its end of that on Linux or macOS.
## Root cause
The test deliberately exercises the Windows branch from any host by patching `sys.platform`:
```python
monkeypatch.setattr(device_utils.sys, "platform", "win32")
```
That branch (`mcp_server/utils/device_utils.py:143-150`) builds:
```python
creationflags=(subprocess.CREATE_NEW_PROCESS_GROUP | subprocess.DETACHED_PROCESS),
```
Both attributes exist **only** on Windows:
```console
$ python -c "import subprocess; print(hasattr(subprocess,'CREATE_NEW_PROCESS_GROUP'))"
False
```
So on POSIX the expression raises `AttributeError` before `Popen` is ever called. `ensure_emulator`'s `except Exception: return False` (`device_utils.py:159-160`) swallows it and returns `False`, and the test fails on its first assertion with no indication of why:
```console
$ uv run pytest -q tests/unit/mcp/test_device_utils.py
tests/unit/mcp/test_device_utils.py:22: in test_ensure_emulator_uses_windows_creation_flags
assert device_utils.ensure_emulator(
E AssertionError: assert False
1 failed, 1 passed
```
The sibling `test_ensure_emulator_starts_new_session_on_posix` passes, because `start_new_session` is portable.
## Steps to reproduce
1. Check out `main` (`371aa6d`) on Linux or macOS
2. `uv sync --dev`
3. `uv run pytest -q tests/unit/mcp/test_device_utils.py`
Reproduced on Linux aarch64, Python 3.12.12, with the committed `uv.lock`.
## Why this has gone unnoticed
The test step in `.github/workflows/ci.yml` is gated behind `if: github.event_name == 'workflow_dispatch'`, so pushes and pull requests never run the suite (already noted in #18). GitHub's `windows-latest` image would pass this test anyway; `ubuntu-latest` would not.
## Suggested fix
Stub the two Windows-only constants in the test with their documented values, so the `win32` branch is reachable from any host:
```python
monkeypatch.setattr(
device_utils.subprocess, "CREATE_NEW_PROCESS_GROUP", 0x00000200, raising=False
)
monkeypatch.setattr(device_utils.subprocess, "DETACHED_PROCESS", 0x00000008, raising=False)
```
The assertion compares against the same two attributes, so it stays meaningful: it still verifies that the Windows branch passes `creationflags` and omits `start_new_session`.
## Separate, related observation
`ensure_emulator` wraps the whole spawn in `except Exception: return False`. That is what converted a programming error (`AttributeError` on a missing attribute) into a silent "the emulator could not be started", and is why this failure is so opaque. Narrowing it to `(OSError, subprocess.SubprocessError)` — matching the `except` already used in the boot-polling loop below it — would surface bugs like this instead of hiding them. That changes runtime behaviour for users, so I have deliberately left it out of the fix and am raising it here for a maintainer to decide on.
I have a patch for the test-side fix and will send it shortly.
Contributor guide
Research direction
Start with tests/unit/mcp/test_device_utils.py::test_ensure_emulator_uses_windows_creation_flags and the Windows branch in mcp_server/utils/device_utils.py:143-150. Run uv run pytest -q tests/unit/mcp/test_device_utils.py, then make the test provide the documented Windows constants so it can exercise the win32 branch on non-Windows hosts. Done means the test passes while still checking creationflags and omitting start_new_session.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 90/100