google / google/artemis

test_mobile_inspect_trace_invalid_action passes or fails based on unrelated on-disk state

Open Beginner friendly
#62 2 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.