[flutter_tools] Add support for URI formats like ?line=x for "flutter…

… test" (#119740) * [flutter_tools] Add support for URI formats like ?line=x for "flutter test" * Remove unnecessary function * Handle parsing absolute paths on Windows * Use Windows-style paths when running on Windows * Fix paths in isFile * Remove unnecessary clear
engine-flutter-autoroll · Feb 27, 2023 · 06952ba · 06952ba
1 parent 9a4e897
commit 06952ba
Show file tree

Hide file tree

Showing 6 changed files with 91 additions and 33 deletions.
diff --git a/dev/automated_tests/flutter_test/uri_format_test.dart b/dev/automated_tests/flutter_test/uri_format_test.dart
@@ -0,0 +1,17 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:test/test.dart' hide TypeMatcher, isInstanceOf;
+
+void main() {
+  // This test must start on Line 11 or the test
+  // "flutter test should run a test by line number in URI format"
+  // in test/integration.shard/test_test.dart updated.
+  test('exactTestName', () {
+    expect(2 + 2, 4);
+  });
+  test('not exactTestName', () {
+    throw 'this test should have been filtered out';
+  });
+}
diff --git a/packages/flutter_tools/bin/fuchsia_tester.dart b/packages/flutter_tools/bin/fuchsia_tester.dart
@@ -138,7 +138,7 @@ Future<void> run(List<String> args) async {
     // TODO(dnfield): This should be injected.
     exitCode = await const FlutterTestRunner().runTests(
       const TestWrapper(),
-      tests.keys.toList(),
+      tests.keys.map(Uri.file).toList(),
       debuggingOptions: DebuggingOptions.enabled(
         BuildInfo(
           BuildMode.debug,

diff --git a/packages/flutter_tools/lib/src/commands/test.dart b/packages/flutter_tools/lib/src/commands/test.dart
@@ -236,7 +236,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
   bool get isIntegrationTest => _isIntegrationTest;
   bool _isIntegrationTest = false;
 
-  List<String> _testFiles = <String>[];
+  final Set<Uri> _testFileUris = <Uri>{};
 
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async {
@@ -261,37 +261,43 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
 
   @override
   Future<FlutterCommandResult> verifyThenRunCommand(String? commandPath) {
-    _testFiles = argResults!.rest.map<String>(globals.fs.path.absolute).toList();
-    if (_testFiles.isEmpty) {
+    final List<Uri> testUris = argResults!.rest.map(_parseTestArgument).toList();
+    if (testUris.isEmpty) {
       // We don't scan the entire package, only the test/ subdirectory, so that
       // files with names like "hit_test.dart" don't get run.
       final Directory testDir = globals.fs.directory('test');
       if (!testDir.existsSync()) {
         throwToolExit('Test directory "${testDir.path}" not found.');
       }
-      _testFiles = _findTests(testDir).toList();
-      if (_testFiles.isEmpty) {
+      _testFileUris.addAll(_findTests(testDir).map(Uri.file));
+      if (_testFileUris.isEmpty) {
         throwToolExit(
           'Test directory "${testDir.path}" does not appear to contain any test files.\n'
           'Test files must be in that directory and end with the pattern "_test.dart".'
         );
       }
     } else {
-      _testFiles = <String>[
-        for (String path in _testFiles)
-          if (globals.fs.isDirectorySync(path))
-            ..._findTests(globals.fs.directory(path))
-          else
-            globals.fs.path.normalize(globals.fs.path.absolute(path)),
-      ];
+      for (final Uri uri in testUris) {
+        // Test files may have query strings to support name/line/col:
+        //     flutter test test/foo.dart?name=a&line=1
+        String testPath = uri.replace(query: '').toFilePath();
+        testPath = globals.fs.path.absolute(testPath);
+        testPath = globals.fs.path.normalize(testPath);
+        if (globals.fs.isDirectorySync(testPath)) {
+          _testFileUris.addAll(_findTests(globals.fs.directory(testPath)).map(Uri.file));
+        } else {
+          _testFileUris.add(Uri.file(testPath).replace(query: uri.query));
+        }
+      }
     }
 
     // This needs to be set before [super.verifyThenRunCommand] so that the
     // correct [requiredArtifacts] can be identified before [run] takes place.
-    _isIntegrationTest = _shouldRunAsIntegrationTests(globals.fs.currentDirectory.absolute.path, _testFiles);
+    final List<String> testFilePaths = _testFileUris.map((Uri uri) => uri.replace(query: '').toFilePath()).toList();
+    _isIntegrationTest = _shouldRunAsIntegrationTests(globals.fs.currentDirectory.absolute.path, testFilePaths);
 
     globals.printTrace(
-      'Found ${_testFiles.length} files which will be executed as '
+      'Found ${_testFileUris.length} files which will be executed as '
       '${_isIntegrationTest ? 'Integration' : 'Widget'} Tests.',
     );
     return super.verifyThenRunCommand(commandPath);
@@ -338,7 +344,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
     }
 
     final bool startPaused = boolArgDeprecated('start-paused');
-    if (startPaused && _testFiles.length != 1) {
+    if (startPaused && _testFileUris.length != 1) {
       throwToolExit(
         'When using --start-paused, you must specify a single test file to run.',
         exitCode: 1,
@@ -451,7 +457,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
     final Stopwatch? testRunnerTimeRecorderStopwatch = testTimeRecorder?.start(TestTimePhases.TestRunner);
     final int result = await testRunner.runTests(
       testWrapper,
-      _testFiles,
+      _testFileUris.toList(),
       debuggingOptions: debuggingOptions,
       names: names,
       plainNames: plainNames,
@@ -500,6 +506,22 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
     return FlutterCommandResult.success();
   }
 
+  /// Parses a test file/directory target passed as an argument and returns it
+  /// as an absolute file:/// [URI] with optional querystring for name/line/col.
+  Uri _parseTestArgument(String arg) {
+    // We can't parse Windows paths as URIs if they have query strings, so
+    // parse the file and query parts separately.
+    final int queryStart = arg.indexOf('?');
+    String filePart = queryStart == -1 ? arg : arg.substring(0, queryStart);
+    final String queryPart = queryStart == -1 ? '' : arg.substring(queryStart + 1);
+
+    filePart = globals.fs.path.absolute(filePart);
+    filePart = globals.fs.path.normalize(filePart);
+
+    return Uri.file(filePart)
+        .replace(query: queryPart.isEmpty ? null : queryPart);
+  }
+
   Future<void> _buildTestAsset() async {
     final AssetBundle assetBundle = AssetBundleFactory.instance.createBundle();
     final int build = await assetBundle.build(packagesPath: '.packages');

diff --git a/packages/flutter_tools/lib/src/test/runner.dart b/packages/flutter_tools/lib/src/test/runner.dart
@@ -24,7 +24,7 @@ abstract class FlutterTestRunner {
   /// Runs tests using package:test and the Flutter engine.
   Future<int> runTests(
     TestWrapper testWrapper,
-    List<String> testFiles, {
+    List<Uri> testFiles, {
     required DebuggingOptions debuggingOptions,
     List<String> names = const <String>[],
     List<String> plainNames = const <String>[],
@@ -62,7 +62,7 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner {
   @override
   Future<int> runTests(
     TestWrapper testWrapper,
-    List<String> testFiles, {
+    List<Uri> testFiles, {
     required DebuggingOptions debuggingOptions,
     List<String> names = const <String>[],
     List<String> plainNames = const <String>[],
@@ -128,6 +128,7 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner {
         '--shard-index=$shardIndex',
       '--chain-stack-traces',
     ];
+
     if (web) {
       final String tempBuildDir = globals.fs.systemTempDirectory
         .createTempSync('flutter_test.')
@@ -144,13 +145,13 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner {
       ).initialize(
         projectDirectory: flutterProject!.directory,
         testOutputDir: tempBuildDir,
-        testFiles: testFiles,
+        testFiles: testFiles.map((Uri uri) => uri.toFilePath()).toList(),
         buildInfo: debuggingOptions.buildInfo,
       );
       testArgs
         ..add('--platform=chrome')
         ..add('--')
-        ..addAll(testFiles);
+        ..addAll(testFiles.map((Uri uri) => uri.toString()));
       testWrapper.registerPlatformPlugin(
         <Runtime>[Runtime.chrome],
         () {
@@ -186,7 +187,7 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner {
 
     testArgs
       ..add('--')
-      ..addAll(testFiles);
+      ..addAll(testFiles.map((Uri uri) => uri.toString()));
 
     final InternetAddressType serverType =
         ipv6 ? InternetAddressType.IPv6 : InternetAddressType.IPv4;

diff --git a/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/test_test.dart
@@ -13,6 +13,7 @@ import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/commands/test.dart';
 import 'package:flutter_tools/src/device.dart';
+import 'package:flutter_tools/src/globals.dart' as globals;
 import 'package:flutter_tools/src/project.dart';
 import 'package:flutter_tools/src/runner/flutter_command.dart';
 import 'package:flutter_tools/src/test/runner.dart';
@@ -59,17 +60,18 @@ void main() {
   late LoggingLogger logger;
 
   setUp(() {
-    fs = MemoryFileSystem.test();
-    fs.file('/package/pubspec.yaml').createSync(recursive: true);
-    fs.file('/package/pubspec.yaml').writeAsStringSync(_pubspecContents);
-    (fs.directory('/package/.dart_tool')
+    fs = MemoryFileSystem.test(style: globals.platform.isWindows ? FileSystemStyle.windows : FileSystemStyle.posix);
+    final Directory package = fs.directory('package');
+    package.childFile('pubspec.yaml').createSync(recursive: true);
+    package.childFile('pubspec.yaml').writeAsStringSync(_pubspecContents);
+    (package.childDirectory('.dart_tool')
         .childFile('package_config.json')
       ..createSync(recursive: true))
         .writeAsString(_packageConfigContents);
-    fs.directory('/package/test').childFile('some_test.dart').createSync(recursive: true);
-    fs.directory('/package/integration_test').childFile('some_integration_test.dart').createSync(recursive: true);
+    package.childDirectory('test').childFile('some_test.dart').createSync(recursive: true);
+    package.childDirectory('integration_test').childFile('some_integration_test.dart').createSync(recursive: true);
 
-    fs.currentDirectory = '/package';
+    fs.currentDirectory = package.path;
 
     logger = LoggingLogger();
   });
@@ -721,7 +723,7 @@ dev_dependencies:
       '--no-pub',
     ]);
 
-    final bool fileExists = await fs.isFile('build/unit_test_assets/AssetManifest.json');
+    final bool fileExists = await fs.isFile(globals.fs.path.join('build', 'unit_test_assets', 'AssetManifest.json'));
     expect(fileExists, true);
 
   }, overrides: <Type, Generator>{
@@ -742,7 +744,7 @@ dev_dependencies:
       '--no-test-assets',
     ]);
 
-    final bool fileExists = await fs.isFile('build/unit_test_assets/AssetManifest.json');
+    final bool fileExists = await fs.isFile(globals.fs.path.join('build', 'unit_test_assets', 'AssetManifest.json'));
     expect(fileExists, false);
 
   }, overrides: <Type, Generator>{
@@ -854,7 +856,7 @@ class FakeFlutterTestRunner implements FlutterTestRunner {
   @override
   Future<int> runTests(
     TestWrapper testWrapper,
-    List<String> testFiles, {
+    List<Uri> testFiles, {
     required DebuggingOptions debuggingOptions,
     List<String> names = const <String>[],
     List<String> plainNames = const <String>[],

diff --git a/packages/flutter_tools/test/integration.shard/test_test.dart b/packages/flutter_tools/test/integration.shard/test_test.dart
@@ -165,6 +165,20 @@ void main() {
     expect(result.exitCode, 1);
   });
 
+  testWithoutContext('flutter test should run a test with an exact name in URI format', () async {
+    final ProcessResult result = await _runFlutterTest('uri_format', automatedTestsDirectory, flutterTestDirectory,
+      query: 'full-name=exactTestName');
+    expect(result.stdout, contains(RegExp(r'\+\d+: All tests passed!')));
+    expect(result.exitCode, 0);
+  });
+
+  testWithoutContext('flutter test should run a test by line number in URI format', () async {
+    final ProcessResult result = await _runFlutterTest('uri_format', automatedTestsDirectory, flutterTestDirectory,
+      query: 'line=11');
+    expect(result.stdout, contains(RegExp(r'\+\d+: All tests passed!')));
+    expect(result.exitCode, 0);
+  });
+
   testWithoutContext('flutter test should test runs to completion', () async {
     final ProcessResult result = await _runFlutterTest('trivial', automatedTestsDirectory, flutterTestDirectory,
       extraArguments: const <String>['--verbose']);
@@ -315,6 +329,7 @@ Future<ProcessResult> _runFlutterTest(
   String workingDirectory,
   String testDirectory, {
   List<String> extraArguments = const <String>[],
+  String? query,
 }) async {
 
   String testPath;
@@ -342,7 +357,8 @@ Future<ProcessResult> _runFlutterTest(
     '--reporter',
     'compact',
     ...extraArguments,
-    testPath,
+    if (query != null) Uri.file(testPath).replace(query: query).toString()
+    else testPath,
   ];
 
   return Process.run(