Make run-on-changed-packages flag handle repo-level changes (#3946)

Changes to some files (e.g., CI scripts) have the potential to cause
failures in any packages, without changes to those packages themselves.
This updates the --run-on-changed-packages to consider all packages as
changed if any of those files are changed, to avoid issues where a
change that changes both some repo-level files and some package-specific
files only run presubmit tests on the packages that are directly
changed, causing post-submit-only failures.

Fixes flutter/flutter#82965

script/tool/CHANGELOG.md

@@ -1,8 +1,10 @@
-## NEXT
+## 0.1.2
 
 - Add `against-pub` flag for version-check, which allows the command to check version with pub.
 - Add `machine` flag for publish-check, which replaces outputs to something parsable by machines.
 - Add `skip-conformation` flag to publish-plugin to allow auto publishing.
+- Change `run-on-changed-packages` to consider all packages as changed if any
+  files have been changed that could affect the entire repository.
 
 ## 0.1.1
 

script/tool/lib/src/common.dart

@@ -203,7 +203,8 @@ abstract class PluginCommand extends Command<void> {
     argParser.addFlag(_runOnChangedPackagesArg,
         help: 'Run the command on changed packages/plugins.\n'
             'If the $_pluginsArg is specified, this flag is ignored.\n'
-            'If no plugins have changed, the command runs on all plugins.\n'
+            'If no packages have changed, or if there have been changes that may\n'
+            'affect all packages, the command runs on all packages.\n'
             'The packages excluded with $_excludeArg is also excluded even if changed.\n'
             'See $_kBaseSha if a custom base is needed to determine the diff.');
     argParser.addOption(_kBaseSha,
@@ -335,7 +336,9 @@ abstract class PluginCommand extends Command<void> {
     final Set<String> excludedPlugins =
         Set<String>.from(getStringListArg(_excludeArg));
     final bool runOnChangedPackages = getBoolArg(_runOnChangedPackagesArg);
-    if (plugins.isEmpty && runOnChangedPackages) {
+    if (plugins.isEmpty &&
+        runOnChangedPackages &&
+        !(await _changesRequireFullTest())) {
       plugins = await _getChangedPackages();
     }
 
@@ -458,6 +461,7 @@ abstract class PluginCommand extends Command<void> {
     return gitVersionFinder;
   }
 
+  // Returns packages that have been changed relative to the git base.
   Future<Set<String>> _getChangedPackages() async {
     final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
 
@@ -472,14 +476,40 @@ abstract class PluginCommand extends Command<void> {
         packages.add(pathComponents[packagesIndex + 1]);
       }
     }
-    if (packages.isNotEmpty) {
-      final String changedPackages = packages.join(',');
-      print(changedPackages);
-    } else {
+    if (packages.isEmpty) {
       print('No changed packages.');
+    } else {
+      final String changedPackages = packages.join(',');
+      print('Changed packages: $changedPackages');
     }
     return packages;
   }
+
+  // Returns true if one or more files changed that have the potential to affect
+  // any plugin (e.g., CI script changes).
+  Future<bool> _changesRequireFullTest() async {
+    final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
+
+    const List<String> specialFiles = <String>[
+      '.ci.yaml', // LUCI config.
+      '.cirrus.yml', // Cirrus config.
+      '.clang-format', // ObjC and C/C++ formatting options.
+      'analysis_options.yaml', // Dart analysis settings.
+    ];
+    const List<String> specialDirectories = <String>[
+      '.ci/', // Support files for CI.
+      'script/', // This tool, and its wrapper scripts.
+    ];
+    // Directory entries must end with / to avoid over-matching, since the
+    // check below is done via string prefixing.
+    assert(specialDirectories.every((String dir) => dir.endsWith('/')));
+
+    final List<String> allChangedFiles =
+        await gitVersionFinder.getChangedFiles();
+    return allChangedFiles.any((String path) =>
+        specialFiles.contains(path) ||
+        specialDirectories.any((String dir) => path.startsWith(dir)));
+  }
 }
 
 /// A class used to run processes.

script/tool/pubspec.yaml

@@ -1,7 +1,7 @@
 name: flutter_plugin_tools
 description: Productivity utils for flutter/plugins and flutter/packages
 repository: https://github.com/flutter/plugins/tree/master/script/tool
-version: 0.1.1
+version: 0.1.2
 
 dependencies:
   args: ^2.1.0

script/tool/test/common_test.dart

@@ -145,7 +145,103 @@ void main() {
 
     test('all plugins should be tested if there are no plugin related changes.',
         () async {
-      gitDiffResponse = '.cirrus';
+      gitDiffResponse = 'AUTHORS';
+      final Directory plugin1 =
+          createFakePlugin('plugin1', packagesDirectory: packagesDir);
+      final Directory plugin2 =
+          createFakePlugin('plugin2', packagesDirectory: packagesDir);
+      await runner.run(
+          <String>['sample', '--base-sha=master', '--run-on-changed-packages']);
+
+      expect(plugins, unorderedEquals(<String>[plugin1.path, plugin2.path]));
+    });
+
+    test('all plugins should be tested if .cirrus.yml changes.',
+        () async {
+      gitDiffResponse = '''
+.cirrus.yml
+packages/plugin1/CHANGELOG
+''';
+      final Directory plugin1 =
+          createFakePlugin('plugin1', packagesDirectory: packagesDir);
+      final Directory plugin2 =
+          createFakePlugin('plugin2', packagesDirectory: packagesDir);
+      await runner.run(
+          <String>['sample', '--base-sha=master', '--run-on-changed-packages']);
+
+      expect(plugins, unorderedEquals(<String>[plugin1.path, plugin2.path]));
+    });
+
+    test('all plugins should be tested if .ci.yaml changes',
+        () async {
+      gitDiffResponse = '''
+.ci.yaml
+packages/plugin1/CHANGELOG
+''';
+      final Directory plugin1 =
+          createFakePlugin('plugin1', packagesDirectory: packagesDir);
+      final Directory plugin2 =
+          createFakePlugin('plugin2', packagesDirectory: packagesDir);
+      await runner.run(
+          <String>['sample', '--base-sha=master', '--run-on-changed-packages']);
+
+      expect(plugins, unorderedEquals(<String>[plugin1.path, plugin2.path]));
+    });
+
+    test('all plugins should be tested if anything in .ci/ changes',
+        () async {
+      gitDiffResponse = '''
+.ci/Dockerfile
+packages/plugin1/CHANGELOG
+''';
+      final Directory plugin1 =
+          createFakePlugin('plugin1', packagesDirectory: packagesDir);
+      final Directory plugin2 =
+          createFakePlugin('plugin2', packagesDirectory: packagesDir);
+      await runner.run(
+          <String>['sample', '--base-sha=master', '--run-on-changed-packages']);
+
+      expect(plugins, unorderedEquals(<String>[plugin1.path, plugin2.path]));
+    });
+
+    test('all plugins should be tested if anything in script changes.',
+        () async {
+      gitDiffResponse = '''
+script/tool_runner.sh
+packages/plugin1/CHANGELOG
+''';
+      final Directory plugin1 =
+          createFakePlugin('plugin1', packagesDirectory: packagesDir);
+      final Directory plugin2 =
+          createFakePlugin('plugin2', packagesDirectory: packagesDir);
+      await runner.run(
+          <String>['sample', '--base-sha=master', '--run-on-changed-packages']);
+
+      expect(plugins, unorderedEquals(<String>[plugin1.path, plugin2.path]));
+    });
+
+    test('all plugins should be tested if the root analysis options change.',
+        () async {
+      gitDiffResponse = '''
+analysis_options.yaml
+packages/plugin1/CHANGELOG
+''';
+      final Directory plugin1 =
+          createFakePlugin('plugin1', packagesDirectory: packagesDir);
+      final Directory plugin2 =
+          createFakePlugin('plugin2', packagesDirectory: packagesDir);
+      await runner.run(
+          <String>['sample', '--base-sha=master', '--run-on-changed-packages']);
+
+      expect(plugins, unorderedEquals(<String>[plugin1.path, plugin2.path]));
+    });
+
+    test('all plugins should be tested if formatting options change.',
+        () async {
+      gitDiffResponse = '''
+.clang-format
+packages/plugin1/CHANGELOG
+''';
       final Directory plugin1 =
           createFakePlugin('plugin1', packagesDirectory: packagesDir);
       final Directory plugin2 =