Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[flutter_plugin_tools] Support YAML exception lists #4183

Merged
merged 11 commits into from
Jul 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 5 additions & 31 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ task:
- 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:
Expand Down Expand Up @@ -137,22 +137,6 @@ task:
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.
Expand All @@ -177,16 +161,13 @@ task:
- 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"
Expand All @@ -199,7 +180,7 @@ task:
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:
Expand All @@ -221,10 +202,6 @@ task:
- 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"
Expand All @@ -247,7 +224,7 @@ task:
# `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:
Expand All @@ -259,9 +236,6 @@ task:
- ./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"
Expand All @@ -272,6 +246,6 @@ task:
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
1 change: 0 additions & 1 deletion packages/e2e/analysis_options.yaml

This file was deleted.

8 changes: 8 additions & 0 deletions script/configs/README.md
Original file line number Diff line number Diff line change
@@ -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.
41 changes: 41 additions & 0 deletions script/configs/custom_analysis.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Plugins that deliberately use their own analysis_options.yaml.
#
# This only exists to allow incrementally switching to the newer, stricter
stuartmorgan marked this conversation as resolved.
Show resolved Hide resolved
# 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
22 changes: 22 additions & 0 deletions script/configs/exclude_integration_android.yaml
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions script/configs/exclude_integration_ios.yaml
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions script/configs/exclude_integration_web.yaml
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions script/configs/exclude_native_macos.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Deprecated plugins that will not be getting unit test backfill.
- connectivity_macos
- package_info
4 changes: 4 additions & 0 deletions script/tool/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
21 changes: 19 additions & 2 deletions script/tool/lib/src/analyze_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:async';

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';
Expand All @@ -23,7 +24,10 @@ class AnalyzeCommand extends PackageLoopingCommand {
}) : 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',
Expand All @@ -37,6 +41,8 @@ class AnalyzeCommand extends PackageLoopingCommand {

late String _dartBinaryPath;

Set<String> _allowedCustomAnalysisDirectories = const <String>{};

@override
final String name = 'analyze';

Expand All @@ -56,7 +62,7 @@ class AnalyzeCommand extends PackageLoopingCommand {
continue;
}

final bool allowed = (getStringListArg(_customAnalysisFlag)).any(
final bool allowed = _allowedCustomAnalysisDirectories.any(
(String directory) =>
directory.isNotEmpty &&
path.isWithin(
Expand Down Expand Up @@ -107,6 +113,17 @@ class AnalyzeCommand extends PackageLoopingCommand {
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 =
Expand Down
17 changes: 15 additions & 2 deletions script/tool/lib/src/common/plugin_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'package:file/file.dart';
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';
Expand Down Expand Up @@ -48,7 +49,9 @@ abstract class PluginCommand extends Command<void> {
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,
Expand Down Expand Up @@ -214,8 +217,18 @@ abstract class PluginCommand extends Command<void> {
/// 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 &&
Expand Down
19 changes: 19 additions & 0 deletions script/tool/test/analyze_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,25 @@ void main() {
]));
});

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,
Expand Down
13 changes: 13 additions & 0 deletions script/tool/test/common/plugin_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,19 @@ void main() {
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);
Expand Down
Loading