[flutter_plugin_tools] Support YAML exception lists (#4183)
Currently the tool accepts `--custom-analysis` to allow a list of packages for which custom `analysis_options.yaml` are allowed, and `--exclude` to exclude a set of packages when running a command against all, or all changed, packages. This results in these exception lists being embedded into CI configuration files (e.g., .cirrus.yaml) or scripts, which makes them harder to maintain, and harder to re-use in other contexts (local runs, new CI systems).
This adds support for both flags to accept paths to YAML files that contain the lists, so that they can be maintained separately, and with inline comments about the reasons things are on the lists.
Also updates the CI to use this new support, eliminating those lists from `.cirrus.yaml` and `tool_runner.sh`
Fixes https://github.com/flutter/flutter/issues/86799
diff --git a/.cirrus.yml b/.cirrus.yml
index edefc19..54c4c37 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -72,7 +72,7 @@
- cd script/tool
- dart analyze --fatal-infos
script:
- - ./script/tool_runner.sh analyze
+ - ./script/tool_runner.sh analyze --custom-analysis=script/configs/custom_analysis.yaml
### Android tasks ###
- name: build_all_plugins_apk
env:
@@ -137,22 +137,6 @@
CHANNEL: "stable"
MAPS_API_KEY: ENCRYPTED[596a9f6bca436694625ac50851dc5da6b4d34cba8025f7db5bc9465142e8cd44e15f69e3507787753accebfc4910d550]
GCLOUD_FIREBASE_TESTLAB_KEY: ENCRYPTED[07586610af1fdfc894e5969f70ef2458341b9b7e9c3b7c4225a663b4a48732b7208a4d91c3b7d45305a6b55fa2a37fc4]
- # Currently missing harness files (https://github.com/flutter/flutter/issues/86749):
- # camera/camera
- # google_sign_in/google_sign_in
- # in_app_purchase/in_app_purchase
- # in_app_purchase_android
- # quick_actions
- # shared_preferences/shared_preferences
- # url_launcher/url_launcher
- # video_player/video_player
- # webview_flutter
- # Deprecated; no plan to backfill the missing files:
- # android_intent,connectivity/connectivity,device_info/device_info,sensors,share,wifi_info_flutter/wifi_info_flutter
- # No integration tests to run:
- # image_picker/image_picker - Native UI is the critical functionality
- # espresso - No Dart code, so no integration tests
- PLUGINS_TO_EXCLUDE_INTEGRATION_TESTS: "camera/camera,google_sign_in/google_sign_in,in_app_purchase/in_app_purchase,in_app_purchase_android,quick_actions,shared_preferences/shared_preferences,url_launcher/url_launcher,video_player/video_player,webview_flutter,android_intent,connectivity/connectivity,device_info/device_info,sensors,share,wifi_info_flutter/wifi_info_flutter,image_picker/image_picker,espresso"
build_script:
# Unsetting CIRRUS_CHANGE_MESSAGE and CIRRUS_COMMIT_MESSAGE as they
# might include non-ASCII characters which makes Gradle crash.
@@ -177,16 +161,13 @@
- export CIRRUS_COMMIT_MESSAGE=""
- if [[ -n "$GCLOUD_FIREBASE_TESTLAB_KEY" ]]; then
- echo $GCLOUD_FIREBASE_TESTLAB_KEY > ${HOME}/gcloud-service-key.json
- - ./script/tool_runner.sh firebase-test-lab --device model=flame,version=29 --device model=starqlteue,version=26 --exclude $PLUGINS_TO_EXCLUDE_INTEGRATION_TESTS
+ - ./script/tool_runner.sh firebase-test-lab --device model=flame,version=29 --device model=starqlteue,version=26 --exclude=script/configs/exclude_integration_android.yaml
- else
- echo "This user does not have permission to run Firebase Test Lab tests."
- fi
### Web tasks ###
- name: build-web+drive-examples
env:
- # Currently missing; see https://github.com/flutter/flutter/issues/81982
- # and https://github.com/flutter/flutter/issues/82211
- PLUGINS_TO_EXCLUDE_INTEGRATION_TESTS: "file_selector,shared_preferences_web"
matrix:
CHANNEL: "master"
CHANNEL: "stable"
@@ -199,7 +180,7 @@
build_script:
- ./script/tool_runner.sh build-examples --web
drive_script:
- - ./script/tool_runner.sh drive-examples --web --exclude $PLUGINS_TO_EXCLUDE_INTEGRATION_TESTS
+ - ./script/tool_runner.sh drive-examples --web --exclude=script/configs/exclude_integration_web.yaml
# macOS tasks.
task:
@@ -221,10 +202,6 @@
- name: build-ipas+drive-examples
env:
PATH: $PATH:/usr/local/bin
- # in_app_purchase_ios is currently missing tests; see https://github.com/flutter/flutter/issues/81695
- # ios_platform_images is currently missing tests; see https://github.com/flutter/flutter/issues/82208
- # sensor hangs on CI.
- PLUGINS_TO_EXCLUDE_INTEGRATION_TESTS: "in_app_purchase_ios,ios_platform_images,sensors"
matrix:
PLUGIN_SHARDING: "--shardIndex 0 --shardCount 4"
PLUGIN_SHARDING: "--shardIndex 1 --shardCount 4"
@@ -247,7 +224,7 @@
# `drive-examples` contains integration tests, which changes the UI of the application.
# This UI change sometimes affects `xctest`.
# So we run `drive-examples` after `native-test`; changing the order will result ci failure.
- - ./script/tool_runner.sh drive-examples --ios --exclude $PLUGINS_TO_EXCLUDE_INTEGRATION_TESTS
+ - ./script/tool_runner.sh drive-examples --ios --exclude=script/configs/exclude_integration_ios.yaml
### macOS desktop tasks ###
- name: build_all_plugins_macos
env:
@@ -259,9 +236,6 @@
- ./script/build_all_plugins_app.sh macos
- name: build-macos+drive-examples
env:
- # conncectivity_macos is deprecated, so is not getting unit test backfill.
- # package_info is deprecated, so is not getting unit test backfill.
- PLUGINS_TO_EXCLUDE_MACOS_XCTESTS: "connectivity_macos,package_info"
matrix:
CHANNEL: "master"
CHANNEL: "stable"
@@ -272,6 +246,6 @@
xcode_analyze_script:
- ./script/tool_runner.sh xcode-analyze --macos
native_test_script:
- - ./script/tool_runner.sh native-test --macos --exclude $PLUGINS_TO_EXCLUDE_MACOS_XCTESTS
+ - ./script/tool_runner.sh native-test --macos --exclude=script/configs/exclude_native_macos.yaml
drive_script:
- ./script/tool_runner.sh drive-examples --macos
diff --git a/packages/e2e/analysis_options.yaml b/packages/e2e/analysis_options.yaml
deleted file mode 100644
index cda4f6e..0000000
--- a/packages/e2e/analysis_options.yaml
+++ /dev/null
@@ -1 +0,0 @@
-include: ../../analysis_options_legacy.yaml
diff --git a/script/configs/README.md b/script/configs/README.md
new file mode 100644
index 0000000..96423cf
--- /dev/null
+++ b/script/configs/README.md
@@ -0,0 +1,8 @@
+This folder contains configuration files that are passed to commands in place
+of plugin lists. They are primarily used by CI to opt specific packages out of
+tests, but can also useful when running multi-plugin tests locally.
+
+**Any entry added to a file in this directory should include a comment**.
+Skipping tests or checks for plugins is usually not something we want to do,
+so should the comment should either include an issue link to the issue tracking
+removing it or—much more rarely—explaining why it is a permanent exclusion.
diff --git a/script/configs/custom_analysis.yaml b/script/configs/custom_analysis.yaml
new file mode 100644
index 0000000..f6dc8e2
--- /dev/null
+++ b/script/configs/custom_analysis.yaml
@@ -0,0 +1,41 @@
+# Plugins that deliberately use their own analysis_options.yaml.
+#
+# This only exists to allow incrementally switching to the newer, stricter
+# analysis_options.yaml based on flutter/flutter, rather than the original
+# rules based on pedantic (now at analysis_options_legacy.yaml).
+#
+# DO NOT add new entries to the list, unless it is to push the legacy rules
+# from a top-level package into more specific packages in order to incrementally
+# migrate a federated plugin.
+#
+# TODO(ecosystem): Remove everything from this list. See:
+# https://github.com/flutter/flutter/issues/76229
+- camera
+- file_selector
+- flutter_plugin_android_lifecycle
+- google_maps_flutter
+- google_sign_in
+- image_picker
+- in_app_purchase
+- integration_test
+- ios_platform_images
+- local_auth
+- plugin_platform_interface
+- quick_actions
+- shared_preferences
+- url_launcher
+- video_player
+- webview_flutter
+
+# These plugins are deprecated in favor of the Community Plus versions, and
+# will be removed from the repo once the critical support window has passed,
+# so are not worth updating.
+- android_alarm_manager
+- android_intent
+- battery
+- connectivity
+- device_info
+- package_info
+- sensors
+- share
+- wifi_info_flutter
diff --git a/script/configs/exclude_integration_android.yaml b/script/configs/exclude_integration_android.yaml
new file mode 100644
index 0000000..9fc31ec
--- /dev/null
+++ b/script/configs/exclude_integration_android.yaml
@@ -0,0 +1,22 @@
+# Currently missing harness files: https://github.com/flutter/flutter/issues/86749)
+- camera/camera
+- google_sign_in/google_sign_in
+- in_app_purchase/in_app_purchase
+- in_app_purchase_android
+- quick_actions
+- shared_preferences/shared_preferences
+- url_launcher/url_launcher
+- video_player/video_player
+- webview_flutter
+
+# Deprecated; no plan to backfill the missing files
+- android_intent
+- connectivity/connectivity
+- device_info/device_info
+- sensors
+- share
+- wifi_info_flutter/wifi_info_flutter
+
+# No integration tests to run:
+- image_picker/image_picker
+- espresso
diff --git a/script/configs/exclude_integration_ios.yaml b/script/configs/exclude_integration_ios.yaml
new file mode 100644
index 0000000..e1ae6ad
--- /dev/null
+++ b/script/configs/exclude_integration_ios.yaml
@@ -0,0 +1,6 @@
+# Currently missing: https://github.com/flutter/flutter/issues/81695
+- in_app_purchase_ios
+# Currently missing: https://github.com/flutter/flutter/issues/82208
+- ios_platform_images
+# Hangs on CI. Deprecated, so there is no plan to fix it.
+- sensors
diff --git a/script/configs/exclude_integration_web.yaml b/script/configs/exclude_integration_web.yaml
new file mode 100644
index 0000000..99e2083
--- /dev/null
+++ b/script/configs/exclude_integration_web.yaml
@@ -0,0 +1,4 @@
+# Currently missing: https://github.com/flutter/flutter/issues/81982
+- shared_preferences_web
+# Currently missing: https://github.com/flutter/flutter/issues/82211
+- file_selector
diff --git a/script/configs/exclude_native_macos.yaml b/script/configs/exclude_native_macos.yaml
new file mode 100644
index 0000000..8a817a9
--- /dev/null
+++ b/script/configs/exclude_native_macos.yaml
@@ -0,0 +1,3 @@
+# Deprecated plugins that will not be getting unit test backfill.
+- connectivity_macos
+- package_info
diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md
index dc30c05..7d1eac0 100644
--- a/script/tool/CHANGELOG.md
+++ b/script/tool/CHANGELOG.md
@@ -1,5 +1,9 @@
## NEXT
+- `--exclude` and `--custom-analysis` now accept paths to YAML files that
+ contain lists of packages to exclude, in addition to just package names,
+ so that exclude lists can be maintained separately from scripts and CI
+ configuration.
- Added an `xctest` flag to select specific test targets, to allow running only
unit tests or integration tests.
- **Breaking change**: Split Xcode analysis out of `xctest` and into a new
diff --git a/script/tool/lib/src/analyze_command.dart b/script/tool/lib/src/analyze_command.dart
index e56b95d..4fd15f0 100644
--- a/script/tool/lib/src/analyze_command.dart
+++ b/script/tool/lib/src/analyze_command.dart
@@ -6,6 +6,7 @@
import 'package:file/file.dart';
import 'package:platform/platform.dart';
+import 'package:yaml/yaml.dart';
import 'common/core.dart';
import 'common/package_looping_command.dart';
@@ -23,7 +24,10 @@
}) : super(packagesDir, processRunner: processRunner, platform: platform) {
argParser.addMultiOption(_customAnalysisFlag,
help:
- 'Directories (comma separated) that are allowed to have their own analysis options.',
+ 'Directories (comma separated) that are allowed to have their own '
+ 'analysis options.\n\n'
+ 'Alternately, a list of one or more YAML files that contain a list '
+ 'of allowed directories.',
defaultsTo: <String>[]);
argParser.addOption(_analysisSdk,
valueHelp: 'dart-sdk',
@@ -37,6 +41,8 @@
late String _dartBinaryPath;
+ Set<String> _allowedCustomAnalysisDirectories = const <String>{};
+
@override
final String name = 'analyze';
@@ -56,7 +62,7 @@
continue;
}
- final bool allowed = (getStringListArg(_customAnalysisFlag)).any(
+ final bool allowed = _allowedCustomAnalysisDirectories.any(
(String directory) =>
directory.isNotEmpty &&
path.isWithin(
@@ -107,6 +113,17 @@
throw ToolExit(_exitPackagesGetFailed);
}
+ _allowedCustomAnalysisDirectories =
+ getStringListArg(_customAnalysisFlag).expand<String>((String item) {
+ if (item.endsWith('.yaml')) {
+ final File file = packagesDir.fileSystem.file(item);
+ return (loadYaml(file.readAsStringSync()) as YamlList)
+ .toList()
+ .cast<String>();
+ }
+ return <String>[item];
+ }).toSet();
+
// Use the Dart SDK override if one was passed in.
final String? dartSdk = argResults![_analysisSdk] as String?;
_dartBinaryPath =
diff --git a/script/tool/lib/src/common/plugin_command.dart b/script/tool/lib/src/common/plugin_command.dart
index ecdcb05..7781eee 100644
--- a/script/tool/lib/src/common/plugin_command.dart
+++ b/script/tool/lib/src/common/plugin_command.dart
@@ -9,6 +9,7 @@
import 'package:git/git.dart';
import 'package:path/path.dart' as p;
import 'package:platform/platform.dart';
+import 'package:yaml/yaml.dart';
import 'core.dart';
import 'git_version_finder.dart';
@@ -48,7 +49,9 @@
argParser.addMultiOption(
_excludeArg,
abbr: 'e',
- help: 'Exclude packages from this command.',
+ help: 'A list of packages to exclude from from this command.\n\n'
+ 'Alternately, a list of one or more YAML files that contain a list '
+ 'of packages to exclude.',
defaultsTo: <String>[],
);
argParser.addFlag(_runOnChangedPackagesArg,
@@ -214,8 +217,18 @@
/// of packages in the flutter/packages repository.
Stream<Directory> _getAllPlugins() async* {
Set<String> plugins = Set<String>.from(getStringListArg(_packagesArg));
+
final Set<String> excludedPlugins =
- Set<String>.from(getStringListArg(_excludeArg));
+ getStringListArg(_excludeArg).expand<String>((String item) {
+ if (item.endsWith('.yaml')) {
+ final File file = packagesDir.fileSystem.file(item);
+ return (loadYaml(file.readAsStringSync()) as YamlList)
+ .toList()
+ .cast<String>();
+ }
+ return <String>[item];
+ }).toSet();
+
final bool runOnChangedPackages = getBoolArg(_runOnChangedPackagesArg);
if (plugins.isEmpty &&
runOnChangedPackages &&
diff --git a/script/tool/test/analyze_command_test.dart b/script/tool/test/analyze_command_test.dart
index 69a2c4f..9dc8b6a 100644
--- a/script/tool/test/analyze_command_test.dart
+++ b/script/tool/test/analyze_command_test.dart
@@ -176,6 +176,25 @@
]));
});
+ test('takes an allow config file', () async {
+ final Directory pluginDir = createFakePlugin('foo', packagesDir,
+ extraFiles: <String>['analysis_options.yaml']);
+ final File allowFile = packagesDir.childFile('custom.yaml');
+ allowFile.writeAsStringSync('- foo');
+
+ await runCapturingPrint(
+ runner, <String>['analyze', '--custom-analysis', allowFile.path]);
+
+ expect(
+ processRunner.recordedCalls,
+ orderedEquals(<ProcessCall>[
+ ProcessCall(
+ 'flutter', const <String>['packages', 'get'], pluginDir.path),
+ ProcessCall('dart', const <String>['analyze', '--fatal-infos'],
+ pluginDir.path),
+ ]));
+ });
+
// See: https://github.com/flutter/flutter/issues/78994
test('takes an empty allow list', () async {
createFakePlugin('foo', packagesDir,
diff --git a/script/tool/test/common/plugin_command_test.dart b/script/tool/test/common/plugin_command_test.dart
index fdab961..7f67acf 100644
--- a/script/tool/test/common/plugin_command_test.dart
+++ b/script/tool/test/common/plugin_command_test.dart
@@ -172,6 +172,19 @@
expect(plugins, unorderedEquals(<String>[plugin2.path]));
});
+ test('exclude accepts config files', () async {
+ createFakePlugin('plugin1', packagesDir);
+ final File configFile = packagesDir.childFile('exclude.yaml');
+ configFile.writeAsStringSync('- plugin1');
+
+ await runCapturingPrint(runner, <String>[
+ 'sample',
+ '--packages=plugin1',
+ '--exclude=${configFile.path}'
+ ]);
+ expect(plugins, unorderedEquals(<String>[]));
+ });
+
group('test run-on-changed-packages', () {
test('all plugins should be tested if there are no changes.', () async {
final Directory plugin1 = createFakePlugin('plugin1', packagesDir);
diff --git a/script/tool_runner.sh b/script/tool_runner.sh
index d16e940..11a54ce 100755
--- a/script/tool_runner.sh
+++ b/script/tool_runner.sh
@@ -10,58 +10,7 @@
source "$SCRIPT_DIR/common.sh"
-# Plugins that are excluded from this task.
-ALL_EXCLUDED=("")
-
-# Plugins that deliberately use their own analysis_options.yaml.
-#
-# This list should only be deleted from, never added to. This only exists
-# because we adopted stricter analysis rules recently and needed to exclude
-# already failing packages to start linting the repo as a whole.
-#
-# Finding all: `find packages -name analysis_options.yaml | sort | cut -d/ -f2`
-#
-# TODO(ecosystem): Remove everything from this list. https://github.com/flutter/flutter/issues/76229
-CUSTOM_ANALYSIS_PLUGINS=(
- android_alarm_manager
- android_intent
- battery
- camera
- connectivity
- cross_file
- device_info
- e2e
- espresso
- file_selector
- flutter_plugin_android_lifecycle
- google_maps_flutter
- google_sign_in
- image_picker
- in_app_purchase
- integration_test
- ios_platform_images
- local_auth
- package_info
- plugin_platform_interface
- quick_actions
- sensors
- share
- shared_preferences
- url_launcher
- video_player
- webview_flutter
- wifi_info_flutter
-)
-
-# Comma-separated string of the list above
-readonly CUSTOM_FLAG=$(IFS=, ; echo "${CUSTOM_ANALYSIS_PLUGINS[*]}")
-# Set some default actions if run without arguments.
ACTIONS=("$@")
-if [[ "${#ACTIONS[@]}" == 0 ]]; then
- ACTIONS=("analyze" "--custom-analysis" "$CUSTOM_FLAG" "test" "java-test")
-elif [[ "${ACTIONS[0]}" == "analyze" ]]; then
- ACTIONS=("${ACTIONS[@]}" "--custom-analysis" "$CUSTOM_FLAG")
-fi
BRANCH_NAME="${BRANCH_NAME:-"$(git rev-parse --abbrev-ref HEAD)"}"
@@ -71,8 +20,8 @@
if [[ "${BRANCH_NAME}" == "master" ]]; then
echo "Running for all packages"
- (cd "$REPO_DIR" && plugin_tools "${ACTIONS[@]}" --exclude="$ALL_EXCLUDED" ${PLUGIN_SHARDING[@]})
+ (cd "$REPO_DIR" && plugin_tools "${ACTIONS[@]}" ${PLUGIN_SHARDING[@]})
else
echo running "${ACTIONS[@]}"
- (cd "$REPO_DIR" && plugin_tools "${ACTIONS[@]}" --run-on-changed-packages --exclude="$ALL_EXCLUDED" ${PLUGIN_SHARDING[@]})
+ (cd "$REPO_DIR" && plugin_tools "${ACTIONS[@]}" --run-on-changed-packages ${PLUGIN_SHARDING[@]})
fi
