[flutter_plugin_tools] Track and log exclusions (flutter#4205)

Makes commands that use the package-looping base command track and
report exclusions. This will make it much easier to debug/audit
situations where tests aren't running when expected (e.g., when enabling
a new type of test for a package that previously had to be explicitly
excluded from that test to avoid failing for having no tests, but
forgetting to remove the package from the exclusion list).

Also fixes a latent issue with using different exclusion lists on
different commands in a single CI task when using sharding could cause
unexpected failures due to different sets of plugins being included for
each step (e.g., build+drive with an exclude list on drive could
potentially try to drive a plugin that hadn't been built in that shard)
by sharding before filtering out excluded packages.

Adds testing for sharding in general, as there was previously none.

Author: stuartmorgan-g

script/tool/CHANGELOG.md

@@ -21,6 +21,8 @@
     `--no-integration`.
 - **Breaking change**: Replaced `java-test` with Android unit test support for
   the new `native-test` command.
+- Commands that print a run summary at the end now track and log exclusions
+  similarly to skips for easier auditing.
 
 ## 0.4.1
 

script/tool/lib/src/analyze_command.dart

@@ -5,6 +5,7 @@
 import 'dart:async';
 
 import 'package:file/file.dart';
+import 'package:flutter_plugin_tools/src/common/plugin_command.dart';
 import 'package:platform/platform.dart';
 import 'package:yaml/yaml.dart';
 
@@ -84,7 +85,10 @@ class AnalyzeCommand extends PackageLoopingCommand {
   /// Ensures that the dependent packages have been fetched for all packages
   /// (including their sub-packages) that will be analyzed.
   Future<bool> _runPackagesGetOnTargetPackages() async {
-    final List<Directory> packageDirectories = await getPackages().toList();
+    final List<Directory> packageDirectories =
+        await getTargetPackagesAndSubpackages()
+            .map((PackageEnumerationEntry package) => package.directory)
+            .toList();
     final Set<String> packagePaths =
         packageDirectories.map((Directory dir) => dir.path).toSet();
     packageDirectories.removeWhere((Directory directory) {

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

@@ -22,6 +22,10 @@ enum RunState {
   /// The command was skipped for the package.
   skipped,
 
+  /// The command was skipped for the package because it was explicitly excluded
+  /// in the command arguments.
+  excluded,
+
   /// The command failed for the package.
   failed,
 }
@@ -35,6 +39,9 @@ class PackageResult {
   PackageResult.skip(String reason)
       : this._(RunState.skipped, <String>[reason]);
 
+  /// A run that was excluded by the command invocation.
+  PackageResult.exclude() : this._(RunState.excluded);
+
   /// A run that failed.
   ///
   /// If [errors] are provided, they will be listed in the summary, otherwise
@@ -70,13 +77,14 @@ abstract class PackageLoopingCommand extends PluginCommand {
             processRunner: processRunner, platform: platform, gitDir: gitDir);
 
   /// Packages that had at least one [logWarning] call.
-  final Set<Directory> _packagesWithWarnings = <Directory>{};
+  final Set<PackageEnumerationEntry> _packagesWithWarnings =
+      <PackageEnumerationEntry>{};
 
   /// Number of warnings that happened outside of a [runForPackage] call.
   int _otherWarningCount = 0;
 
   /// The package currently being run by [runForPackage].
-  Directory? _currentPackage;
+  PackageEnumerationEntry? _currentPackage;
 
   /// 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
@@ -215,15 +223,24 @@ abstract class PackageLoopingCommand extends PluginCommand {
 
     await initializeRun();
 
-    final List<Directory> packages = includeSubpackages
-        ? await getPackages().toList()
-        : await getPlugins().toList();
+    final List<PackageEnumerationEntry> packages = includeSubpackages
+        ? await getTargetPackagesAndSubpackages(filterExcluded: false).toList()
+        : await getTargetPackages(filterExcluded: false).toList();
 
-    final Map<Directory, PackageResult> results = <Directory, PackageResult>{};
-    for (final Directory package in packages) {
+    final Map<PackageEnumerationEntry, PackageResult> results =
+        <PackageEnumerationEntry, PackageResult>{};
+    for (final PackageEnumerationEntry package in packages) {
       _currentPackage = package;
       _printPackageHeading(package);
-      final PackageResult result = await runForPackage(package);
+
+      // Command implementations should never see excluded packages; they are
+      // included at this level only for logging.
+      if (package.excluded) {
+        results[package] = PackageResult.exclude();
+        continue;
+      }
+
+      final PackageResult result = await runForPackage(package.directory);
       if (result.state == RunState.skipped) {
         final String message =
             '${indentation}SKIPPING: ${result.details.first}';
@@ -266,33 +283,47 @@ abstract class PackageLoopingCommand extends PluginCommand {
   /// 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)}';
+  void _printPackageHeading(PackageEnumerationEntry package) {
+    final String packageDisplayName = getPackageDescription(package.directory);
+    String heading = package.excluded
+        ? 'Not running for $packageDisplayName; excluded'
+        : 'Running for $packageDisplayName';
     if (hasLongOutput) {
       heading = '''
 
 ============================================================
 || $heading
 ============================================================
 ''';
-    } else {
+    } else if (!package.excluded) {
       heading = '$heading...';
     }
-    captureOutput ? print(heading) : print(Colorize(heading)..cyan());
+    if (captureOutput) {
+      print(heading);
+    } else {
+      final Colorize colorizeHeading = Colorize(heading);
+      print(package.excluded
+          ? colorizeHeading.darkGray()
+          : colorizeHeading.cyan());
+    }
   }
 
   /// Prints a summary of packges run, packages skipped, and warnings.
-  void _printRunSummary(
-      List<Directory> packages, Map<Directory, PackageResult> results) {
-    final Set<Directory> skippedPackages = results.entries
-        .where((MapEntry<Directory, PackageResult> entry) =>
+  void _printRunSummary(List<PackageEnumerationEntry> packages,
+      Map<PackageEnumerationEntry, PackageResult> results) {
+    final Set<PackageEnumerationEntry> skippedPackages = results.entries
+        .where((MapEntry<PackageEnumerationEntry, PackageResult> entry) =>
             entry.value.state == RunState.skipped)
-        .map((MapEntry<Directory, PackageResult> entry) => entry.key)
+        .map((MapEntry<PackageEnumerationEntry, PackageResult> entry) =>
+            entry.key)
         .toSet();
-    final int skipCount = skippedPackages.length;
+    final int skipCount = skippedPackages.length +
+        packages
+            .where((PackageEnumerationEntry package) => package.excluded)
+            .length;
     // Split the warnings into those from packages that ran, and those that
     // were skipped.
-    final Set<Directory> _skippedPackagesWithWarnings =
+    final Set<PackageEnumerationEntry> _skippedPackagesWithWarnings =
         _packagesWithWarnings.intersection(skippedPackages);
     final int skippedWarningCount = _skippedPackagesWithWarnings.length;
     final int runWarningCount =
@@ -318,14 +349,17 @@ abstract class PackageLoopingCommand extends PluginCommand {
 
   /// Prints a one-line-per-package overview of the run results for each
   /// package.
-  void _printPerPackageRunOverview(List<Directory> packages,
-      {required Set<Directory> skipped}) {
+  void _printPerPackageRunOverview(List<PackageEnumerationEntry> packages,
+      {required Set<PackageEnumerationEntry> skipped}) {
     print('Run overview:');
-    for (final Directory package in packages) {
+    for (final PackageEnumerationEntry package in packages) {
       final bool hadWarning = _packagesWithWarnings.contains(package);
       Styles style;
       String summary;
-      if (skipped.contains(package)) {
+      if (package.excluded) {
+        summary = 'excluded';
+        style = Styles.DARK_GRAY;
+      } else if (skipped.contains(package)) {
         summary = 'skipped';
         style = hadWarning ? Styles.LIGHT_YELLOW : Styles.DARK_GRAY;
       } else {
@@ -339,17 +373,17 @@ abstract class PackageLoopingCommand extends PluginCommand {
       if (!captureOutput) {
         summary = (Colorize(summary)..apply(style)).toString();
       }
-      print('  ${getPackageDescription(package)} - $summary');
+      print('  ${getPackageDescription(package.directory)} - $summary');
     }
     print('');
   }
 
   /// Prints a summary of all of the failures from [results].
-  void _printFailureSummary(
-      List<Directory> packages, Map<Directory, PackageResult> results) {
+  void _printFailureSummary(List<PackageEnumerationEntry> packages,
+      Map<PackageEnumerationEntry, PackageResult> results) {
     const String indentation = '  ';
     _printError(failureListHeader);
-    for (final Directory package in packages) {
+    for (final PackageEnumerationEntry package in packages) {
       final PackageResult result = results[package]!;
       if (result.state == RunState.failed) {
         final String errorIndentation = indentation * 2;
@@ -359,7 +393,7 @@ abstract class PackageLoopingCommand extends PluginCommand {
               ':\n$errorIndentation${result.details.join('\n$errorIndentation')}';
         }
         _printError(
-            '$indentation${getPackageDescription(package)}$errorDetails');
+            '$indentation${getPackageDescription(package.directory)}$errorDetails');
       }
     }
     _printError(failureListFooter);