diff --git a/doc/mcp.md b/doc/mcp.md index aa2a8ebb4..8d989d622 100644 --- a/doc/mcp.md +++ b/doc/mcp.md @@ -123,9 +123,10 @@ Runs tests in a Dart or Flutter project. "tool": "test", "arguments": { "directory": "./my_app", + "paths": ["test/src/foo_test.dart", "test/widgets"], "dart": false, "coverage": true, - "recursive": true, + "recursive": false, "optimization": true, "concurrency": "4", "min_coverage": "100", @@ -145,6 +146,8 @@ Runs tests in a Dart or Flutter project. All parameters are optional. When `optimization` is not specified, `--no-optimization` is applied by default. When `timeout_seconds` is not specified, no timeout is applied. +`directory` selects the package to test; `paths` selects test files or directories *within* that package. To run another package's tests, point `directory` at it — putting a package path in `paths` runs the wrong thing, or nothing at all. Passing `paths` disables the test optimization step, and it cannot be combined with `recursive`, since each package in a recursive run gets the same relative path and only one of them can resolve it. + ### `packages_get` Installs or updates Dart/Flutter package dependencies. diff --git a/lib/src/cli/test_cli_runner.dart b/lib/src/cli/test_cli_runner.dart index 3d6c42c06..cf471621f 100644 --- a/lib/src/cli/test_cli_runner.dart +++ b/lib/src/cli/test_cli_runner.dart @@ -62,14 +62,19 @@ class MinCoverageNotMet implements Exception { class TestCLIRunner { /// Determines whether the user is targetting test files or not. /// - /// The user can only target test files by using the `--` option terminator. - /// The additional options after the `--` are passed to the test runner which - /// allows the user to target specific test files or directories. + /// [rest] holds the positional arguments left over after option parsing. + /// Trailing options are allowed, so bare targets work + /// (`very_good test test/foo_test.dart`); the `--` option terminator is only + /// needed for a target that begins with `-`, which would otherwise be read + /// as an option. Either way the parser strips the `--` out of [rest], so + /// what arrives here is the targets alone. /// /// The heuristics used to determine if the user is not targetting test files /// are: /// * No [rest] arguments are passed. - /// * All [rest] arguments are options (i.e. they do not start with `-`). + /// * Every [rest] argument is an option (i.e. it starts with `-`), which + /// only happens when the options were passed through a `--` for the + /// underlying test runner. /// /// See also: /// * [What does -- mean in Shell?](https://www.cyberciti.biz/faq/what-does-double-dash-mean-in-ssh-command/) diff --git a/lib/src/commands/dart/commands/dart_test_command.dart b/lib/src/commands/dart/commands/dart_test_command.dart index b5dabf9ae..ad42190ea 100644 --- a/lib/src/commands/dart/commands/dart_test_command.dart +++ b/lib/src/commands/dart/commands/dart_test_command.dart @@ -360,6 +360,15 @@ class DartTestCommand extends Command { final pubspec = File(path.join(targetPath, 'pubspec.yaml')); final recursive = _argResults['recursive'] as bool; + if (recursive && TestCLIRunner.isTargettingTestFiles(_argResults.rest)) { + _logger.err(''' +Cannot target specific test files together with --recursive. +Test targets are resolved against a single package root, so the same path +cannot apply to every package. Drop --recursive and run from the package +that contains them.'''); + return ExitCode.usage.code; + } + if (!recursive && !pubspec.existsSync()) { _logger.err(''' Could not find a pubspec.yaml in $targetPath. diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index ca565a4f9..d08fadf23 100644 --- a/lib/src/commands/test/test.dart +++ b/lib/src/commands/test/test.dart @@ -438,6 +438,15 @@ class TestCommand extends Command { final pubspec = File(path.join(targetPath, 'pubspec.yaml')); final recursive = _argResults['recursive'] as bool; + if (recursive && TestCLIRunner.isTargettingTestFiles(_argResults.rest)) { + _logger.err(''' +Cannot target specific test files together with --recursive. +Test targets are resolved against a single package root, so the same path +cannot apply to every package. Drop --recursive and run from the package +that contains them.'''); + return ExitCode.usage.code; + } + if (!recursive && !pubspec.existsSync()) { _logger.err(''' Could not find a pubspec.yaml in $targetPath. diff --git a/lib/src/mcp/mcp_server.dart b/lib/src/mcp/mcp_server.dart index 7b80a8125..79d1ebc05 100644 --- a/lib/src/mcp/mcp_server.dart +++ b/lib/src/mcp/mcp_server.dart @@ -170,8 +170,21 @@ If is omitted, then core will be selected. properties: { 'directory': StringSchema( description: - 'Target directory path (defaults to current directory). ' - 'Can be absolute or relative path to project root.', + 'The package to test (defaults to current directory). ' + 'Can be absolute or relative path to project root. ' + 'A package path belongs here, not in "paths".', + ), + 'paths': ListSchema( + description: + 'Test files or directories to run, relative to the package ' + 'selected by "directory" (e.g. ' + "['test/src/foo_test.dart', 'test/widgets']). These are " + 'targets inside that one package: to test a different ' + 'package, set "directory" to it rather than putting its ' + 'path here. When omitted, the whole suite runs. Cannot be ' + 'combined with "recursive". Note that targeting specific ' + 'paths disables the test optimization step.', + items: StringSchema(), ), 'dart': BooleanSchema( description: '''Whether to run Dart tests. If not specified, Flutter tests will be run if a Flutter project is detected.''', @@ -180,7 +193,11 @@ If is omitted, then core will be selected. description: 'Whether to collect coverage information.', ), 'recursive': BooleanSchema( - description: 'Run tests recursively for all nested packages.', + description: + 'Run tests recursively for all nested packages. ' + 'Cannot be combined with "paths", because each package runs ' + 'in its own working directory and a path is only meaningful ' + 'within one of them.', ), 'optimization': BooleanSchema( description: ''' @@ -448,6 +465,16 @@ Only one value can be selected. ]); } + // Positional test targets go last, after every option, so that they are + // parsed as `rest` rather than as a value for the preceding option. The + // `--` terminator keeps a target that begins with `-` from being read as + // an option; the parser strips it back out of `rest`, so the test command + // sees the paths and nothing else. + final paths = args['paths'] as List?; + if (paths != null && paths.isNotEmpty) { + cliArgs.addAll(['--', ...paths.cast()]); + } + return cliArgs; } diff --git a/test/src/commands/dart/commands/dart_test_test.dart b/test/src/commands/dart/commands/dart_test_test.dart index 589183c29..47e3be231 100644 --- a/test/src/commands/dart/commands/dart_test_test.dart +++ b/test/src/commands/dart/commands/dart_test_test.dart @@ -760,6 +760,43 @@ void main() { }, ); + test( + 'exits with usage code when targeting test files with --recursive', + () async { + when(() => argResults['recursive']).thenReturn(true); + when(() => argResults.rest).thenReturn(['test/my_test.dart']); + + final result = await testCommand.run(); + + expect(result, equals(ExitCode.usage.code)); + verify( + () => logger.err(any(that: contains('--recursive'))), + ).called(1); + }, + ); + + test( + 'allows --recursive when rest arguements are all options', + () async { + when(() => argResults['recursive']).thenReturn(true); + when(() => argResults.rest).thenReturn(['--track-wdiget-creation']); + + final result = await testCommand.run(); + + expect(result, equals(ExitCode.success.code)); + verify( + () => dartTest( + recursive: true, + optimizePerformance: true, + arguments: [...defaultArguments, '--track-wdiget-creation'], + logger: logger, + stdout: logger.write, + stderr: logger.err, + ), + ).called(1); + }, + ); + test( 'enables optimizePerformance when rest arguement is an option', () async { diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index 4dd2070ac..252e9b453 100644 --- a/test/src/commands/test/test_test.dart +++ b/test/src/commands/test/test_test.dart @@ -539,6 +539,43 @@ void main() { }, ); + test( + 'exits with usage code when targeting test files with --recursive', + () async { + when(() => argResults['recursive']).thenReturn(true); + when(() => argResults.rest).thenReturn(['test/my_test.dart']); + + final result = await testCommand.run(); + + expect(result, equals(ExitCode.usage.code)); + verify( + () => logger.err(any(that: contains('--recursive'))), + ).called(1); + }, + ); + + test( + 'allows --recursive when rest arguements are all options', + () async { + when(() => argResults['recursive']).thenReturn(true); + when(() => argResults.rest).thenReturn(['--track-wdiget-creation']); + + final result = await testCommand.run(); + + expect(result, equals(ExitCode.success.code)); + verify( + () => flutterTest( + recursive: true, + optimizePerformance: true, + arguments: [...defaultArguments, '--track-wdiget-creation'], + logger: logger, + stdout: logger.write, + stderr: logger.err, + ), + ).called(1); + }, + ); + test( 'enables optimizePerformance when rest arguement is an option', () async { diff --git a/test/src/mcp/mcp_server_test.dart b/test/src/mcp/mcp_server_test.dart index 6c36ce9a3..0ce7b5ddb 100644 --- a/test/src/mcp/mcp_server_test.dart +++ b/test/src/mcp/mcp_server_test.dart @@ -502,6 +502,88 @@ void main() { as List; expect(capturedArgs, equals(['test', '--timeout', '120'])); }); + + test('passes paths as positional test targets behind `--`', () async { + await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: { + 'paths': ['test/src/foo_test.dart', 'test/widgets'], + }, + ), + ), + ); + + final capturedArgs = + verify(() => mockCommandRunner.run(captureAny())).captured.first + as List; + expect( + capturedArgs, + equals(['test', '--', 'test/src/foo_test.dart', 'test/widgets']), + ); + }); + + test('passes paths after options so they are parsed as rest', () async { + await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: { + 'dart': true, + 'concurrency': '8', + 'paths': ['test/src/foo_test.dart'], + }, + ), + ), + ); + + final capturedArgs = + verify(() => mockCommandRunner.run(captureAny())).captured.first + as List; + expect( + capturedArgs, + equals(['dart', 'test', '-j', '8', '--', 'test/src/foo_test.dart']), + ); + }); + + test('passes a path beginning with `-` through unparsed', () async { + await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: { + 'paths': ['-weird_test.dart'], + }, + ), + ), + ); + + final capturedArgs = + verify(() => mockCommandRunner.run(captureAny())).captured.first + as List; + expect(capturedArgs, equals(['test', '--', '-weird_test.dart'])); + }); + + test('adds no positional targets when paths is empty', () async { + await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: {'paths': []}, + ), + ), + ); + + final capturedArgs = + verify(() => mockCommandRunner.run(captureAny())).captured.first + as List; + expect(capturedArgs, equals(['test'])); + }); }); group('Tool: packages_get', () {