From 06952ba2549ec009fd82d24f571fdb9c263e2326 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Mon, 27 Feb 2023 19:13:00 +0000 Subject: [PATCH] [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 --- .../flutter_test/uri_format_test.dart | 17 ++++++ .../flutter_tools/bin/fuchsia_tester.dart | 2 +- .../flutter_tools/lib/src/commands/test.dart | 54 +++++++++++++------ .../flutter_tools/lib/src/test/runner.dart | 11 ++-- .../commands.shard/hermetic/test_test.dart | 22 ++++---- .../test/integration.shard/test_test.dart | 18 ++++++- 6 files changed, 91 insertions(+), 33 deletions(-) create mode 100644 dev/automated_tests/flutter_test/uri_format_test.dart diff --git a/dev/automated_tests/flutter_test/uri_format_test.dart b/dev/automated_tests/flutter_test/uri_format_test.dart new file mode 100644 index 0000000000..4af5200fe7 --- /dev/null +++ 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 index b18a4191b1..c0017d25a5 100644 --- a/packages/flutter_tools/bin/fuchsia_tester.dart +++ b/packages/flutter_tools/bin/fuchsia_tester.dart @@ -138,7 +138,7 @@ Future run(List 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 index 4c008f4e09..801856b76a 100644 --- 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 _testFiles = []; + final Set _testFileUris = {}; @override Future> get requiredArtifacts async { @@ -261,37 +261,43 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { @override Future verifyThenRunCommand(String? commandPath) { - _testFiles = argResults!.rest.map(globals.fs.path.absolute).toList(); - if (_testFiles.isEmpty) { + final List 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 = [ - 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 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 _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 index ea310feea5..cb0390c368 100644 --- 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 runTests( TestWrapper testWrapper, - List testFiles, { + List testFiles, { required DebuggingOptions debuggingOptions, List names = const [], List plainNames = const [], @@ -62,7 +62,7 @@ class _FlutterTestRunnerImpl implements FlutterTestRunner { @override Future runTests( TestWrapper testWrapper, - List testFiles, { + List testFiles, { required DebuggingOptions debuggingOptions, List names = const [], List plainNames = const [], @@ -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.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 index eb0296c716..ed189deed1 100644 --- 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: { @@ -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: { @@ -854,7 +856,7 @@ class FakeFlutterTestRunner implements FlutterTestRunner { @override Future runTests( TestWrapper testWrapper, - List testFiles, { + List testFiles, { required DebuggingOptions debuggingOptions, List names = const [], List plainNames = const [], diff --git a/packages/flutter_tools/test/integration.shard/test_test.dart b/packages/flutter_tools/test/integration.shard/test_test.dart index 0360030985..6cc3388281 100644 --- 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 ['--verbose']); @@ -315,6 +329,7 @@ Future _runFlutterTest( String workingDirectory, String testDirectory, { List extraArguments = const [], + String? query, }) async { String testPath; @@ -342,7 +357,8 @@ Future _runFlutterTest( '--reporter', 'compact', ...extraArguments, - testPath, + if (query != null) Uri.file(testPath).replace(query: query).toString() + else testPath, ]; return Process.run(