From 356d316717baedacc3fe836c044673b29653d783 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Tue, 22 Jun 2021 13:32:03 -0700 Subject: [PATCH] [flutter_plugin_tools] Add a new base command for looping over packages (#4067) Most of our commands are generally of the form: ``` for (each plugin as defined by the tool flags) check some things for success or failure print a summary all of the failing things exit non-zero if anything failed ``` Currently all that logic not consistent, having been at various points copied and pasted around, modified, in some cases rewritten. There's unnecessary boilerplate in each new command, and there's unnecessary variation that makes it harder both to maintain the tool, and to consume the test output: - There's no standard format for separating each plugin's run to search within a log - There's no standard format for the summary at the end - In some cases commands have been written to ToolExit on failure, which means we don't actually get the rest of the runs This makes a new base class for commands that follow this structure to use, with shared code for all the common bits. This makes it harder to accidentally write new commands incorrectly, easier to maintain the code, and lets us standardize output so that searching within large logs will be easier. This ports two commands over as a proof of concept to demonstrate that it works; more will be converted in follow-ups. Related to https://github.com/flutter/flutter/issues/83413 --- script/tool/CHANGELOG.md | 1 + script/tool/lib/src/common/core.dart | 22 +- .../src/common/package_looping_command.dart | 161 +++++++ .../tool/lib/src/common/plugin_command.dart | 3 + .../tool/lib/src/pubspec_check_command.dart | 39 +- script/tool/lib/src/xctest_command.dart | 89 ++-- .../common/package_looping_command_test.dart | 433 ++++++++++++++++++ .../tool/test/pubspec_check_command_test.dart | 22 +- script/tool/test/xctest_command_test.dart | 36 +- 9 files changed, 710 insertions(+), 96 deletions(-) create mode 100644 script/tool/lib/src/common/package_looping_command.dart create mode 100644 script/tool/test/common/package_looping_command_test.dart diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index ae81ced636..f43d2fcc9b 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -6,6 +6,7 @@ - `xctest` now supports running macOS tests in addition to iOS - **Breaking change**: it now requires an `--ios` and/or `--macos` flag. - The tooling now runs in strong null-safe mode. +- Modified the output format of `pubspec-check` and `xctest` ## 0.2.0 diff --git a/script/tool/lib/src/common/core.dart b/script/tool/lib/src/common/core.dart index 1da6ef7a4c..b2be8f56d1 100644 --- a/script/tool/lib/src/common/core.dart +++ b/script/tool/lib/src/common/core.dart @@ -53,13 +53,25 @@ bool isFlutterPackage(FileSystemEntity entity) { } } +/// Prints `successMessage` in green. +void printSuccess(String successMessage) { + print(Colorize(successMessage)..green()); +} + /// Prints `errorMessage` in red. void printError(String errorMessage) { - final Colorize redError = Colorize(errorMessage)..red(); - print(redError); + print(Colorize(errorMessage)..red()); } /// Error thrown when a command needs to exit with a non-zero exit code. +/// +/// While there is no specific definition of the meaning of different non-zero +/// exit codes for this tool, commands should follow the general convention: +/// 1: The command ran correctly, but found errors. +/// 2: The command failed to run because the arguments were invalid. +/// >2: The command failed to run correctly for some other reason. Ideally, +/// each such failure should have a unique exit code within the context of +/// that command. class ToolExit extends Error { /// Creates a tool exit with the given [exitCode]. ToolExit(this.exitCode); @@ -67,3 +79,9 @@ class ToolExit extends Error { /// The code that the process should exit with. final int exitCode; } + +/// A exit code for [ToolExit] for a successful run that found errors. +const int exitCommandFoundErrors = 1; + +/// A exit code for [ToolExit] for a failure to run due to invalid arguments. +const int exitInvalidArguments = 2; diff --git a/script/tool/lib/src/common/package_looping_command.dart b/script/tool/lib/src/common/package_looping_command.dart new file mode 100644 index 0000000000..1349a5ed5d --- /dev/null +++ b/script/tool/lib/src/common/package_looping_command.dart @@ -0,0 +1,161 @@ +// Copyright 2013 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:colorize/colorize.dart'; +import 'package:file/file.dart'; +import 'package:git/git.dart'; +import 'package:path/path.dart' as p; + +import 'core.dart'; +import 'plugin_command.dart'; +import 'process_runner.dart'; + +/// An abstract base class for a command that iterates over a set of packages +/// controlled by a standard set of flags, running some actions on each package, +/// and collecting and reporting the success/failure of those actions. +abstract class PackageLoopingCommand extends PluginCommand { + /// Creates a command to operate on [packagesDir] with the given environment. + PackageLoopingCommand( + Directory packagesDir, { + ProcessRunner processRunner = const ProcessRunner(), + GitDir? gitDir, + }) : super(packagesDir, processRunner: processRunner, gitDir: gitDir); + + /// Called during [run] before any calls to [runForPackage]. This provides an + /// opportunity to fail early if the command can't be run (e.g., because the + /// arguments are invalid), and to set up any run-level state. + Future initializeRun() async {} + + /// Runs the command for [package], returning a list of errors. + /// + /// Errors may either be an empty string if there is no context that should + /// be included in the final error summary (e.g., a command that only has a + /// single failure mode), or strings that should be listed for that package + /// in the final summary. An empty list indicates success. + Future> runForPackage(Directory package); + + /// Whether or not the output (if any) of [runForPackage] is long, or short. + /// + /// This changes the logging that happens at the start of each package's + /// run; long output gets a banner-style message to make it easier to find, + /// while short output gets a single-line entry. + /// + /// When this is false, runForPackage output should be indented if possible, + /// to make the output structure easier to follow. + bool get hasLongOutput => true; + + /// Whether to loop over all packages (e.g., including example/), rather than + /// only top-level packages. + bool get includeSubpackages => false; + + /// The text to output at the start when reporting one or more failures. + /// This will be followed by a list of packages that reported errors, with + /// the per-package details if any. + /// + /// This only needs to be overridden if the summary should provide extra + /// context. + String get failureListHeader => 'The following packages had errors:'; + + /// The text to output at the end when reporting one or more failures. This + /// will be printed immediately after the a list of packages that reported + /// errors. + /// + /// This only needs to be overridden if the summary should provide extra + /// context. + String get failureListFooter => 'See above for full details.'; + + // ---------------------------------------- + + /// A convenience constant for [runForPackage] success that's more + /// self-documenting than the value. + static const List success = []; + + /// A convenience constant for [runForPackage] failure without additional + /// context that's more self-documenting than the value. + static const List failure = ['']; + + /// Prints a message using a standard format indicating that the package was + /// skipped, with an explanation of why. + void printSkip(String reason) { + print(Colorize('SKIPPING: $reason')..darkGray()); + } + + /// Returns the identifying name to use for [package]. + /// + /// Implementations should not expect a specific format for this string, since + /// it uses heuristics to try to be precise without being overly verbose. If + /// an exact format (e.g., published name, or basename) is required, that + /// should be used instead. + String getPackageDescription(Directory package) { + String packageName = p.relative(package.path, from: packagesDir.path); + final List components = p.split(packageName); + // For the common federated plugin pattern of `foo/foo_subpackage`, drop + // the first part since it's not useful. + if (components.length == 2 && + components[1].startsWith('${components[0]}_')) { + packageName = components[1]; + } + return packageName; + } + + // ---------------------------------------- + + @override + Future run() async { + await initializeRun(); + + final List packages = includeSubpackages + ? await getPackages().toList() + : await getPlugins().toList(); + + final Map> results = >{}; + for (final Directory package in packages) { + _printPackageHeading(package); + results[package] = await runForPackage(package); + } + + // If there were any errors reported, summarize them and exit. + if (results.values.any((List failures) => failures.isNotEmpty)) { + const String indentation = ' '; + printError(failureListHeader); + for (final Directory package in packages) { + final List errors = results[package]!; + if (errors.isNotEmpty) { + final String errorIndentation = indentation * 2; + String errorDetails = errors.join('\n$errorIndentation'); + if (errorDetails.isNotEmpty) { + errorDetails = ':\n$errorIndentation$errorDetails'; + } + printError( + '$indentation${getPackageDescription(package)}$errorDetails'); + } + } + printError(failureListFooter); + throw ToolExit(exitCommandFoundErrors); + } + + printSuccess('\n\nNo issues found!'); + } + + /// Prints the status message indicating that the command is being run for + /// [package]. + /// + /// Something is always printed to make it easier to distinguish between + /// a command running for a package and producing no output, and a command + /// not having been run for a package. + void _printPackageHeading(Directory package) { + String heading = 'Running for ${getPackageDescription(package)}'; + if (hasLongOutput) { + heading = ''' + +============================================================ +|| $heading +============================================================ +'''; + } else { + heading = '$heading...'; + } + print(Colorize(heading)..cyan()); + } +} diff --git a/script/tool/lib/src/common/plugin_command.dart b/script/tool/lib/src/common/plugin_command.dart index 1ab9d8dcc6..4c095858e4 100644 --- a/script/tool/lib/src/common/plugin_command.dart +++ b/script/tool/lib/src/common/plugin_command.dart @@ -14,6 +14,7 @@ import 'git_version_finder.dart'; import 'process_runner.dart'; /// Interface definition for all commands in this tool. +// TODO(stuartmorgan): Move most of this logic to PackageLoopingCommand. abstract class PluginCommand extends Command { /// Creates a command to operate on [packagesDir] with the given environment. PluginCommand( @@ -136,6 +137,8 @@ abstract class PluginCommand extends Command { /// Returns the root Dart package folders of the plugins involved in this /// command execution. + // TODO(stuartmorgan): Rename/restructure this, _getAllPlugins, and + // getPackages, as the current naming is very confusing. Stream getPlugins() async* { // To avoid assuming consistency of `Directory.list` across command // invocations, we collect and sort the plugin folders before sharding. diff --git a/script/tool/lib/src/pubspec_check_command.dart b/script/tool/lib/src/pubspec_check_command.dart index 480d3a4c11..44b6b06154 100644 --- a/script/tool/lib/src/pubspec_check_command.dart +++ b/script/tool/lib/src/pubspec_check_command.dart @@ -4,11 +4,9 @@ import 'package:file/file.dart'; import 'package:git/git.dart'; -import 'package:path/path.dart' as p; import 'package:pubspec_parse/pubspec_parse.dart'; -import 'common/core.dart'; -import 'common/plugin_command.dart'; +import 'common/package_looping_command.dart'; import 'common/process_runner.dart'; /// A command to enforce pubspec conventions across the repository. @@ -16,7 +14,7 @@ import 'common/process_runner.dart'; /// This both ensures that repo best practices for which optional fields are /// used are followed, and that the structure is consistent to make edits /// across multiple pubspec files easier. -class PubspecCheckCommand extends PluginCommand { +class PubspecCheckCommand extends PackageLoopingCommand { /// Creates an instance of the version check command. PubspecCheckCommand( Directory packagesDir, { @@ -52,29 +50,20 @@ class PubspecCheckCommand extends PluginCommand { 'Checks that pubspecs follow repository conventions.'; @override - Future run() async { - final List failingPackages = []; - await for (final Directory package in getPackages()) { - final String relativePackagePath = - p.relative(package.path, from: packagesDir.path); - print('Checking $relativePackagePath...'); - final File pubspec = package.childFile('pubspec.yaml'); - final bool passesCheck = !pubspec.existsSync() || - await _checkPubspec(pubspec, packageName: package.basename); - if (!passesCheck) { - failingPackages.add(relativePackagePath); - } - } + bool get hasLongOutput => false; - if (failingPackages.isNotEmpty) { - print('The following packages have pubspec issues:'); - for (final String package in failingPackages) { - print(' $package'); - } - throw ToolExit(1); - } + @override + bool get includeSubpackages => true; - print('\nNo pubspec issues found!'); + @override + Future> runForPackage(Directory package) async { + final File pubspec = package.childFile('pubspec.yaml'); + final bool passesCheck = !pubspec.existsSync() || + await _checkPubspec(pubspec, packageName: package.basename); + if (!passesCheck) { + return PackageLoopingCommand.failure; + } + return PackageLoopingCommand.success; } Future _checkPubspec( diff --git a/script/tool/lib/src/xctest_command.dart b/script/tool/lib/src/xctest_command.dart index 741aa9d728..3f50feef6f 100644 --- a/script/tool/lib/src/xctest_command.dart +++ b/script/tool/lib/src/xctest_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'; @@ -19,12 +19,15 @@ const String _kXCRunCommand = 'xcrun'; const String _kFoundNoSimulatorsMessage = 'Cannot find any available simulators, tests failed'; +const int _exitFindingSimulatorsFailed = 3; +const int _exitNoSimulators = 4; + /// The command to run XCTests (XCUnitTest and XCUITest) in plugins. /// The tests target have to be added to the Xcode project of the example app, /// usually at "example/{ios,macos}/Runner.xcworkspace". /// /// The static analyzer is also run. -class XCTestCommand extends PluginCommand { +class XCTestCommand extends PackageLoopingCommand { /// Creates an instance of the test command. XCTestCommand( Directory packagesDir, { @@ -41,6 +44,9 @@ class XCTestCommand extends PluginCommand { argParser.addFlag(kPlatformMacos, help: 'Runs the macOS tests'); } + // The device destination flags for iOS tests. + List _iosDestinationFlags = []; + @override final String name = 'xctest'; @@ -50,62 +56,53 @@ class XCTestCommand extends PluginCommand { 'This command requires "flutter" and "xcrun" to be in your path.'; @override - Future run() async { - final bool testIos = getBoolArg(kPlatformIos); - final bool testMacos = getBoolArg(kPlatformMacos); + String get failureListHeader => 'The following packages are failing XCTests:'; - if (!(testIos || testMacos)) { - print('At least one platform flag must be provided.'); - throw ToolExit(2); + @override + Future initializeRun() async { + final bool shouldTestIos = getBoolArg(kPlatformIos); + final bool shouldTestMacos = getBoolArg(kPlatformMacos); + + if (!(shouldTestIos || shouldTestMacos)) { + printError('At least one platform flag must be provided.'); + throw ToolExit(exitInvalidArguments); } - List iosDestinationFlags = []; - if (testIos) { + if (shouldTestIos) { String destination = getStringArg(_kiOSDestination); if (destination.isEmpty) { final String? simulatorId = await _findAvailableIphoneSimulator(); if (simulatorId == null) { - print(_kFoundNoSimulatorsMessage); - throw ToolExit(1); + printError(_kFoundNoSimulatorsMessage); + throw ToolExit(_exitNoSimulators); } destination = 'id=$simulatorId'; } - iosDestinationFlags = [ + _iosDestinationFlags = [ '-destination', destination, ]; } + } - final List failingPackages = []; - await for (final Directory plugin in getPlugins()) { - final String packageName = - p.relative(plugin.path, from: packagesDir.path); - print('============================================================'); - print('Start running for $packageName...'); - bool passed = true; - if (testIos) { - passed &= await _testPlugin(plugin, 'iOS', - extraXcrunFlags: iosDestinationFlags); - } - if (testMacos) { - passed &= await _testPlugin(plugin, 'macOS'); - } - if (!passed) { - failingPackages.add(packageName); - } - } + @override + Future> runForPackage(Directory package) async { + final List failures = []; + final bool testIos = getBoolArg(kPlatformIos); + final bool testMacos = getBoolArg(kPlatformMacos); + // Only provide the failing platform(s) in the summary if testing multiple + // platforms, otherwise it's just noise. + final bool provideErrorDetails = testIos && testMacos; - // Command end, print reports. - if (failingPackages.isEmpty) { - print('All XCTests have passed!'); - } else { - print( - 'The following packages are failing XCTests (see above for details):'); - for (final String package in failingPackages) { - print(' * $package'); - } - throw ToolExit(1); + if (testIos && + !await _testPlugin(package, 'iOS', + extraXcrunFlags: _iosDestinationFlags)) { + failures.add(provideErrorDetails ? 'iOS' : ''); } + if (testMacos && !await _testPlugin(package, 'macOS')) { + failures.add(provideErrorDetails ? 'macOS' : ''); + } + return failures; } /// Runs all applicable tests for [plugin], printing status and returning @@ -117,8 +114,7 @@ class XCTestCommand extends PluginCommand { }) async { if (!pluginSupportsPlatform(platform.toLowerCase(), plugin, requiredMode: PlatformSupport.inline)) { - print('$platform is not implemented by this plugin package.'); - print('\n'); + printSkip('$platform is not implemented by this plugin package.\n'); return true; } bool passing = true; @@ -136,7 +132,7 @@ class XCTestCommand extends PluginCommand { extraFlags: extraXcrunFlags); } if (exitCode == 0) { - print('Successfully ran $platform xctest for $examplePath'); + printSuccess('Successfully ran $platform xctest for $examplePath'); } else { passing = false; } @@ -184,9 +180,10 @@ class XCTestCommand extends PluginCommand { final io.ProcessResult findSimulatorsResult = await processRunner.run(_kXCRunCommand, findSimulatorsArguments); if (findSimulatorsResult.exitCode != 0) { - print('Error occurred while running "$findSimulatorCompleteCommand":\n' + printError( + 'Error occurred while running "$findSimulatorCompleteCommand":\n' '${findSimulatorsResult.stderr}'); - throw ToolExit(1); + throw ToolExit(_exitFindingSimulatorsFailed); } final Map simulatorListJson = jsonDecode(findSimulatorsResult.stdout as String) diff --git a/script/tool/test/common/package_looping_command_test.dart b/script/tool/test/common/package_looping_command_test.dart new file mode 100644 index 0000000000..1012f764a6 --- /dev/null +++ b/script/tool/test/common/package_looping_command_test.dart @@ -0,0 +1,433 @@ +// Copyright 2013 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 'dart:async'; +import 'dart:io'; + +import 'package:args/command_runner.dart'; +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_plugin_tools/src/common/core.dart'; +import 'package:flutter_plugin_tools/src/common/package_looping_command.dart'; +import 'package:flutter_plugin_tools/src/common/process_runner.dart'; +import 'package:git/git.dart'; +import 'package:mockito/mockito.dart'; +import 'package:test/test.dart'; + +import '../util.dart'; +import 'plugin_command_test.mocks.dart'; + +// Constants for colorized output start and end. +const String _startHeadingColor = '\x1B[36m'; +const String _startSkipColor = '\x1B[90m'; +const String _startSuccessColor = '\x1B[32m'; +const String _startErrorColor = '\x1B[31m'; +const String _endColor = '\x1B[0m'; + +// The filename within a package containing errors to return from runForPackage. +const String _errorFile = 'errors'; + +void main() { + late FileSystem fileSystem; + late Directory packagesDir; + late Directory thirdPartyPackagesDir; + + setUp(() { + fileSystem = MemoryFileSystem(); + packagesDir = createPackagesDirectory(fileSystem: fileSystem); + thirdPartyPackagesDir = packagesDir.parent + .childDirectory('third_party') + .childDirectory('packages'); + }); + + /// Creates a TestPackageLoopingCommand instance that uses [gitDiffResponse] + /// for git diffs, and logs output to [printOutput]. + TestPackageLoopingCommand createTestCommand({ + String gitDiffResponse = '', + bool hasLongOutput = true, + bool includeSubpackages = false, + bool failsDuringInit = false, + String? customFailureListHeader, + String? customFailureListFooter, + }) { + // Set up the git diff response. + final MockGitDir gitDir = MockGitDir(); + when(gitDir.runCommand(any, throwOnError: anyNamed('throwOnError'))) + .thenAnswer((Invocation invocation) { + final MockProcessResult mockProcessResult = MockProcessResult(); + if (invocation.positionalArguments[0][0] == 'diff') { + when(mockProcessResult.stdout as String?) + .thenReturn(gitDiffResponse); + } + return Future.value(mockProcessResult); + }); + + return TestPackageLoopingCommand( + packagesDir, + hasLongOutput: hasLongOutput, + includeSubpackages: includeSubpackages, + failsDuringInit: failsDuringInit, + customFailureListHeader: customFailureListHeader, + customFailureListFooter: customFailureListFooter, + gitDir: gitDir, + ); + } + + /// Runs [command] with the given [arguments], and returns its output. + Future> runCommand( + TestPackageLoopingCommand command, { + List arguments = const [], + void Function(Error error)? errorHandler, + }) async { + late CommandRunner runner; + runner = CommandRunner('test_package_looping_command', + 'Test for base package looping functionality'); + runner.addCommand(command); + return await runCapturingPrint( + runner, + [command.name, ...arguments], + errorHandler: errorHandler, + ); + } + + group('tool exit', () { + test('is handled during initializeRun', () async { + final TestPackageLoopingCommand command = + createTestCommand(failsDuringInit: true); + + expect(() => runCommand(command), throwsA(isA())); + }); + + test('does not stop looping', () async { + createFakePackage('package_a', packagesDir); + final Directory failingPackage = + createFakePlugin('package_b', packagesDir); + createFakePackage('package_c', packagesDir); + failingPackage.childFile(_errorFile).createSync(); + + final TestPackageLoopingCommand command = + createTestCommand(hasLongOutput: false); + Error? commandError; + final List output = + await runCommand(command, errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + '${_startHeadingColor}Running for package_a...$_endColor', + '${_startHeadingColor}Running for package_b...$_endColor', + '${_startHeadingColor}Running for package_c...$_endColor', + ])); + }); + }); + + group('package iteration', () { + test('includes plugins and packages', () async { + final Directory plugin = createFakePlugin('a_plugin', packagesDir); + final Directory package = createFakePackage('a_package', packagesDir); + + final TestPackageLoopingCommand command = createTestCommand(); + await runCommand(command); + + expect(command.checkedPackages, + unorderedEquals([plugin.path, package.path])); + }); + + test('includes third_party/packages', () async { + final Directory package1 = createFakePackage('a_package', packagesDir); + final Directory package2 = + createFakePackage('another_package', thirdPartyPackagesDir); + + final TestPackageLoopingCommand command = createTestCommand(); + await runCommand(command); + + expect(command.checkedPackages, + unorderedEquals([package1.path, package2.path])); + }); + + test('includes subpackages when requested', () async { + final Directory plugin = createFakePlugin('a_plugin', packagesDir, + examples: ['example1', 'example2']); + final Directory package = createFakePackage('a_package', packagesDir); + + final TestPackageLoopingCommand command = + createTestCommand(includeSubpackages: true); + await runCommand(command); + + expect( + command.checkedPackages, + unorderedEquals([ + plugin.path, + plugin.childDirectory('example').childDirectory('example1').path, + plugin.childDirectory('example').childDirectory('example2').path, + package.path, + package.childDirectory('example').path, + ])); + }); + }); + + group('output', () { + test('has the expected package headers for long-form output', () async { + createFakePlugin('package_a', packagesDir); + createFakePackage('package_b', packagesDir); + + final TestPackageLoopingCommand command = + createTestCommand(hasLongOutput: true); + final List output = await runCommand(command); + + const String separator = + '============================================================'; + expect( + output, + containsAllInOrder([ + '$_startHeadingColor\n$separator\n|| Running for package_a\n$separator\n$_endColor', + '$_startHeadingColor\n$separator\n|| Running for package_b\n$separator\n$_endColor', + ])); + }); + + test('has the expected package headers for short-form output', () async { + createFakePlugin('package_a', packagesDir); + createFakePackage('package_b', packagesDir); + + final TestPackageLoopingCommand command = + createTestCommand(hasLongOutput: false); + final List output = await runCommand(command); + + expect( + output, + containsAllInOrder([ + '${_startHeadingColor}Running for package_a...$_endColor', + '${_startHeadingColor}Running for package_b...$_endColor', + ])); + }); + + test('shows the success message when nothing fails', () async { + createFakePackage('package_a', packagesDir); + createFakePackage('package_b', packagesDir); + + final TestPackageLoopingCommand command = + createTestCommand(hasLongOutput: false); + final List output = await runCommand(command); + + expect( + output, + containsAllInOrder([ + '$_startSuccessColor\n\nNo issues found!$_endColor', + ])); + }); + + test('shows failure summaries when something fails without extra details', + () async { + createFakePackage('package_a', packagesDir); + final Directory failingPackage1 = + createFakePlugin('package_b', packagesDir); + createFakePackage('package_c', packagesDir); + final Directory failingPackage2 = + createFakePlugin('package_d', packagesDir); + failingPackage1.childFile(_errorFile).createSync(); + failingPackage2.childFile(_errorFile).createSync(); + + final TestPackageLoopingCommand command = + createTestCommand(hasLongOutput: false); + Error? commandError; + final List output = + await runCommand(command, errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + '${_startErrorColor}The following packages had errors:$_endColor', + '$_startErrorColor package_b$_endColor', + '$_startErrorColor package_d$_endColor', + '${_startErrorColor}See above for full details.$_endColor', + ])); + }); + + test('uses custom summary header and footer if provided', () async { + createFakePackage('package_a', packagesDir); + final Directory failingPackage1 = + createFakePlugin('package_b', packagesDir); + createFakePackage('package_c', packagesDir); + final Directory failingPackage2 = + createFakePlugin('package_d', packagesDir); + failingPackage1.childFile(_errorFile).createSync(); + failingPackage2.childFile(_errorFile).createSync(); + + final TestPackageLoopingCommand command = createTestCommand( + hasLongOutput: false, + customFailureListHeader: 'This is a custom header', + customFailureListFooter: 'And a custom footer!'); + Error? commandError; + final List output = + await runCommand(command, errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + '${_startErrorColor}This is a custom header$_endColor', + '$_startErrorColor package_b$_endColor', + '$_startErrorColor package_d$_endColor', + '${_startErrorColor}And a custom footer!$_endColor', + ])); + }); + + test('shows failure summaries when something fails with extra details', + () async { + createFakePackage('package_a', packagesDir); + final Directory failingPackage1 = + createFakePlugin('package_b', packagesDir); + createFakePackage('package_c', packagesDir); + final Directory failingPackage2 = + createFakePlugin('package_d', packagesDir); + final File errorFile1 = failingPackage1.childFile(_errorFile); + errorFile1.createSync(); + errorFile1.writeAsStringSync('just one detail'); + final File errorFile2 = failingPackage2.childFile(_errorFile); + errorFile2.createSync(); + errorFile2.writeAsStringSync('first detail\nsecond detail'); + + final TestPackageLoopingCommand command = + createTestCommand(hasLongOutput: false); + Error? commandError; + final List output = + await runCommand(command, errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + '${_startErrorColor}The following packages had errors:$_endColor', + '$_startErrorColor package_b:\n just one detail$_endColor', + '$_startErrorColor package_d:\n first detail\n second detail$_endColor', + '${_startErrorColor}See above for full details.$_endColor', + ])); + }); + }); + + group('utility', () { + test('printSkip has expected output', () async { + final TestPackageLoopingCommand command = + TestPackageLoopingCommand(packagesDir); + + final List printBuffer = []; + Zone.current.fork(specification: ZoneSpecification( + print: (_, __, ___, String message) { + printBuffer.add(message); + }, + )).run(() => command.printSkip('For a reason')); + + expect(printBuffer.first, + '${_startSkipColor}SKIPPING: For a reason$_endColor'); + }); + + test('getPackageDescription prints packageDir-relative paths by default', + () async { + final TestPackageLoopingCommand command = + TestPackageLoopingCommand(packagesDir); + + expect( + command.getPackageDescription(packagesDir.childDirectory('foo')), + 'foo', + ); + expect( + command.getPackageDescription(packagesDir + .childDirectory('foo') + .childDirectory('bar') + .childDirectory('baz')), + 'foo/bar/baz', + ); + }); + + test( + 'getPackageDescription elides group name in grouped federated plugin structure', + () async { + final TestPackageLoopingCommand command = + TestPackageLoopingCommand(packagesDir); + + expect( + command.getPackageDescription(packagesDir + .childDirectory('a_plugin') + .childDirectory('a_plugin_platform_interface')), + 'a_plugin_platform_interface', + ); + expect( + command.getPackageDescription(packagesDir + .childDirectory('a_plugin') + .childDirectory('a_plugin_web')), + 'a_plugin_web', + ); + }); + }); +} + +class TestPackageLoopingCommand extends PackageLoopingCommand { + TestPackageLoopingCommand( + Directory packagesDir, { + this.hasLongOutput = true, + this.includeSubpackages = false, + this.customFailureListHeader, + this.customFailureListFooter, + this.failsDuringInit = false, + ProcessRunner processRunner = const ProcessRunner(), + GitDir? gitDir, + }) : super(packagesDir, processRunner: processRunner, gitDir: gitDir); + + final List checkedPackages = []; + + final String? customFailureListHeader; + final String? customFailureListFooter; + + final bool failsDuringInit; + + @override + bool hasLongOutput; + + @override + bool includeSubpackages; + + @override + String get failureListHeader => + customFailureListHeader ?? super.failureListHeader; + + @override + String get failureListFooter => + customFailureListFooter ?? super.failureListFooter; + + @override + final String name = 'loop-test'; + + @override + final String description = 'sample package looping command'; + + @override + Future initializeRun() async { + if (failsDuringInit) { + throw ToolExit(2); + } + } + + @override + Future> runForPackage(Directory package) async { + checkedPackages.add(package.path); + final File errorFile = package.childFile(_errorFile); + if (errorFile.existsSync()) { + final List errors = errorFile.readAsLinesSync(); + return errors.isNotEmpty ? errors : PackageLoopingCommand.failure; + } + return PackageLoopingCommand.success; + } +} + +class MockProcessResult extends Mock implements ProcessResult {} diff --git a/script/tool/test/pubspec_check_command_test.dart b/script/tool/test/pubspec_check_command_test.dart index af27ac5bd2..38182a4d18 100644 --- a/script/tool/test/pubspec_check_command_test.dart +++ b/script/tool/test/pubspec_check_command_test.dart @@ -104,10 +104,10 @@ ${devDependenciesSection()} expect( output, - containsAllInOrder([ - 'Checking plugin...', - 'Checking plugin/example...', - '\nNo pubspec issues found!', + containsAllInOrder([ + contains('Running for plugin...'), + contains('Running for plugin/example...'), + contains('No issues found!'), ]), ); }); @@ -129,10 +129,10 @@ ${flutterSection()} expect( output, - containsAllInOrder([ - 'Checking plugin...', - 'Checking plugin/example...', - '\nNo pubspec issues found!', + containsAllInOrder([ + contains('Running for plugin...'), + contains('Running for plugin/example...'), + contains('No issues found!'), ]), ); }); @@ -153,9 +153,9 @@ ${dependenciesSection()} expect( output, - containsAllInOrder([ - 'Checking package...', - '\nNo pubspec issues found!', + containsAllInOrder([ + contains('Running for package...'), + contains('No issues found!'), ]), ); }); diff --git a/script/tool/test/xctest_command_test.dart b/script/tool/test/xctest_command_test.dart index 08af85b39e..b12ad852cd 100644 --- a/script/tool/test/xctest_command_test.dart +++ b/script/tool/test/xctest_command_test.dart @@ -125,7 +125,9 @@ void main() { final List output = await runCapturingPrint(runner, ['xctest', '--ios', _kDestination, 'foo_destination']); expect( - output, contains('iOS is not implemented by this plugin package.')); + output, + contains( + contains('iOS is not implemented by this plugin package.'))); expect(processRunner.recordedCalls, orderedEquals([])); }); @@ -142,7 +144,9 @@ void main() { final List output = await runCapturingPrint(runner, ['xctest', '--ios', _kDestination, 'foo_destination']); expect( - output, contains('iOS is not implemented by this plugin package.')); + output, + contains( + contains('iOS is not implemented by this plugin package.'))); expect(processRunner.recordedCalls, orderedEquals([])); }); @@ -176,10 +180,12 @@ void main() { 'plugin1' ]); - expect(output, isNot(contains('Start running for plugin1...'))); - expect(output, contains('Start running for plugin2...')); - expect(output, - contains('Successfully ran iOS xctest for plugin2/example')); + expect(output, isNot(contains(contains('Running for plugin1')))); + expect(output, contains(contains('Running for plugin2'))); + expect( + output, + contains( + contains('Successfully ran iOS xctest for plugin2/example'))); expect( processRunner.recordedCalls, @@ -271,8 +277,10 @@ void main() { processRunner.processToReturn = mockProcess; final List output = await runCapturingPrint(runner, ['xctest', '--macos', _kDestination, 'foo_destination']); - expect(output, - contains('macOS is not implemented by this plugin package.')); + expect( + output, + contains( + contains('macOS is not implemented by this plugin package.'))); expect(processRunner.recordedCalls, orderedEquals([])); }); @@ -288,8 +296,10 @@ void main() { processRunner.processToReturn = mockProcess; final List output = await runCapturingPrint(runner, ['xctest', '--macos', _kDestination, 'foo_destination']); - expect(output, - contains('macOS is not implemented by this plugin package.')); + expect( + output, + contains( + contains('macOS is not implemented by this plugin package.'))); expect(processRunner.recordedCalls, orderedEquals([])); }); @@ -314,8 +324,10 @@ void main() { '--macos', ]); - expect(output, - contains('Successfully ran macOS xctest for plugin/example')); + expect( + output, + contains( + contains('Successfully ran macOS xctest for plugin/example'))); expect( processRunner.recordedCalls,