From 183870038818ceaabbef4024a6254641793c356d Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 23 Jun 2021 12:20:02 -0400 Subject: [PATCH 1/7] Enforce that exactly one platform is passed, since multiple aren't supported correctly in practice --- .../tool/lib/src/drive_examples_command.dart | 55 +++++++++----- .../test/drive_examples_command_test.dart | 74 +++++++++++++++---- 2 files changed, 93 insertions(+), 36 deletions(-) diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index 8a8cd6726d02..572b5e72a036 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -11,6 +11,8 @@ import 'common/plugin_command.dart'; import 'common/plugin_utils.dart'; import 'common/process_runner.dart'; +const int _exitBadPlatformFlags = 2; + /// A command to run the example applications for packages via Flutter driver. class DriveExamplesCommand extends PluginCommand { /// Creates an instance of the drive command. @@ -53,6 +55,29 @@ class DriveExamplesCommand extends PluginCommand { @override Future run() async { + final List platformSwitches = [ + kPlatformAndroid, + kPlatformIos, + kPlatformLinux, + kPlatformMacos, + kPlatformWeb, + kPlatformWindows, + ]; + final int platformCount = platformSwitches + .where((String platform) => getBoolArg(platform)) + .length; + // The way this command is written relies (for legacy reasons) on using the + // default device (which it assumes has already been set up) for iOS or + // Android. Therefore if an explicit target device is passed, it won't + // actually drive the mobile version. If that is fixed, the restriction that + // only one platform can be selected can be removed. + if (platformCount != 1) { + print( + 'Exactly one of ${platformSwitches.map((String platform) => '--$platform').join(', ')} ' + 'must be specified.'); + throw ToolExit(_exitBadPlatformFlags); + } + final List failingTests = []; final List pluginsWithoutTests = []; final bool isLinux = getBoolArg(kPlatformLinux); @@ -69,7 +94,7 @@ class DriveExamplesCommand extends PluginCommand { continue; } print('\n==========\nChecking $pluginName...'); - if (!(await _pluginSupportedOnCurrentPlatform(plugin))) { + if (!(await _pluginSupportedOnTargetPlatform(plugin))) { print('Not supported for the target platform; skipping.'); continue; } @@ -220,36 +245,26 @@ Tried searching for the following: print('All driver tests successful!'); } - Future _pluginSupportedOnCurrentPlatform( - FileSystemEntity plugin) async { - final bool isAndroid = getBoolArg(kPlatformAndroid); - final bool isIOS = getBoolArg(kPlatformIos); - final bool isLinux = getBoolArg(kPlatformLinux); - final bool isMacos = getBoolArg(kPlatformMacos); - final bool isWeb = getBoolArg(kPlatformWeb); - final bool isWindows = getBoolArg(kPlatformWindows); - if (isAndroid) { + Future _pluginSupportedOnTargetPlatform(FileSystemEntity plugin) async { + if (getBoolArg(kPlatformAndroid)) { return isAndroidPlugin(plugin); } - if (isIOS) { + if (getBoolArg(kPlatformIos)) { return isIosPlugin(plugin); } - if (isLinux) { + if (getBoolArg(kPlatformLinux)) { return isLinuxPlugin(plugin); } - if (isMacos) { + if (getBoolArg(kPlatformMacos)) { return isMacOsPlugin(plugin); } - if (isWeb) { + if (getBoolArg(kPlatformWeb)) { return isWebPlugin(plugin); } - if (isWindows) { + if (getBoolArg(kPlatformWindows)) { return isWindowsPlugin(plugin); } - // When we are here, no flags are specified. Only return true if the plugin - // supports Android for legacy command support. - // TODO(cyanglaz): Make Android flag also required like other platforms - // (breaking change). https://github.com/flutter/flutter/issues/58285 - return isAndroidPlugin(plugin); + assert(false); + return false; } } diff --git a/script/tool/test/drive_examples_command_test.dart b/script/tool/test/drive_examples_command_test.dart index 3175f7163546..695f956bb589 100644 --- a/script/tool/test/drive_examples_command_test.dart +++ b/script/tool/test/drive_examples_command_test.dart @@ -35,6 +35,21 @@ void main() { runner.addCommand(command); }); + test('Fails if no platforms are provided', () async { + expect( + () => runCapturingPrint(runner, ['drive-examples']), + throwsA(isA()), + ); + }); + + test('Fails if multiple platforms are provided', () async { + expect( + () => runCapturingPrint( + runner, ['drive-examples', '--ios', '--macos']), + throwsA(isA()), + ); + }); + test('driving under folder "test"', () async { final Directory pluginDirectory = createFakePlugin( 'plugin', @@ -52,9 +67,8 @@ void main() { final Directory pluginExampleDirectory = pluginDirectory.childDirectory('example'); - final List output = await runCapturingPrint(runner, [ - 'drive-examples', - ]); + final List output = + await runCapturingPrint(runner, ['drive-examples', '--ios']); expect( output, @@ -100,9 +114,8 @@ void main() { final Directory pluginExampleDirectory = pluginDirectory.childDirectory('example'); - final List output = await runCapturingPrint(runner, [ - 'drive-examples', - ]); + final List output = + await runCapturingPrint(runner, ['drive-examples', '--ios']); expect( output, @@ -190,9 +203,8 @@ void main() { final Directory pluginExampleDirectory = pluginDirectory.childDirectory('example'); - final List output = await runCapturingPrint(runner, [ - 'drive-examples', - ]); + final List output = + await runCapturingPrint(runner, ['drive-examples', '--ios']); expect( output, @@ -537,7 +549,7 @@ void main() { ])); }); - test('driving when plugin does not support mobile is no-op', () async { + test('driving when plugin does not support Android is no-op', () async { createFakePlugin( 'plugin', packagesDir, @@ -550,9 +562,39 @@ void main() { }, ); - final List output = await runCapturingPrint(runner, [ - 'drive-examples', - ]); + final List output = await runCapturingPrint( + runner, ['drive-examples', '--android']); + + expect( + output, + orderedEquals([ + '\n==========\nChecking plugin...', + 'Not supported for the target platform; skipping.', + '\n\n', + 'All driver tests successful!', + ]), + ); + + // Output should be empty since running drive-examples --macos with no macos + // implementation is a no-op. + expect(processRunner.recordedCalls, []); + }); + + test('driving when plugin does not support iOS is no-op', () async { + createFakePlugin( + 'plugin', + packagesDir, + extraFiles: [ + 'example/test_driver/plugin_test.dart', + 'example/test_driver/plugin.dart', + ], + platformSupport: { + kPlatformMacos: PlatformSupport.inline, + }, + ); + + final List output = + await runCapturingPrint(runner, ['drive-examples', '--ios']); expect( output, @@ -573,9 +615,8 @@ void main() { createFakePlugin('aplugin_platform_interface', packagesDir, examples: []); - final List output = await runCapturingPrint(runner, [ - 'drive-examples', - ]); + final List output = await runCapturingPrint( + runner, ['drive-examples', '--android']); expect( output, @@ -609,6 +650,7 @@ void main() { await runCapturingPrint(runner, [ 'drive-examples', + '--ios', '--enable-experiment=exp1', ]); From 240eeeaf9df6d5deaca66414384f9294ce2adf9a Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 24 Jun 2021 10:08:35 -0400 Subject: [PATCH 2/7] Use explicit device target for mobile --- .../tool/lib/src/drive_examples_command.dart | 147 ++++++++++------- .../test/drive_examples_command_test.dart | 156 ++++++++++++++++-- 2 files changed, 225 insertions(+), 78 deletions(-) diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index 572b5e72a036..49df905be9c5 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:convert'; +import 'dart:io'; + import 'package:file/file.dart'; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; @@ -11,7 +14,8 @@ import 'common/plugin_command.dart'; import 'common/plugin_utils.dart'; import 'common/process_runner.dart'; -const int _exitBadPlatformFlags = 2; +const int _exitNoPlatformFlags = 2; +const int _exitNoAvailableDevice = 3; /// A command to run the example applications for packages via Flutter driver. class DriveExamplesCommand extends PluginCommand { @@ -66,24 +70,55 @@ class DriveExamplesCommand extends PluginCommand { final int platformCount = platformSwitches .where((String platform) => getBoolArg(platform)) .length; - // The way this command is written relies (for legacy reasons) on using the - // default device (which it assumes has already been set up) for iOS or - // Android. Therefore if an explicit target device is passed, it won't - // actually drive the mobile version. If that is fixed, the restriction that - // only one platform can be selected can be removed. + // The flutter tool currently doesn't accept multiple device arguments: + // https://github.com/flutter/flutter/issues/35733 + // If that is implemented, this check can be relaxed. if (platformCount != 1) { - print( + printError( 'Exactly one of ${platformSwitches.map((String platform) => '--$platform').join(', ')} ' 'must be specified.'); - throw ToolExit(_exitBadPlatformFlags); + throw ToolExit(_exitNoPlatformFlags); + } + + String? androidDevice; + if (getBoolArg(kPlatformAndroid)) { + final List devices = await _getDevicesForPlatform('android'); + if (devices.isEmpty) { + printError('No Android devices available'); + throw ToolExit(_exitNoAvailableDevice); + } + androidDevice = devices.first; } + String? iosDevice; + if (getBoolArg(kPlatformIos)) { + final List devices = await _getDevicesForPlatform('ios'); + if (devices.isEmpty) { + printError('No iOS devices available'); + throw ToolExit(_exitNoAvailableDevice); + } + iosDevice = devices.first; + } + + final Map> targetDeviceFlags = >{ + if (getBoolArg(kPlatformAndroid)) + kPlatformAndroid: ['-d', androidDevice!], + if (getBoolArg(kPlatformIos)) kPlatformIos: ['-d', iosDevice!], + if (getBoolArg(kPlatformLinux)) kPlatformLinux: ['-d', 'linux'], + if (getBoolArg(kPlatformMacos)) kPlatformMacos: ['-d', 'macos'], + if (getBoolArg(kPlatformWeb)) + kPlatformWeb: [ + '-d', + 'web-server', + '--web-port=7357', + '--browser-name=chrome' + ], + if (getBoolArg(kPlatformWindows)) + kPlatformWindows: ['-d', 'windows'], + }; + final List failingTests = []; final List pluginsWithoutTests = []; - final bool isLinux = getBoolArg(kPlatformLinux); - final bool isMacos = getBoolArg(kPlatformMacos); - final bool isWeb = getBoolArg(kPlatformWeb); - final bool isWindows = getBoolArg(kPlatformWindows); await for (final Directory plugin in getPlugins()) { final String pluginName = plugin.basename; if (pluginName.endsWith('_platform_interface') && @@ -94,10 +129,22 @@ class DriveExamplesCommand extends PluginCommand { continue; } print('\n==========\nChecking $pluginName...'); - if (!(await _pluginSupportedOnTargetPlatform(plugin))) { - print('Not supported for the target platform; skipping.'); + + final List deviceFlags = []; + for (final MapEntry> entry + in targetDeviceFlags.entries) { + if (pluginSupportsPlatform(entry.key, plugin)) { + deviceFlags.addAll(entry.value); + } else { + // TODO(stuartmorgan): Consider making this an error, not a skip. + print('$pluginName does not support ${entry.key}.'); + } + } + // If there is no supported target platform, skip the plugin. + if (deviceFlags.isEmpty) { continue; } + int examplesFound = 0; bool testsRan = false; final String flutterCommand = @@ -164,40 +211,13 @@ Tried searching for the following: } } - final List driveArgs = ['drive']; + final List driveArgs = ['drive', ...deviceFlags]; final String enableExperiment = getStringArg(kEnableExperiment); if (enableExperiment.isNotEmpty) { driveArgs.add('--enable-experiment=$enableExperiment'); } - if (isLinux && isLinuxPlugin(plugin)) { - driveArgs.addAll([ - '-d', - 'linux', - ]); - } - if (isMacos && isMacOsPlugin(plugin)) { - driveArgs.addAll([ - '-d', - 'macos', - ]); - } - if (isWeb && isWebPlugin(plugin)) { - driveArgs.addAll([ - '-d', - 'web-server', - '--web-port=7357', - '--browser-name=chrome', - ]); - } - if (isWindows && isWindowsPlugin(plugin)) { - driveArgs.addAll([ - '-d', - 'windows', - ]); - } - for (final String targetPath in targetPaths) { testsRan = true; final int exitCode = await processRunner.runAndStream( @@ -245,26 +265,31 @@ Tried searching for the following: print('All driver tests successful!'); } - Future _pluginSupportedOnTargetPlatform(FileSystemEntity plugin) async { - if (getBoolArg(kPlatformAndroid)) { - return isAndroidPlugin(plugin); - } - if (getBoolArg(kPlatformIos)) { - return isIosPlugin(plugin); - } - if (getBoolArg(kPlatformLinux)) { - return isLinuxPlugin(plugin); - } - if (getBoolArg(kPlatformMacos)) { - return isMacOsPlugin(plugin); - } - if (getBoolArg(kPlatformWeb)) { - return isWebPlugin(plugin); + Future> _getDevicesForPlatform(String platform) async { + final List deviceIds = []; + final String flutterCommand = + const LocalPlatform().isWindows ? 'flutter.bat' : 'flutter'; + + final ProcessResult result = await processRunner.run( + flutterCommand, ['devices', '--machine'], + stdoutEncoding: utf8, exitOnError: true); + if (result.exitCode != 0) { + return deviceIds; } - if (getBoolArg(kPlatformWindows)) { - return isWindowsPlugin(plugin); + + final List> devices = + (jsonDecode(result.stdout as String) as List) + .cast>(); + for (final Map deviceInfo in devices) { + final String targetPlatform = + (deviceInfo['targetPlatform'] as String?) ?? ''; + if (targetPlatform.startsWith(platform)) { + final String? deviceId = deviceInfo['id'] as String?; + if (deviceId != null) { + deviceIds.add(deviceId); + } + } } - assert(false); - return false; + return deviceIds; } } diff --git a/script/tool/test/drive_examples_command_test.dart b/script/tool/test/drive_examples_command_test.dart index 695f956bb589..cd2a77427595 100644 --- a/script/tool/test/drive_examples_command_test.dart +++ b/script/tool/test/drive_examples_command_test.dart @@ -12,8 +12,12 @@ import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'package:test/test.dart'; +import 'mocks.dart'; import 'util.dart'; +const String _fakeIosDevice = '67d5c3d1-8bdf-46ad-8f6b-b00e2a972dda'; +const String _fakeAndroidDevice = 'emulator-1234'; + void main() { group('test drive_example_command', () { late FileSystem fileSystem; @@ -35,14 +39,50 @@ void main() { runner.addCommand(command); }); - test('Fails if no platforms are provided', () async { + void setMockFlutterDevicesOutput({ + bool hasIosDevice = true, + bool hasAndroidDevice = true, + }) { + final List devices = [ + if (hasIosDevice) '{"id": "$_fakeIosDevice", "targetPlatform": "ios"}', + if (hasAndroidDevice) + '{"id": "$_fakeAndroidDevice", "targetPlatform": "android-x86"}', + ]; + final String output = '''[${devices.join(',')}]'''; + + final MockProcess mockDevicesProcess = MockProcess(); + mockDevicesProcess.exitCodeCompleter.complete(0); + mockDevicesProcess.stdoutController.close(); // ignore: unawaited_futures + processRunner.processToReturn = mockDevicesProcess; + processRunner.resultStdout = output; + } + + test('fails if no platforms are provided', () async { + setMockFlutterDevicesOutput(); expect( () => runCapturingPrint(runner, ['drive-examples']), throwsA(isA()), ); }); - test('Fails if multiple platforms are provided', () async { + test('fails for iOS if no iOS devices are present', () async { + setMockFlutterDevicesOutput(hasIosDevice: false); + expect( + () => runCapturingPrint(runner, ['drive-examples']), + throwsA(isA()), + ); + }); + + test('fails if no platforms are provided', () async { + setMockFlutterDevicesOutput(hasAndroidDevice: false); + expect( + () => runCapturingPrint(runner, ['drive-examples']), + throwsA(isA()), + ); + }); + + test('fails if multiple platforms are provided', () async { + setMockFlutterDevicesOutput(hasAndroidDevice: false); expect( () => runCapturingPrint( runner, ['drive-examples', '--ios', '--macos']), @@ -67,6 +107,7 @@ void main() { final Directory pluginExampleDirectory = pluginDirectory.childDirectory('example'); + setMockFlutterDevicesOutput(); final List output = await runCapturingPrint(runner, ['drive-examples', '--ios']); @@ -84,10 +125,14 @@ void main() { expect( processRunner.recordedCalls, orderedEquals([ + ProcessCall( + flutterCommand, const ['devices', '--machine'], null), ProcessCall( flutterCommand, [ 'drive', + '-d', + _fakeIosDevice, '--driver', driverTestPath, '--target', @@ -114,6 +159,7 @@ void main() { final Directory pluginExampleDirectory = pluginDirectory.childDirectory('example'); + setMockFlutterDevicesOutput(); final List output = await runCapturingPrint(runner, ['drive-examples', '--ios']); @@ -131,10 +177,14 @@ void main() { expect( processRunner.recordedCalls, orderedEquals([ + ProcessCall( + flutterCommand, const ['devices', '--machine'], null), ProcessCall( flutterCommand, [ 'drive', + '-d', + _fakeIosDevice, '--driver', driverTestPath, '--target', @@ -203,6 +253,7 @@ void main() { final Directory pluginExampleDirectory = pluginDirectory.childDirectory('example'); + setMockFlutterDevicesOutput(); final List output = await runCapturingPrint(runner, ['drive-examples', '--ios']); @@ -220,10 +271,14 @@ void main() { expect( processRunner.recordedCalls, orderedEquals([ + ProcessCall( + flutterCommand, const ['devices', '--machine'], null), ProcessCall( flutterCommand, [ 'drive', + '-d', + _fakeIosDevice, '--driver', driverTestPath, '--target', @@ -234,6 +289,8 @@ void main() { flutterCommand, [ 'drive', + '-d', + _fakeIosDevice, '--driver', driverTestPath, '--target', @@ -258,7 +315,7 @@ void main() { output, orderedEquals([ '\n==========\nChecking plugin...', - 'Not supported for the target platform; skipping.', + 'plugin does not support linux.', '\n\n', 'All driver tests successful!', ]), @@ -334,7 +391,7 @@ void main() { output, orderedEquals([ '\n==========\nChecking plugin...', - 'Not supported for the target platform; skipping.', + 'plugin does not support macos.', '\n\n', 'All driver tests successful!', ]), @@ -344,6 +401,7 @@ void main() { // implementation is a no-op. expect(processRunner.recordedCalls, []); }); + test('driving on a macOS plugin', () async { final Directory pluginDirectory = createFakePlugin( 'plugin', @@ -410,7 +468,7 @@ void main() { output, orderedEquals([ '\n==========\nChecking plugin...', - 'Not supported for the target platform; skipping.', + 'plugin does not support web.', '\n\n', 'All driver tests successful!', ]), @@ -488,7 +546,7 @@ void main() { output, orderedEquals([ '\n==========\nChecking plugin...', - 'Not supported for the target platform; skipping.', + 'plugin does not support windows.', '\n\n', 'All driver tests successful!', ]), @@ -549,6 +607,59 @@ void main() { ])); }); + test('driving on an Android plugin', () async { + final Directory pluginDirectory = createFakePlugin( + 'plugin', + packagesDir, + extraFiles: [ + 'example/test_driver/plugin_test.dart', + 'example/test_driver/plugin.dart', + ], + platformSupport: { + kPlatformAndroid: PlatformSupport.inline, + }, + ); + + final Directory pluginExampleDirectory = + pluginDirectory.childDirectory('example'); + + setMockFlutterDevicesOutput(); + final List output = await runCapturingPrint(runner, [ + 'drive-examples', + '--android', + ]); + + expect( + output, + orderedEquals([ + '\n==========\nChecking plugin...', + '\n\n', + 'All driver tests successful!', + ]), + ); + + final String deviceTestPath = p.join('test_driver', 'plugin.dart'); + final String driverTestPath = p.join('test_driver', 'plugin_test.dart'); + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall( + flutterCommand, const ['devices', '--machine'], null), + ProcessCall( + flutterCommand, + [ + 'drive', + '-d', + _fakeAndroidDevice, + '--driver', + driverTestPath, + '--target', + deviceTestPath + ], + pluginExampleDirectory.path), + ])); + }); + test('driving when plugin does not support Android is no-op', () async { createFakePlugin( 'plugin', @@ -562,6 +673,7 @@ void main() { }, ); + setMockFlutterDevicesOutput(); final List output = await runCapturingPrint( runner, ['drive-examples', '--android']); @@ -569,15 +681,17 @@ void main() { output, orderedEquals([ '\n==========\nChecking plugin...', - 'Not supported for the target platform; skipping.', + 'plugin does not support android.', '\n\n', 'All driver tests successful!', ]), ); - // Output should be empty since running drive-examples --macos with no macos - // implementation is a no-op. - expect(processRunner.recordedCalls, []); + // Output should be empty other than the device query. + expect(processRunner.recordedCalls, [ + ProcessCall( + flutterCommand, const ['devices', '--machine'], null), + ]); }); test('driving when plugin does not support iOS is no-op', () async { @@ -593,6 +707,7 @@ void main() { }, ); + setMockFlutterDevicesOutput(); final List output = await runCapturingPrint(runner, ['drive-examples', '--ios']); @@ -600,23 +715,26 @@ void main() { output, orderedEquals([ '\n==========\nChecking plugin...', - 'Not supported for the target platform; skipping.', + 'plugin does not support ios.', '\n\n', 'All driver tests successful!', ]), ); - // Output should be empty since running drive-examples --macos with no macos - // implementation is a no-op. - expect(processRunner.recordedCalls, []); + // Output should be empty other than the device query. + expect(processRunner.recordedCalls, [ + ProcessCall( + flutterCommand, const ['devices', '--machine'], null), + ]); }); test('platform interface plugins are silently skipped', () async { createFakePlugin('aplugin_platform_interface', packagesDir, examples: []); + setMockFlutterDevicesOutput(); final List output = await runCapturingPrint( - runner, ['drive-examples', '--android']); + runner, ['drive-examples', '--macos']); expect( output, @@ -626,8 +744,7 @@ void main() { ]), ); - // Output should be empty since running drive-examples --macos with no macos - // implementation is a no-op. + // Output should be empty since it's skipped. expect(processRunner.recordedCalls, []); }); @@ -648,6 +765,7 @@ void main() { final Directory pluginExampleDirectory = pluginDirectory.childDirectory('example'); + setMockFlutterDevicesOutput(); await runCapturingPrint(runner, [ 'drive-examples', '--ios', @@ -659,10 +777,14 @@ void main() { expect( processRunner.recordedCalls, orderedEquals([ + ProcessCall( + flutterCommand, const ['devices', '--machine'], null), ProcessCall( flutterCommand, [ 'drive', + '-d', + _fakeIosDevice, '--enable-experiment=exp1', '--driver', driverTestPath, From b7e168a55014978b22feae61c4a22702dd8033a9 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 24 Jun 2021 10:15:36 -0400 Subject: [PATCH 3/7] Remove support for the test/ location, which is no longer used --- .../tool/lib/src/drive_examples_command.dart | 8 +-- .../test/drive_examples_command_test.dart | 56 +------------------ 2 files changed, 3 insertions(+), 61 deletions(-) diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index 49df905be9c5..a036934499b3 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -170,13 +170,7 @@ class DriveExamplesCommand extends PluginCommand { RegExp(r'_test.dart$'), '.dart', ); - String deviceTestPath = p.join('test', deviceTestName); - if (!example.fileSystem - .file(p.join(example.path, deviceTestPath)) - .existsSync()) { - // If the app isn't in test/ folder, look in test_driver/ instead. - deviceTestPath = p.join('test_driver', deviceTestName); - } + final String deviceTestPath = p.join('test_driver', deviceTestName); final List targetPaths = []; if (example.fileSystem diff --git a/script/tool/test/drive_examples_command_test.dart b/script/tool/test/drive_examples_command_test.dart index cd2a77427595..efa8d64c569a 100644 --- a/script/tool/test/drive_examples_command_test.dart +++ b/script/tool/test/drive_examples_command_test.dart @@ -90,58 +90,6 @@ void main() { ); }); - test('driving under folder "test"', () async { - final Directory pluginDirectory = createFakePlugin( - 'plugin', - packagesDir, - extraFiles: [ - 'example/test_driver/plugin_test.dart', - 'example/test/plugin.dart', - ], - platformSupport: { - kPlatformAndroid: PlatformSupport.inline, - kPlatformIos: PlatformSupport.inline, - }, - ); - - final Directory pluginExampleDirectory = - pluginDirectory.childDirectory('example'); - - setMockFlutterDevicesOutput(); - final List output = - await runCapturingPrint(runner, ['drive-examples', '--ios']); - - expect( - output, - orderedEquals([ - '\n==========\nChecking plugin...', - '\n\n', - 'All driver tests successful!', - ]), - ); - - final String deviceTestPath = p.join('test', 'plugin.dart'); - final String driverTestPath = p.join('test_driver', 'plugin_test.dart'); - expect( - processRunner.recordedCalls, - orderedEquals([ - ProcessCall( - flutterCommand, const ['devices', '--machine'], null), - ProcessCall( - flutterCommand, - [ - 'drive', - '-d', - _fakeIosDevice, - '--driver', - driverTestPath, - '--target', - deviceTestPath - ], - pluginExampleDirectory.path), - ])); - }); - test('driving under folder "test_driver"', () async { final Directory pluginDirectory = createFakePlugin( 'plugin', @@ -754,7 +702,7 @@ void main() { packagesDir, extraFiles: [ 'example/test_driver/plugin_test.dart', - 'example/test/plugin.dart', + 'example/test_driver/plugin.dart', ], platformSupport: { kPlatformAndroid: PlatformSupport.inline, @@ -772,7 +720,7 @@ void main() { '--enable-experiment=exp1', ]); - final String deviceTestPath = p.join('test', 'plugin.dart'); + final String deviceTestPath = p.join('test_driver', 'plugin.dart'); final String driverTestPath = p.join('test_driver', 'plugin_test.dart'); expect( processRunner.recordedCalls, From 03f4ed353472dd0370656c0097502acbb29dc4be Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 24 Jun 2021 11:28:22 -0400 Subject: [PATCH 4/7] Extract some helpers --- .../tool/lib/src/build_examples_command.dart | 3 - .../tool/lib/src/common/plugin_command.dart | 5 + .../tool/lib/src/drive_examples_command.dart | 169 ++++++++++-------- 3 files changed, 103 insertions(+), 74 deletions(-) diff --git a/script/tool/lib/src/build_examples_command.dart b/script/tool/lib/src/build_examples_command.dart index aff5ecba4989..2a2b4e889396 100644 --- a/script/tool/lib/src/build_examples_command.dart +++ b/script/tool/lib/src/build_examples_command.dart @@ -7,7 +7,6 @@ import 'dart:io' as io; import 'package:file/file.dart'; import 'package:path/path.dart' as p; -import 'package:platform/platform.dart'; import 'common/core.dart'; import 'common/plugin_command.dart'; @@ -64,8 +63,6 @@ class BuildExamplesCommand extends PluginCommand { 'were specified, so not building anything.'); return; } - final String flutterCommand = - const LocalPlatform().isWindows ? 'flutter.bat' : 'flutter'; final String enableExperiment = getStringArg(kEnableExperiment); diff --git a/script/tool/lib/src/common/plugin_command.dart b/script/tool/lib/src/common/plugin_command.dart index 4c095858e45a..1479cb6571ef 100644 --- a/script/tool/lib/src/common/plugin_command.dart +++ b/script/tool/lib/src/common/plugin_command.dart @@ -8,6 +8,7 @@ import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:git/git.dart'; import 'package:path/path.dart' as p; +import 'package:platform/platform.dart'; import 'core.dart'; import 'git_version_finder.dart'; @@ -84,6 +85,10 @@ abstract class PluginCommand extends Command { int? _shardIndex; int? _shardCount; + /// The command to use when running `flutter`. + String get flutterCommand => + const LocalPlatform().isWindows ? 'flutter.bat' : 'flutter'; + /// The shard of the overall command execution that this instance should run. int get shardIndex { if (_shardIndex == null) { diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index a036934499b3..ee455a0168de 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -7,7 +7,6 @@ import 'dart:io'; import 'package:file/file.dart'; import 'package:path/path.dart' as p; -import 'package:platform/platform.dart'; import 'common/core.dart'; import 'common/plugin_command.dart'; @@ -147,87 +146,46 @@ class DriveExamplesCommand extends PluginCommand { int examplesFound = 0; bool testsRan = false; - final String flutterCommand = - const LocalPlatform().isWindows ? 'flutter.bat' : 'flutter'; for (final Directory example in getExamplesForPlugin(plugin)) { ++examplesFound; final String packageName = p.relative(example.path, from: packagesDir.path); - final Directory driverTests = example.childDirectory('test_driver'); - if (!driverTests.existsSync()) { + + final List drivers = await _getDrivers(example); + if (drivers.isEmpty) { print('No driver tests found for $packageName'); continue; } - // Look for driver tests ending in _test.dart in test_driver/ - await for (final FileSystemEntity test in driverTests.list()) { - final String driverTestName = - p.relative(test.path, from: driverTests.path); - if (!driverTestName.endsWith('_test.dart')) { - continue; - } + + for (final File driver in drivers) { + final List testTargets = []; + // Try to find a matching app to drive without the _test.dart - final String deviceTestName = driverTestName.replaceAll( - RegExp(r'_test.dart$'), - '.dart', - ); - final String deviceTestPath = p.join('test_driver', deviceTestName); - - final List targetPaths = []; - if (example.fileSystem - .file(p.join(example.path, deviceTestPath)) - .existsSync()) { - targetPaths.add(deviceTestPath); + // TODO(stuartmorgan): Migrate all remaining uses of this legacy + // approach (currently only video_player) and remove support for it: + // https://github.com/flutter/flutter/issues/85224. + final File? legacyTestFile = _getLegacyTestFileForTestDriver(driver); + if (legacyTestFile != null) { + testTargets.add(legacyTestFile); } else { - final Directory integrationTests = - example.childDirectory('integration_test'); - - if (await integrationTests.exists()) { - await for (final FileSystemEntity integrationTest - in integrationTests.list()) { - if (!integrationTest.basename.endsWith('_test.dart')) { - continue; - } - targetPaths - .add(p.relative(integrationTest.path, from: example.path)); - } - } - - if (targetPaths.isEmpty) { - print(''' -Unable to infer a target application for $driverTestName to drive. -Tried searching for the following: -1. test/$deviceTestName -2. test_driver/$deviceTestName -3. test_driver/*_test.dart -'''); - failingTests.add(p.relative(test.path, from: example.path)); - continue; - } + (await _getIntegrationTests(example)).forEach(testTargets.add); } - final List driveArgs = ['drive', ...deviceFlags]; - - final String enableExperiment = getStringArg(kEnableExperiment); - if (enableExperiment.isNotEmpty) { - driveArgs.add('--enable-experiment=$enableExperiment'); + if (testTargets.isEmpty) { + final String driverRelativePath = + p.relative(driver.path, from: plugin.path); + print( + 'Found $driverRelativePath, but no integration_test/*_test.dart files.'); + failingTests.add(p.relative(driver.path, from: example.path)); + continue; } - for (final String targetPath in targetPaths) { - testsRan = true; - final int exitCode = await processRunner.runAndStream( - flutterCommand, - [ - ...driveArgs, - '--driver', - p.join('test_driver', driverTestName), - '--target', - targetPath, - ], - workingDir: example, - exitOnError: true); - if (exitCode != 0) { - failingTests.add(p.join(packageName, deviceTestPath)); - } + testsRan = true; + final List failingTargets = await _driveTests( + example, driver, testTargets, + deviceFlags: deviceFlags); + for (final File failingTarget in failingTargets) { + failingTests.add(p.relative(failingTarget.path, from: plugin.path)); } } } @@ -261,8 +219,6 @@ Tried searching for the following: Future> _getDevicesForPlatform(String platform) async { final List deviceIds = []; - final String flutterCommand = - const LocalPlatform().isWindows ? 'flutter.bat' : 'flutter'; final ProcessResult result = await processRunner.run( flutterCommand, ['devices', '--machine'], @@ -286,4 +242,75 @@ Tried searching for the following: } return deviceIds; } + + Future> _getDrivers(Directory example) async { + final List drivers = []; + + final Directory driverDir = example.childDirectory('test_driver'); + await for (final FileSystemEntity driver in driverDir.list()) { + if (driver is File && driver.basename.endsWith('_test.dart')) { + drivers.add(driver); + } + } + return drivers; + } + + File? _getLegacyTestFileForTestDriver(File testDriver) { + final String testName = testDriver.basename.replaceAll( + RegExp(r'_test.dart$'), + '.dart', + ); + final File testFile = testDriver.parent.childFile(testName); + + return testFile.existsSync() ? testFile : null; + } + + Future> _getIntegrationTests(Directory example) async { + final List tests = []; + final Directory integrationTestDir = + example.childDirectory('integration_test'); + + if (integrationTestDir.existsSync()) { + await for (final FileSystemEntity file in integrationTestDir.list()) { + if (file is File && file.basename.endsWith('_test.dart')) { + tests.add(file); + } + } + } + return tests; + } + + /// Runs each file from [targets] using `flutter drive`, returning a list of + /// any failing test targets. + Future> _driveTests( + Directory example, + File driver, + List targets, { + required List deviceFlags, + }) async { + final List failures = []; + + final String enableExperiment = getStringArg(kEnableExperiment); + + for (final File target in targets) { + final int exitCode = await processRunner.runAndStream( + flutterCommand, + [ + 'drive', + ...deviceFlags, + if (enableExperiment.isNotEmpty) + '--enable-experiment=$enableExperiment', + '--driver', + p.relative(driver.path, from: example.path), + '--target', + p.relative(target.path, from: example.path), + ], + workingDir: example, + exitOnError: true); + if (exitCode != 0) { + failures.add(target); + } + } + return failures; + } } From 9d63a2c1921fb56dbe18d7ce41204e71ce7c6a06 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 24 Jun 2021 12:39:19 -0400 Subject: [PATCH 5/7] Migrate to new base command --- .../tool/lib/src/drive_examples_command.dart | 175 ++++++++---------- .../test/drive_examples_command_test.dart | 110 +++++------ 2 files changed, 127 insertions(+), 158 deletions(-) diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index ee455a0168de..dbbfe20379a0 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -9,7 +9,7 @@ import 'package:file/file.dart'; import 'package:path/path.dart' as p; import 'common/core.dart'; -import 'common/plugin_command.dart'; +import 'common/package_looping_command.dart'; import 'common/plugin_utils.dart'; import 'common/process_runner.dart'; @@ -17,7 +17,7 @@ const int _exitNoPlatformFlags = 2; const int _exitNoAvailableDevice = 3; /// A command to run the example applications for packages via Flutter driver. -class DriveExamplesCommand extends PluginCommand { +class DriveExamplesCommand extends PackageLoopingCommand { /// Creates an instance of the drive command. DriveExamplesCommand( Directory packagesDir, { @@ -48,16 +48,16 @@ class DriveExamplesCommand extends PluginCommand { @override final String description = 'Runs driver tests for plugin example apps.\n\n' - 'For each *_test.dart in test_driver/ it drives an application with a ' - 'corresponding name in the test/ or test_driver/ directories.\n\n' - 'For example, test_driver/app_test.dart would match test/app.dart.\n\n' - 'This command requires "flutter" to be in your path.\n\n' - 'If a file with a corresponding name cannot be found, this driver file' - 'will be used to drive the tests that match ' - 'integration_test/*_test.dart.'; + 'For each *_test.dart in test_driver/ it drives an application with the ' + 'either the corresponding test in test_driver (for example, ' + 'test_driver/app_test.dart would match test_driver/app.dart), or the ' + '*_test.dart files in integration_test/.\n\n' + 'This command requires "flutter" to be in your path.'; + + Map> _targetDeviceFlags = const >{}; @override - Future run() async { + Future initializeRun() async { final List platformSwitches = [ kPlatformAndroid, kPlatformIos, @@ -99,7 +99,7 @@ class DriveExamplesCommand extends PluginCommand { iosDevice = devices.first; } - final Map> targetDeviceFlags = >{ + _targetDeviceFlags = >{ if (getBoolArg(kPlatformAndroid)) kPlatformAndroid: ['-d', androidDevice!], if (getBoolArg(kPlatformIos)) kPlatformIos: ['-d', iosDevice!], @@ -115,106 +115,87 @@ class DriveExamplesCommand extends PluginCommand { if (getBoolArg(kPlatformWindows)) kPlatformWindows: ['-d', 'windows'], }; + } - final List failingTests = []; - final List pluginsWithoutTests = []; - await for (final Directory plugin in getPlugins()) { - final String pluginName = plugin.basename; - if (pluginName.endsWith('_platform_interface') && - !plugin.childDirectory('example').existsSync()) { - // Platform interface packages generally aren't intended to have - // examples, and don't need integration tests, so silently skip them - // unless for some reason there is an example directory. - continue; - } - print('\n==========\nChecking $pluginName...'); + @override + Future> runForPackage(Directory package) async { + if (package.basename.endsWith('_platform_interface') && + !package.childDirectory('example').existsSync()) { + // Platform interface packages generally aren't intended to have + // examples, and don't need integration tests, so skip rather than fail. + printSkip( + 'Platform interfaces are not expected to have integratino tests.'); + return PackageLoopingCommand.success; + } - final List deviceFlags = []; - for (final MapEntry> entry - in targetDeviceFlags.entries) { - if (pluginSupportsPlatform(entry.key, plugin)) { - deviceFlags.addAll(entry.value); - } else { - // TODO(stuartmorgan): Consider making this an error, not a skip. - print('$pluginName does not support ${entry.key}.'); - } + final List deviceFlags = []; + for (final MapEntry> entry + in _targetDeviceFlags.entries) { + if (pluginSupportsPlatform(entry.key, package)) { + deviceFlags.addAll(entry.value); + } else { + print('Skipping unsupported platform ${entry.key}...'); } - // If there is no supported target platform, skip the plugin. - if (deviceFlags.isEmpty) { + } + // If there is no supported target platform, skip the plugin. + if (deviceFlags.isEmpty) { + printSkip( + '${getPackageDescription(package)} does not support any requested platform.'); + return PackageLoopingCommand.success; + } + + int examplesFound = 0; + bool testsRan = false; + final List errors = []; + for (final Directory example in getExamplesForPlugin(package)) { + ++examplesFound; + final String exampleName = + p.relative(example.path, from: packagesDir.path); + + final List drivers = await _getDrivers(example); + if (drivers.isEmpty) { + print('No driver tests found for $exampleName'); continue; } - int examplesFound = 0; - bool testsRan = false; - for (final Directory example in getExamplesForPlugin(plugin)) { - ++examplesFound; - final String packageName = - p.relative(example.path, from: packagesDir.path); + for (final File driver in drivers) { + final List testTargets = []; - final List drivers = await _getDrivers(example); - if (drivers.isEmpty) { - print('No driver tests found for $packageName'); - continue; + // Try to find a matching app to drive without the _test.dart + // TODO(stuartmorgan): Migrate all remaining uses of this legacy + // approach (currently only video_player) and remove support for it: + // https://github.com/flutter/flutter/issues/85224. + final File? legacyTestFile = _getLegacyTestFileForTestDriver(driver); + if (legacyTestFile != null) { + testTargets.add(legacyTestFile); + } else { + (await _getIntegrationTests(example)).forEach(testTargets.add); } - for (final File driver in drivers) { - final List testTargets = []; - - // Try to find a matching app to drive without the _test.dart - // TODO(stuartmorgan): Migrate all remaining uses of this legacy - // approach (currently only video_player) and remove support for it: - // https://github.com/flutter/flutter/issues/85224. - final File? legacyTestFile = _getLegacyTestFileForTestDriver(driver); - if (legacyTestFile != null) { - testTargets.add(legacyTestFile); - } else { - (await _getIntegrationTests(example)).forEach(testTargets.add); - } - - if (testTargets.isEmpty) { - final String driverRelativePath = - p.relative(driver.path, from: plugin.path); - print( - 'Found $driverRelativePath, but no integration_test/*_test.dart files.'); - failingTests.add(p.relative(driver.path, from: example.path)); - continue; - } - - testsRan = true; - final List failingTargets = await _driveTests( - example, driver, testTargets, - deviceFlags: deviceFlags); - for (final File failingTarget in failingTargets) { - failingTests.add(p.relative(failingTarget.path, from: plugin.path)); - } + if (testTargets.isEmpty) { + final String driverRelativePath = + p.relative(driver.path, from: package.path); + printError( + 'Found $driverRelativePath, but no integration_test/*_test.dart files.'); + errors.add( + 'No test files for ${p.relative(driver.path, from: package.path)}'); + continue; } - } - if (!testsRan) { - pluginsWithoutTests.add(pluginName); - print( - 'No driver tests run for $pluginName ($examplesFound examples found)'); - } - } - print('\n\n'); - if (failingTests.isNotEmpty) { - print('The following driver tests are failing (see above for details):'); - for (final String test in failingTests) { - print(' * $test'); + testsRan = true; + final List failingTargets = await _driveTests( + example, driver, testTargets, + deviceFlags: deviceFlags); + for (final File failingTarget in failingTargets) { + errors.add(p.relative(failingTarget.path, from: package.path)); + } } - throw ToolExit(1); } - - if (pluginsWithoutTests.isNotEmpty) { - print('The following plugins did not run any integration tests:'); - for (final String plugin in pluginsWithoutTests) { - print(' * $plugin'); - } - print('If this is intentional, they must be explicitly excluded.'); - throw ToolExit(1); + if (!testsRan) { + printError('No driver tests were run ($examplesFound examples found).'); + errors.add('No tests ran (use --exclude if this is intentional).'); } - - print('All driver tests successful!'); + return errors; } Future> _getDevicesForPlatform(String platform) async { diff --git a/script/tool/test/drive_examples_command_test.dart b/script/tool/test/drive_examples_command_test.dart index efa8d64c569a..a70abe9db071 100644 --- a/script/tool/test/drive_examples_command_test.dart +++ b/script/tool/test/drive_examples_command_test.dart @@ -113,10 +113,9 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('No issues found!'), ]), ); @@ -207,10 +206,9 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('No issues found!'), ]), ); @@ -261,11 +259,10 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - 'plugin does not support linux.', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('Skipping unsupported platform linux...'), + contains('No issues found!'), ]), ); @@ -297,10 +294,9 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('No issues found!'), ]), ); @@ -337,11 +333,10 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - 'plugin does not support macos.', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('Skipping unsupported platform macos...'), + contains('No issues found!'), ]), ); @@ -374,10 +369,9 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('No issues found!'), ]), ); @@ -414,11 +408,9 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - 'plugin does not support web.', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('No issues found!'), ]), ); @@ -450,10 +442,9 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('No issues found!'), ]), ); @@ -492,11 +483,10 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - 'plugin does not support windows.', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('Skipping unsupported platform windows...'), + contains('No issues found!'), ]), ); @@ -528,10 +518,9 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('No issues found!'), ]), ); @@ -579,10 +568,9 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('No issues found!'), ]), ); @@ -627,11 +615,10 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - 'plugin does not support android.', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('Skipping unsupported platform android...'), + contains('No issues found!'), ]), ); @@ -661,11 +648,10 @@ void main() { expect( output, - orderedEquals([ - '\n==========\nChecking plugin...', - 'plugin does not support ios.', - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for plugin'), + contains('Skipping unsupported platform ios...'), + contains('No issues found!'), ]), ); @@ -686,9 +672,11 @@ void main() { expect( output, - orderedEquals([ - '\n\n', - 'All driver tests successful!', + containsAllInOrder([ + contains('Running for aplugin_platform_interface'), + contains( + 'SKIPPING: Platform interfaces are not expected to have integratino tests.'), + contains('No issues found!'), ]), ); From 9d0d125f7ebb6cc305a867f8d969be04c85617ed Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 24 Jun 2021 13:22:39 -0400 Subject: [PATCH 6/7] Add some failure tests --- .../tool/lib/src/drive_examples_command.dart | 10 +- .../test/drive_examples_command_test.dart | 168 ++++++++++++++++++ 2 files changed, 174 insertions(+), 4 deletions(-) diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index dbbfe20379a0..51a305d4484b 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -192,7 +192,7 @@ class DriveExamplesCommand extends PackageLoopingCommand { } } if (!testsRan) { - printError('No driver tests were run ($examplesFound examples found).'); + printError('No driver tests were run ($examplesFound example(s) found).'); errors.add('No tests ran (use --exclude if this is intentional).'); } return errors; @@ -228,9 +228,11 @@ class DriveExamplesCommand extends PackageLoopingCommand { final List drivers = []; final Directory driverDir = example.childDirectory('test_driver'); - await for (final FileSystemEntity driver in driverDir.list()) { - if (driver is File && driver.basename.endsWith('_test.dart')) { - drivers.add(driver); + if (driverDir.existsSync()) { + await for (final FileSystemEntity driver in driverDir.list()) { + if (driver is File && driver.basename.endsWith('_test.dart')) { + drivers.add(driver); + } } } return drivers; diff --git a/script/tool/test/drive_examples_command_test.dart b/script/tool/test/drive_examples_command_test.dart index a70abe9db071..c855f6d2efa3 100644 --- a/script/tool/test/drive_examples_command_test.dart +++ b/script/tool/test/drive_examples_command_test.dart @@ -730,5 +730,173 @@ void main() { pluginExampleDirectory.path), ])); }); + + test('fails when no example is present', () async { + createFakePlugin( + 'plugin', + packagesDir, + examples: [], + platformSupport: { + kPlatformWeb: PlatformSupport.inline, + }, + ); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['drive-examples', '--web'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Running for plugin'), + contains('No driver tests were run (0 example(s) found).'), + contains('The following packages had errors:'), + contains(' plugin:\n' + ' No tests ran (use --exclude if this is intentional)'), + ]), + ); + }); + + test('fails when no driver is present', () async { + createFakePlugin( + 'plugin', + packagesDir, + extraFiles: [ + 'example/integration_test/bar_test.dart', + 'example/integration_test/foo_test.dart', + ], + platformSupport: { + kPlatformWeb: PlatformSupport.inline, + }, + ); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['drive-examples', '--web'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Running for plugin'), + contains('No driver tests found for plugin/example'), + contains('No driver tests were run (1 example(s) found).'), + contains('The following packages had errors:'), + contains(' plugin:\n' + ' No tests ran (use --exclude if this is intentional)'), + ]), + ); + }); + + test('fails when no integration tests are present', () async { + createFakePlugin( + 'plugin', + packagesDir, + extraFiles: [ + 'example/test_driver/integration_test.dart', + ], + platformSupport: { + kPlatformWeb: PlatformSupport.inline, + }, + ); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['drive-examples', '--web'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Running for plugin'), + contains('Found example/test_driver/integration_test.dart, but no ' + 'integration_test/*_test.dart files.'), + contains('No driver tests were run (1 example(s) found).'), + contains('The following packages had errors:'), + contains(' plugin:\n' + ' No test files for example/test_driver/integration_test.dart\n' + ' No tests ran (use --exclude if this is intentional)'), + ]), + ); + }); + + test('reports test failures', () async { + final Directory pluginDirectory = createFakePlugin( + 'plugin', + packagesDir, + extraFiles: [ + 'example/test_driver/integration_test.dart', + 'example/integration_test/bar_test.dart', + 'example/integration_test/foo_test.dart', + ], + platformSupport: { + kPlatformMacos: PlatformSupport.inline, + }, + ); + + // Simulate failure from `flutter drive`. + final MockProcess mockDriveProcess = MockProcess(); + mockDriveProcess.exitCodeCompleter.complete(1); + processRunner.processToReturn = mockDriveProcess; + + Error? commandError; + final List output = + await runCapturingPrint(runner, ['drive-examples', '--macos'], + errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Running for plugin'), + contains('The following packages had errors:'), + contains(' plugin:\n' + ' example/integration_test/bar_test.dart\n' + ' example/integration_test/foo_test.dart'), + ]), + ); + + final Directory pluginExampleDirectory = + pluginDirectory.childDirectory('example'); + final String driverTestPath = + p.join('test_driver', 'integration_test.dart'); + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall( + flutterCommand, + [ + 'drive', + '-d', + 'macos', + '--driver', + driverTestPath, + '--target', + p.join('integration_test', 'bar_test.dart'), + ], + pluginExampleDirectory.path), + ProcessCall( + flutterCommand, + [ + 'drive', + '-d', + 'macos', + '--driver', + driverTestPath, + '--target', + p.join('integration_test', 'foo_test.dart'), + ], + pluginExampleDirectory.path), + ])); + }); }); } From e3475b19de245b8f6adfe38eafe52b8ef50ba711 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 30 Jun 2021 13:56:23 -0400 Subject: [PATCH 7/7] Address review comments, fix tests --- .../tool/lib/src/drive_examples_command.dart | 13 ++- .../test/drive_examples_command_test.dart | 107 ++++++++++++++---- 2 files changed, 96 insertions(+), 24 deletions(-) diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index 51a305d4484b..a4aa7c12913d 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -48,7 +48,7 @@ class DriveExamplesCommand extends PackageLoopingCommand { @override final String description = 'Runs driver tests for plugin example apps.\n\n' - 'For each *_test.dart in test_driver/ it drives an application with the ' + 'For each *_test.dart in test_driver/ it drives an application with ' 'either the corresponding test in test_driver (for example, ' 'test_driver/app_test.dart would match test_driver/app.dart), or the ' '*_test.dart files in integration_test/.\n\n' @@ -263,8 +263,15 @@ class DriveExamplesCommand extends PackageLoopingCommand { return tests; } - /// Runs each file from [targets] using `flutter drive`, returning a list of - /// any failing test targets. + /// For each file in [targets], uses + /// `flutter drive --driver [driver] --target ` + /// to drive [example], returning a list of any failing test targets. + /// + /// [deviceFlags] should contain the flags to run the test on a specific + /// target device (plus any supporting device-specific flags). E.g.: + /// - `['-d', 'macos']` for driving for macOS. + /// - `['-d', 'web-server', '--web-port=', '--browser-name=]` + /// for web Future> _driveTests( Directory example, File driver, diff --git a/script/tool/test/drive_examples_command_test.dart b/script/tool/test/drive_examples_command_test.dart index c855f6d2efa3..e441a0f68d9e 100644 --- a/script/tool/test/drive_examples_command_test.dart +++ b/script/tool/test/drive_examples_command_test.dart @@ -59,34 +59,71 @@ void main() { test('fails if no platforms are provided', () async { setMockFlutterDevicesOutput(); + Error? commandError; + final List output = await runCapturingPrint( + runner, ['drive-examples'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); expect( - () => runCapturingPrint(runner, ['drive-examples']), - throwsA(isA()), + output, + containsAllInOrder([ + contains('Exactly one of'), + ]), ); }); - test('fails for iOS if no iOS devices are present', () async { - setMockFlutterDevicesOutput(hasIosDevice: false); + test('fails if multiple platforms are provided', () async { + setMockFlutterDevicesOutput(); + Error? commandError; + final List output = await runCapturingPrint( + runner, ['drive-examples', '--ios', '--macos'], + errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); expect( - () => runCapturingPrint(runner, ['drive-examples']), - throwsA(isA()), + output, + containsAllInOrder([ + contains('Exactly one of'), + ]), ); }); - test('fails if no platforms are provided', () async { - setMockFlutterDevicesOutput(hasAndroidDevice: false); + test('fails for iOS if no iOS devices are present', () async { + setMockFlutterDevicesOutput(hasIosDevice: false); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['drive-examples', '--ios'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); expect( - () => runCapturingPrint(runner, ['drive-examples']), - throwsA(isA()), + output, + containsAllInOrder([ + contains('No iOS devices'), + ]), ); }); - test('fails if multiple platforms are provided', () async { - setMockFlutterDevicesOutput(hasAndroidDevice: false); + test('fails if Android if no Android devices are present', () async { + Error? commandError; + final List output = await runCapturingPrint( + runner, ['drive-examples', '--android'], + errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); expect( - () => runCapturingPrint( - runner, ['drive-examples', '--ios', '--macos']), - throwsA(isA()), + output, + containsAllInOrder([ + contains('No Android devices'), + ]), ); }); @@ -143,6 +180,7 @@ void main() { test('driving under folder "test_driver" when test files are missing"', () async { + setMockFlutterDevicesOutput(); createFakePlugin( 'plugin', packagesDir, @@ -155,13 +193,27 @@ void main() { }, ); - await expectLater( - () => runCapturingPrint(runner, ['drive-examples']), - throwsA(const TypeMatcher())); + Error? commandError; + final List output = await runCapturingPrint( + runner, ['drive-examples', '--android'], + errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Running for plugin'), + contains('No driver tests were run (1 example(s) found).'), + contains('No test files for example/test_driver/plugin_test.dart'), + ]), + ); }); test('a plugin without any integration test files is reported as an error', () async { + setMockFlutterDevicesOutput(); createFakePlugin( 'plugin', packagesDir, @@ -174,9 +226,22 @@ void main() { }, ); - await expectLater( - () => runCapturingPrint(runner, ['drive-examples']), - throwsA(const TypeMatcher())); + Error? commandError; + final List output = await runCapturingPrint( + runner, ['drive-examples', '--android'], + errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Running for plugin'), + contains('No driver tests were run (1 example(s) found).'), + contains('No tests ran'), + ]), + ); }); test(