From c324fff934286f5bc17d87739e97f2f52558ee1b Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 25 Jun 2021 13:26:50 -0400 Subject: [PATCH] Migrate command, add failure test, remove skip --- script/tool/CHANGELOG.md | 3 +- .../tool/lib/src/common/plugin_command.dart | 10 ++- .../tool/lib/src/lint_podspecs_command.dart | 65 ++++++--------- .../tool/test/lint_podspecs_command_test.dart | 79 ++++++++++++------- 4 files changed, 87 insertions(+), 70 deletions(-) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 3e2ec3f4c991..1b6cf2a44718 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -7,7 +7,8 @@ - **Breaking change**: it now requires an `--ios` and/or `--macos` flag. - The tooling now runs in strong null-safe mode. - `publish plugins` check against pub.dev to determine if a release should happen. -- Modified the output format of `pubspec-check` and `xctest` +- Modified the output format of many commands +- Removed `podspec`'s `--skip` in favor of `--ignore` using the new structure. ## 0.2.0 diff --git a/script/tool/lib/src/common/plugin_command.dart b/script/tool/lib/src/common/plugin_command.dart index 4c095858e45a..e3ee109dd0cf 100644 --- a/script/tool/lib/src/common/plugin_command.dart +++ b/script/tool/lib/src/common/plugin_command.dart @@ -250,10 +250,16 @@ abstract class PluginCommand extends Command { /// Returns the files contained, recursively, within the plugins /// involved in this command execution. Stream getFiles() { - return getPlugins().asyncExpand((Directory folder) => folder + return getPlugins() + .asyncExpand((Directory folder) => getFilesForPackage(folder)); + } + + /// Returns the files contained, recursively, within [package]. + Stream getFilesForPackage(Directory package) { + return package .list(recursive: true, followLinks: false) .where((FileSystemEntity entity) => entity is File) - .cast()); + .cast(); } /// Returns whether the specified entity is a directory containing a diff --git a/script/tool/lib/src/lint_podspecs_command.dart b/script/tool/lib/src/lint_podspecs_command.dart index 5e86d2be40b8..2b4beeb92a1f 100644 --- a/script/tool/lib/src/lint_podspecs_command.dart +++ b/script/tool/lib/src/lint_podspecs_command.dart @@ -10,29 +10,26 @@ import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'common/core.dart'; -import 'common/plugin_command.dart'; +import 'common/package_looping_command.dart'; import 'common/process_runner.dart'; +const int _exitUnsupportedPlatform = 2; + /// Lint the CocoaPod podspecs and run unit tests. /// /// See https://guides.cocoapods.org/terminal/commands.html#pod_lib_lint. -class LintPodspecsCommand extends PluginCommand { +class LintPodspecsCommand extends PackageLoopingCommand { /// Creates an instance of the linter command. LintPodspecsCommand( Directory packagesDir, { ProcessRunner processRunner = const ProcessRunner(), Platform platform = const LocalPlatform(), - Print print = print, }) : _platform = platform, - _print = print, super(packagesDir, processRunner: processRunner) { - argParser.addMultiOption('skip', - help: - 'Skip all linting for podspecs with this basename (example: federated plugins with placeholder podspecs)', - valueHelp: 'podspec_file_name'); argParser.addMultiOption('ignore-warnings', help: - 'Do not pass --allow-warnings flag to "pod lib lint" for podspecs with this basename (example: plugins with known warnings)', + 'Do not pass --allow-warnings flag to "pod lib lint" for podspecs ' + 'with this basename (example: plugins with known warnings)', valueHelp: 'podspec_file_name'); } @@ -49,13 +46,11 @@ class LintPodspecsCommand extends PluginCommand { final Platform _platform; - final Print _print; - @override - Future run() async { + Future initializeRun() async { if (!_platform.isMacOS) { - _print('Detected platform is not macOS, skipping podspec lint'); - return; + printError('This command is only supported on macOS'); + throw ToolExit(_exitUnsupportedPlatform); } await processRunner.run( @@ -65,32 +60,24 @@ class LintPodspecsCommand extends PluginCommand { exitOnError: true, logOnError: true, ); + } - _print('Starting podspec lint test'); - - final List failingPlugins = []; - for (final File podspec in await _podspecsToLint()) { + @override + Future> runForPackage(Directory package) async { + final List errors = []; + for (final File podspec in await _podspecsToLint(package)) { if (!await _lintPodspec(podspec)) { - failingPlugins.add(p.basenameWithoutExtension(podspec.path)); - } - } - - _print('\n\n'); - if (failingPlugins.isNotEmpty) { - _print('The following plugins have podspec errors (see above):'); - for (final String plugin in failingPlugins) { - _print(' * $plugin'); + errors.add(p.basename(podspec.path)); } - throw ToolExit(1); } + return errors; } - Future> _podspecsToLint() async { - final List podspecs = await getFiles().where((File entity) { + Future> _podspecsToLint(Directory package) async { + final List podspecs = + await getFilesForPackage(package).where((File entity) { final String filePath = entity.path; - return p.extension(filePath) == '.podspec' && - !getStringListArg('skip') - .contains(p.basenameWithoutExtension(filePath)); + return p.extension(filePath) == '.podspec'; }).toList(); podspecs.sort( @@ -103,19 +90,19 @@ class LintPodspecsCommand extends PluginCommand { final String podspecPath = podspec.path; final String podspecBasename = p.basename(podspecPath); - _print('Linting $podspecBasename'); + print('Linting $podspecBasename'); // Lint plugin as framework (use_frameworks!). final ProcessResult frameworkResult = await _runPodLint(podspecPath, libraryLint: true); - _print(frameworkResult.stdout); - _print(frameworkResult.stderr); + print(frameworkResult.stdout); + print(frameworkResult.stderr); // Lint plugin as library. final ProcessResult libraryResult = await _runPodLint(podspecPath, libraryLint: false); - _print(libraryResult.stdout); - _print(libraryResult.stderr); + print(libraryResult.stdout); + print(libraryResult.stderr); return frameworkResult.exitCode == 0 && libraryResult.exitCode == 0; } @@ -135,7 +122,7 @@ class LintPodspecsCommand extends PluginCommand { if (libraryLint) '--use-libraries' ]; - _print('Running "pod ${arguments.join(' ')}"'); + print('Running "pod ${arguments.join(' ')}"'); return processRunner.run('pod', arguments, workingDir: packagesDir, stdoutEncoding: utf8, stderrEncoding: utf8); } diff --git a/script/tool/test/lint_podspecs_command_test.dart b/script/tool/test/lint_podspecs_command_test.dart index d86c9145fc19..a6a5502913a0 100644 --- a/script/tool/test/lint_podspecs_command_test.dart +++ b/script/tool/test/lint_podspecs_command_test.dart @@ -5,6 +5,7 @@ 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/lint_podspecs_command.dart'; import 'package:path/path.dart' as p; import 'package:test/test.dart'; @@ -19,20 +20,17 @@ void main() { late CommandRunner runner; late MockPlatform mockPlatform; late RecordingProcessRunner processRunner; - late List printedMessages; setUp(() { fileSystem = MemoryFileSystem(); packagesDir = createPackagesDirectory(fileSystem: fileSystem); - printedMessages = []; mockPlatform = MockPlatform(isMacOS: true); processRunner = RecordingProcessRunner(); final LintPodspecsCommand command = LintPodspecsCommand( packagesDir, processRunner: processRunner, platform: mockPlatform, - print: (Object? message) => printedMessages.add(message.toString()), ); runner = @@ -47,14 +45,26 @@ void main() { test('only runs on macOS', () async { createFakePlugin('plugin1', packagesDir, extraFiles: ['plugin1.podspec']); - mockPlatform.isMacOS = false; - await runner.run(['podspecs']); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['podspecs'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); expect( processRunner.recordedCalls, equals([]), ); + + expect( + output, + containsAllInOrder( + [contains('only supported on macOS')], + )); }); test('runs pod lib lint on a podspec', () async { @@ -70,7 +80,8 @@ void main() { processRunner.resultStdout = 'Foo'; processRunner.resultStderr = 'Bar'; - await runner.run(['podspecs']); + final List output = + await runCapturingPrint(runner, ['podspecs']); expect( processRunner.recordedCalls, @@ -102,33 +113,17 @@ void main() { ]), ); - expect(printedMessages, contains('Linting plugin1.podspec')); - expect(printedMessages, contains('Foo')); - expect(printedMessages, contains('Bar')); - }); - - test('skips podspecs with known issues', () async { - createFakePlugin('plugin1', packagesDir, - extraFiles: ['plugin1.podspec']); - createFakePlugin('plugin2', packagesDir, - extraFiles: ['plugin2.podspec']); - - await runner - .run(['podspecs', '--skip=plugin1', '--skip=plugin2']); - - expect( - processRunner.recordedCalls, - orderedEquals([ - ProcessCall('which', const ['pod'], packagesDir.path), - ]), - ); + expect(output, contains('Linting plugin1.podspec')); + expect(output, contains('Foo')); + expect(output, contains('Bar')); }); test('allow warnings for podspecs with known warnings', () async { final Directory plugin1Dir = createFakePlugin('plugin1', packagesDir, extraFiles: ['plugin1.podspec']); - await runner.run(['podspecs', '--ignore-warnings=plugin1']); + final List output = await runCapturingPrint( + runner, ['podspecs', '--ignore-warnings=plugin1']); expect( processRunner.recordedCalls, @@ -162,7 +157,35 @@ void main() { ]), ); - expect(printedMessages, contains('Linting plugin1.podspec')); + expect(output, contains('Linting plugin1.podspec')); + }); + + test('fails if linting fails', () async { + createFakePlugin('plugin1', packagesDir, + extraFiles: ['plugin1.podspec']); + + // Simulate failure from `pod`. + final MockProcess mockDriveProcess = MockProcess(); + mockDriveProcess.exitCodeCompleter.complete(1); + processRunner.processToReturn = mockDriveProcess; + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['podspecs'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + + expect( + output, + containsAllInOrder( + [ + contains('The following packages had errors:'), + contains('plugin1:\n' + ' plugin1.podspec') + ], + )); }); }); }