test_mobile_inspect_trace_invalid_action passes or fails based on unrelated on-disk state
- Dominant language
- Python
- Stars
- 5.9k
- Forks
- 516
- Avg merge
- 22m
- Merged PRs (30d)
- 5
Description
## Problem
`tests/unit/mcp/test_mcp_tools.py::test_mobile_inspect_trace_invalid_action` does not use the `temp_trace_env` fixture, so it calls `mobile_inspect_trace` against whatever `trace_store.TRACES_DIR` resolves to on the machine running the tests, which defaults to `/traces`. `mobile_inspect_trace` checks for a `data_engine.db` file in that directory before it looks at the `action` argument at all. When the file is missing, the tool returns a "Database not found" error unconditionally, so the test's `action="invalid_action"` case never reaches the code path it is meant to check.
On a machine where `/traces/data_engine.db` already exists, from an earlier local run, an earlier test in the same session, or a leftover dev artifact, the test passes by coincidence without ever exercising the "unsupported action" branch. On a clean checkout, or any environment where that file does not exist yet, the same test fails with an unrelated database error instead of the assertion it is written to check.
## Root cause
In `mcp_server/tools/inspect_trace.py`, `mobile_inspect_trace()` resolves `db_path` from `trace_store.TRACES_DIR` (or a `project_root/traces` fallback) and returns early with a "Database not found" message if that path does not exist, before the `action` parameter is validated. The actual "Action '...' is not supported" branch only runs after that early return. The test never patches `trace_store.TRACES_DIR` and never seeds a database, so its result depends on incidental on disk state rather than the isolated condition it claims to test.
## Safe reproduction
Run the test by itself on a checkout with no `traces/data_engine.db` under the project root:
pytest tests/unit/mcp/test_mcp_tools.py::test_mobile_inspect_trace_invalid_action
Result:
AssertionError: assert 'not supported' in 'Data engine database does not exist at /traces/data_engine.db yet.'
Creating any file, empty or not, at that path makes the test pass, since the only gate before the final else branch is `os.path.exists(db_path)`. The action never touches the database contents for an unknown action string, so the test's real intent (validating unknown-action handling) is never actually exercised either way.
## Possible fix direction
Give this test its own isolated trace environment, the same way the other tests in this module do, for example by requesting `temp_trace_env` and writing an empty `data_engine.db` into it before calling `mobile_inspect_trace`. That would make the pass or fail result depend only on the action-validation logic, not on whatever is left on disk.
Contributor guide
Research direction
Start with tests/unit/mcp/test_mcp_tools.py::test_mobile_inspect_trace_invalid_action and compare its setup with the module's temp_trace_env fixture. Ensure the test uses an isolated trace directory and provides the expected data_engine.db before calling mobile_inspect_trace. Run the test alone with pytest and confirm it consistently checks the unsupported-action error rather than on-disk database state.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100