From 9c960ff7dad1df3751af4a0fde9cb9f04047e200 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 22 Jan 2025 20:39:13 -0800 Subject: [PATCH] Add a better error message when `flutter drive --target` is used incorrectly. (#162023) Closes https://github.com/flutter/flutter/issues/62626. --- .../flutter_tools/lib/src/commands/drive.dart | 9 ++ .../commands.shard/hermetic/drive_test.dart | 88 +++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index 9fb1750a0d..3cf3a74614 100644 --- a/packages/flutter_tools/lib/src/commands/drive.dart +++ b/packages/flutter_tools/lib/src/commands/drive.dart @@ -284,6 +284,15 @@ class DriveCommand extends RunCommandBase { throwToolExit(null); } if (await _fileSystem.type(testFile) != FileSystemEntityType.file) { + // A very common source of error is holding "flutter drive" wrong, + // and providing the "test_driver/foo_test.dart" as the target, when + // the intention was to provide "lib/foo.dart". + if (_fileSystem.path.isWithin('test_driver', targetFile)) { + _logger.printError( + 'The file path passed to --target should be an app entrypoint that ' + 'contains a "main()". Did you mean "flutter drive --driver $targetFile"?', + ); + } throwToolExit('Test file not found: $testFile'); } final Device? device = await targetedDevice; diff --git a/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart index d74ac3ce16..45357b9bfb 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart @@ -53,6 +53,94 @@ void main() { Cache.enableLocking(); }); + testUsingContext( + 'fails if the specified --target is not found', + () async { + final DriveCommand command = DriveCommand( + fileSystem: fileSystem, + logger: logger, + platform: platform, + signals: signals, + ); + fileSystem.file('lib/main.dart').createSync(recursive: true); + fileSystem.file('test_driver/main_test.dart').createSync(recursive: true); + fileSystem.file('pubspec.yaml').createSync(); + + await expectLater( + () => createTestCommandRunner( + command, + ).run(['drive', '--no-pub', '--target', 'lib/app.dart']), + throwsToolExit(message: 'Target file "lib/app.dart" not found'), + ); + + expect(logger.errorText, isEmpty); + expect(logger.statusText, isEmpty); + }, + overrides: { + FileSystem: () => fileSystem, + ProcessManager: () => FakeProcessManager.empty(), + }, + ); + + testUsingContext( + 'fails if the default --target is not found', + () async { + final DriveCommand command = DriveCommand( + fileSystem: fileSystem, + logger: logger, + platform: platform, + signals: signals, + ); + fileSystem.file('lib/app.dart').createSync(recursive: true); + fileSystem.file('test_driver/app_test.dart').createSync(recursive: true); + fileSystem.file('pubspec.yaml').createSync(); + + await expectLater( + () => createTestCommandRunner(command).run(['drive', '--no-pub']), + throwsToolExit(message: 'Target file "lib/main.dart" not found'), + ); + + expect(logger.errorText, isEmpty); + expect(logger.statusText, isEmpty); + }, + overrides: { + FileSystem: () => fileSystem, + ProcessManager: () => FakeProcessManager.empty(), + }, + ); + + testUsingContext( + 'fails with an informative error message if --target looks like --driver', + () async { + final DriveCommand command = DriveCommand( + fileSystem: fileSystem, + logger: logger, + platform: platform, + signals: signals, + ); + fileSystem.file('lib/main.dart').createSync(recursive: true); + fileSystem.file('test_driver/main_test.dart').createSync(recursive: true); + fileSystem.file('pubspec.yaml').createSync(); + + await expectLater( + () => createTestCommandRunner( + command, + ).run(['drive', '--no-pub', '--target', 'test_driver/main_test.dart']), + throwsToolExit(message: 'Test file not found: /test_driver/main_test_test.dart'), + ); + + expect( + logger.errorText, + contains('The file path passed to --target should be an app entrypoint'), + ); + expect(logger.statusText, isEmpty); + }, + overrides: { + FileSystem: () => fileSystem, + ProcessManager: () => FakeProcessManager.empty(), + }, + ); + testUsingContext( 'warns if screenshot is not supported but continues test', () async {