Migrate command, add failure test, remove skip (#4106)

Switches `podspec` to the new base command that handles the boilerplate of looping over target packages.

Includes test improvements:
- Adds failure tests; previously no failure cases were covered.
- Captures output using the standard mechanism instead of using a custom
  `_print`, simplifying the command code.

Also removes `--skip`, which is redundant with `--exclude`.

Part of flutter/flutter#83413

Author: stuartmorgan-g

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
 

script/tool/lib/src/common/plugin_command.dart

@@ -250,10 +250,16 @@ abstract class PluginCommand extends Command<void> {
   /// Returns the files contained, recursively, within the plugins
   /// involved in this command execution.
   Stream<File> getFiles() {
-    return getPlugins().asyncExpand<File>((Directory folder) => folder
+    return getPlugins()
+        .asyncExpand<File>((Directory folder) => getFilesForPackage(folder));
+  }
+
+  /// Returns the files contained, recursively, within [package].
+  Stream<File> getFilesForPackage(Directory package) {
+    return package
         .list(recursive: true, followLinks: false)
         .where((FileSystemEntity entity) => entity is File)
-        .cast<File>());
+        .cast<File>();
   }
 
   /// Returns whether the specified entity is a directory containing a

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<void> run() async {
+  Future<void> 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<String> failingPlugins = <String>[];
-    for (final File podspec in await _podspecsToLint()) {
+  @override
+  Future<List<String>> runForPackage(Directory package) async {
+    final List<String> errors = <String>[];
+    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<List<File>> _podspecsToLint() async {
-    final List<File> podspecs = await getFiles().where((File entity) {
+  Future<List<File>> _podspecsToLint(Directory package) async {
+    final List<File> 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);
   }

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<void> runner;
     late MockPlatform mockPlatform;
     late RecordingProcessRunner processRunner;
-    late List<String> printedMessages;
 
     setUp(() {
       fileSystem = MemoryFileSystem();
       packagesDir = createPackagesDirectory(fileSystem: fileSystem);
 
-      printedMessages = <String>[];
       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: <String>['plugin1.podspec']);
-
       mockPlatform.isMacOS = false;
-      await runner.run(<String>['podspecs']);
+
+      Error? commandError;
+      final List<String> output = await runCapturingPrint(
+          runner, <String>['podspecs'], errorHandler: (Error e) {
+        commandError = e;
+      });
+
+      expect(commandError, isA<ToolExit>());
 
       expect(
         processRunner.recordedCalls,
         equals(<ProcessCall>[]),
       );
+
+      expect(
+          output,
+          containsAllInOrder(
+            <Matcher>[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(<String>['podspecs']);
+      final List<String> output =
+          await runCapturingPrint(runner, <String>['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: <String>['plugin1.podspec']);
-      createFakePlugin('plugin2', packagesDir,
-          extraFiles: <String>['plugin2.podspec']);
-
-      await runner
-          .run(<String>['podspecs', '--skip=plugin1', '--skip=plugin2']);
-
-      expect(
-        processRunner.recordedCalls,
-        orderedEquals(<ProcessCall>[
-          ProcessCall('which', const <String>['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: <String>['plugin1.podspec']);
 
-      await runner.run(<String>['podspecs', '--ignore-warnings=plugin1']);
+      final List<String> output = await runCapturingPrint(
+          runner, <String>['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: <String>['plugin1.podspec']);
+
+      // Simulate failure from `pod`.
+      final MockProcess mockDriveProcess = MockProcess();
+      mockDriveProcess.exitCodeCompleter.complete(1);
+      processRunner.processToReturn = mockDriveProcess;
+
+      Error? commandError;
+      final List<String> output = await runCapturingPrint(
+          runner, <String>['podspecs'], errorHandler: (Error e) {
+        commandError = e;
+      });
+
+      expect(commandError, isA<ToolExit>());
+
+      expect(
+          output,
+          containsAllInOrder(
+            <Matcher>[
+              contains('The following packages had errors:'),
+              contains('plugin1:\n'
+                  '    plugin1.podspec')
+            ],
+          ));
     });
   });
 }