feat(mcp): allow the test tool to target specific paths - #1676
Open
HadiHassan22 wants to merge 1 commit into
Open
feat(mcp): allow the test tool to target specific paths#1676HadiHassan22 wants to merge 1 commit into
test tool to target specific paths#1676HadiHassan22 wants to merge 1 commit into
Conversation
The `test` tool exposed no way to run a subset of a suite, so every call ran everything. The CLI already supports positional test targets — the test command forwards `argResults.rest` to the runner — but the MCP tool had no argument that reached them, and `directory` is deliberately applied as the working directory rather than as a target. Adds an optional `paths` array that is appended after every option, so the args land in `rest`. Behaviour matches the CLI, including the existing rule that targeting specific files disables test optimization.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes #1675.
The
testMCP tool had no way to run a subset of a suite, so every call ran the whole thing. There was no workaround through the existing arguments:directoryis deliberately applied as the working directory rather than as a target (per the existingNOTEin_parseTest), andtags/exclude_tagsonly filter on annotations already present in the test source.The CLI has no such gap —
very_good test test/foo_test.dartworks today, because the test commands keepargResults.restand forward it to the runner. This just exposes that through MCP.Changes
pathsargument to thetesttool: a list of strings, each a test file or directory._parseTestappends them after every option, so they are parsed asrestrather than consumed as the value of a preceding option.very_good testandvery_good dart test; omitting the argument leaves behaviour exactly as before.The argument description also notes that targeting specific paths disables the test optimization step. That is pre-existing CLI behaviour via
TestCLIRunner.isTargettingTestFiles, not something this PR changes — it just makes it discoverable to a caller who can no longer see the command line.Why this matters for MCP callers
An agent driving the CLI through MCP was strictly less capable than one shelling out. Beyond the wasted wall-clock of running everything to check one directory, the tool returns the runner's full output, so a suite whose failure produces a large widget-tree or stack dump can return well over 100k characters — nearly all of it irrelevant to the tests the caller cared about. Narrowing the run is the cheapest mitigation available.
Testing
Three tests added to
test/src/mcp/mcp_server_test.dart, covering both branches of the new code:dart+concurrency)Verified locally:
dart format lib test— no changesdart analyze --fatal-infos --fatal-warnings lib test— no issuesvery_good dart test -x pull-request-only— 516 passingOpen questions
Happy to change the argument name (
pathsvstargetsvstest_paths) or its shape — I used a string array, thoughpackages_get'signoreuses a comma-separated string, so let me know if you'd rather stay consistent with that.