From aa071945b7953816a99e6e7c46bdb1a56b518bc5 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 14 Jun 2022 13:41:10 -0400 Subject: [PATCH 1/5] Add new command --- script/tool/CHANGELOG.md | 1 + .../lib/src/dependabot_check_command.dart | 113 ++++++++++++++ script/tool/lib/src/main.dart | 2 + .../test/dependabot_check_command_test.dart | 141 ++++++++++++++++++ 4 files changed, 257 insertions(+) create mode 100644 script/tool/lib/src/dependabot_check_command.dart create mode 100644 script/tool/test/dependabot_check_command_test.dart diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 36d8d23eb753..07cafee7f8b6 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -3,6 +3,7 @@ - Supports empty custom analysis allow list files. - `drive-examples` now validates files to ensure that they don't accidentally use `test(...)`. +- Adds a new `dependabot-check` command to ensure complete Dependabot coverage. ## 0.8.6 diff --git a/script/tool/lib/src/dependabot_check_command.dart b/script/tool/lib/src/dependabot_check_command.dart new file mode 100644 index 000000000000..5fc9d10821ca --- /dev/null +++ b/script/tool/lib/src/dependabot_check_command.dart @@ -0,0 +1,113 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; + +import 'package:file/file.dart'; +import 'package:git/git.dart'; +import 'package:yaml/yaml.dart'; + +import 'common/core.dart'; +import 'common/package_looping_command.dart'; +import 'common/repository_package.dart'; + +/// A command to verify Dependabot configuration coverage of packages. +class DependabotCheckCommand extends PackageLoopingCommand { + /// Creates Dependabot check command instance. + DependabotCheckCommand(Directory packagesDir, {GitDir? gitDir}) + : super(packagesDir, gitDir: gitDir) { + argParser.addOption(_configPathFlag, + help: 'Path to the Dependabot configuration file', + defaultsTo: '.github/dependabot.yml'); + } + + static const String _configPathFlag = 'config'; + + late Directory _repoRoot; + + // The set of directories covered by "gradle" entries in the config. + Set _gradleDirs = const {}; + + @override + final String name = 'dependabot-check'; + + @override + final String description = + 'Checks that all packages have Dependabot coverage.'; + + @override + final PackageLoopingType packageLoopingType = + PackageLoopingType.includeAllSubpackages; + + @override + final bool hasLongOutput = false; + + @override + Future initializeRun() async { + _repoRoot = packagesDir.fileSystem.directory((await gitDir).path); + + final YamlMap config = loadYaml(_repoRoot + .childFile(getStringArg(_configPathFlag)) + .readAsStringSync()) as YamlMap; + final dynamic entries = config['updates']; + if (entries is! YamlList) { + return; + } + + const String typeKey = 'package-ecosystem'; + const String dirKey = 'directory'; + _gradleDirs = entries + .where((dynamic entry) => entry[typeKey] == 'gradle') + .map((dynamic entry) => (entry as YamlMap)[dirKey] as String) + .toSet(); + } + + @override + Future runForPackage(RepositoryPackage package) async { + bool skipped = true; + final List errors = []; + + final RunState gradleState = _validateGradle(package); + skipped = skipped && gradleState == RunState.skipped; + if (gradleState == RunState.failed) { + printError('${indentation}Missing Gradle coverage.'); + errors.add('Missing Gradle coverage'); + } + + // TODO(stuartmorgan): Add other ecosystem checks here as more are enabled. + + if (skipped) { + return PackageResult.skip('No supported package ecosystems'); + } + return errors.isEmpty + ? PackageResult.success() + : PackageResult.fail(errors); + } + + /// Returns the state for the Gradle ecosystem coverage of [package]: + /// - succeeded if it includes gradle and is covered. + /// - failed if it includes gradle and is not covered. + /// - skipped if it doesn't include gradle. + RunState _validateGradle(RepositoryPackage package) { + final Directory androidDir = + package.platformDirectory(FlutterPlatform.android); + final Directory appDir = androidDir.childDirectory('app'); + if (appDir.existsSync()) { + // It's an app, so only check for the app directory to be covered. + final String dependabotPath = + '/${getRelativePosixPath(appDir, from: _repoRoot)}'; + return _gradleDirs.contains(dependabotPath) + ? RunState.succeeded + : RunState.failed; + } else if (androidDir.existsSync()) { + // It's a library, so only check for the android directory to be covered. + final String dependabotPath = + '/${getRelativePosixPath(androidDir, from: _repoRoot)}'; + return _gradleDirs.contains(dependabotPath) + ? RunState.succeeded + : RunState.failed; + } + return RunState.skipped; + } +} diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index 739aef56878d..966e7b6be56a 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -7,6 +7,7 @@ import 'dart:io' as io; import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:file/local.dart'; +import 'package:flutter_plugin_tools/src/dependabot_check_command.dart'; import 'analyze_command.dart'; import 'build_examples_command.dart'; @@ -55,6 +56,7 @@ void main(List args) { ..addCommand(BuildExamplesCommand(packagesDir)) ..addCommand(CreateAllPluginsAppCommand(packagesDir)) ..addCommand(CustomTestCommand(packagesDir)) + ..addCommand(DependabotCheckCommand(packagesDir)) ..addCommand(DriveExamplesCommand(packagesDir)) ..addCommand(FederationSafetyCheckCommand(packagesDir)) ..addCommand(FirebaseTestLabCommand(packagesDir)) diff --git a/script/tool/test/dependabot_check_command_test.dart b/script/tool/test/dependabot_check_command_test.dart new file mode 100644 index 000000000000..a4c8693b2c21 --- /dev/null +++ b/script/tool/test/dependabot_check_command_test.dart @@ -0,0 +1,141 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +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/dependabot_check_command.dart'; +import 'package:mockito/mockito.dart'; +import 'package:test/test.dart'; + +import 'common/plugin_command_test.mocks.dart'; +import 'util.dart'; + +void main() { + late CommandRunner runner; + late FileSystem fileSystem; + late Directory root; + late Directory packagesDir; + + setUp(() { + fileSystem = MemoryFileSystem(); + root = fileSystem.currentDirectory; + packagesDir = root.childDirectory('packages'); + + final MockGitDir gitDir = MockGitDir(); + when(gitDir.path).thenReturn(root.path); + + final DependabotCheckCommand command = DependabotCheckCommand( + packagesDir, + gitDir: gitDir, + ); + runner = CommandRunner( + 'dependabot_test', 'Test for $DependabotCheckCommand'); + runner.addCommand(command); + }); + + void _setDependabotCoverage({ + Iterable gradleDirs = const [], + }) { + final Iterable gradleEntries = + gradleDirs.map((String directory) => ''' + - package-ecosystem: "gradle" + directory: "/$directory" + schedule: + interval: "daily" +'''); + final File configFile = + root.childDirectory('.github').childFile('dependabot.yml'); + configFile.createSync(recursive: true); + configFile.writeAsStringSync(''' +version: 2 +updates: +${gradleEntries.join('\n')} +'''); + } + + test('skips with no supported ecosystems', () async { + _setDependabotCoverage(); + createFakePackage('a_package', packagesDir); + + final List output = + await runCapturingPrint(runner, ['dependabot-check']); + + expect( + output, + containsAllInOrder([ + contains('SKIPPING: No supported package ecosystems'), + ])); + }); + + test('fails for app missing Gradle coverage', () async { + _setDependabotCoverage(); + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + package.directory + .childDirectory('example') + .childDirectory('android') + .childDirectory('app') + .createSync(recursive: true); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['dependabot-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Missing Gradle coverage.'), + contains('a_package/example:\n' + ' Missing Gradle coverage') + ])); + }); + + test('fails for plugin missing Gradle coverage', () async { + _setDependabotCoverage(); + final RepositoryPackage plugin = createFakePlugin('a_plugin', packagesDir); + plugin.directory.childDirectory('android').createSync(recursive: true); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['dependabot-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Missing Gradle coverage.'), + contains('a_plugin:\n' + ' Missing Gradle coverage') + ])); + }); + + test('passes for correct Gradle coverage', () async { + _setDependabotCoverage(gradleDirs: [ + 'packages/a_plugin/android', + 'packages/a_plugin/example/android/app', + ]); + final RepositoryPackage plugin = createFakePlugin('a_plugin', packagesDir); + // Test the plugin. + plugin.directory.childDirectory('android').createSync(recursive: true); + // And its example app. + plugin.directory + .childDirectory('example') + .childDirectory('android') + .childDirectory('app') + .createSync(recursive: true); + + final List output = + await runCapturingPrint(runner, ['dependabot-check']); + + expect(output, + containsAllInOrder([contains('Ran for 2 package(s)')])); + }); +} From 3a2039cd4df95e268fbfc44072911979030cca5d Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 14 Jun 2022 13:42:16 -0400 Subject: [PATCH 2/5] Enable check in CI --- .cirrus.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.cirrus.yml b/.cirrus.yml index 70f9ddbfb251..6ea4680f6dec 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -113,6 +113,7 @@ task: # run with --require-excerpts and no exclusions. - ./script/tool_runner.sh readme-check --require-excerpts --exclude=script/configs/temp_exclude_excerpt.yaml license_script: dart $PLUGIN_TOOL license-check + dependabot_script: dart $PLUGIN_TOOL dependabot-check - name: federated_safety # This check is only meaningful for PRs, as it validates changes # rather than state. From bc9d4b69ab8d5d1bbfd1ec751d3c6c333b3c8789 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 14 Jun 2022 13:46:14 -0400 Subject: [PATCH 3/5] Fix violations --- .github/dependabot.yml | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 02bfe34d9608..1c2837f3f181 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,7 +1,15 @@ version: 2 updates: - package-ecosystem: "gradle" - directory: "/packages/camera/camera/android" + directory: "/packages/camera/camera_android/android" + commit-message: + prefix: "[camera]" + schedule: + interval: "daily" + open-pull-requests-limit: 10 + + - package-ecosystem: "gradle" + directory: "/packages/camera/camera_android/example/android/app" commit-message: prefix: "[camera]" schedule: @@ -216,6 +224,14 @@ updates: interval: "daily" open-pull-requests-limit: 10 + - package-ecosystem: "gradle" + directory: "/packages/shared_preferences/shared_preferences_android/android" + commit-message: + prefix: "[shared_pref]" + schedule: + interval: "daily" + open-pull-requests-limit: 10 + - package-ecosystem: "gradle" directory: "/packages/shared_preferences/shared_preferences_android/example/android/app" commit-message: @@ -248,6 +264,14 @@ updates: interval: "daily" open-pull-requests-limit: 10 + - package-ecosystem: "gradle" + directory: "/packages/video_player/video_player/example/android/app" + commit-message: + prefix: "[video_player]" + schedule: + interval: "daily" + open-pull-requests-limit: 10 + - package-ecosystem: "gradle" directory: "/packages/video_player/video_player_android/android" commit-message: @@ -281,13 +305,13 @@ updates: open-pull-requests-limit: 10 - package-ecosystem: "gradle" - directory: "/packages/webview_flutter/webview_flutter_android/example/android" + directory: "/packages/webview_flutter/webview_flutter_android/example/android/app" commit-message: prefix: "[webview]" schedule: interval: "daily" open-pull-requests-limit: 10 - + - package-ecosystem: "github-actions" directory: "/" commit-message: From a2d137299982ef9b65da89c2d06551805442a8ce Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Wed, 15 Jun 2022 09:51:29 -0400 Subject: [PATCH 4/5] Switch new entries to weekly per #5972 --- .github/dependabot.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 1c2837f3f181..e71182c6f984 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -5,7 +5,7 @@ updates: commit-message: prefix: "[camera]" schedule: - interval: "daily" + interval: "weekly" open-pull-requests-limit: 10 - package-ecosystem: "gradle" @@ -229,7 +229,7 @@ updates: commit-message: prefix: "[shared_pref]" schedule: - interval: "daily" + interval: "weekly" open-pull-requests-limit: 10 - package-ecosystem: "gradle" @@ -269,7 +269,7 @@ updates: commit-message: prefix: "[video_player]" schedule: - interval: "daily" + interval: "weekly" open-pull-requests-limit: 10 - package-ecosystem: "gradle" From ddd106a308fe3b312de1c8ecca8b38d80c119fac Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 22 Jun 2022 13:26:05 -0400 Subject: [PATCH 5/5] Review comments --- script/tool/lib/src/dependabot_check_command.dart | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/script/tool/lib/src/dependabot_check_command.dart b/script/tool/lib/src/dependabot_check_command.dart index 5fc9d10821ca..5aa762e916e5 100644 --- a/script/tool/lib/src/dependabot_check_command.dart +++ b/script/tool/lib/src/dependabot_check_command.dart @@ -68,7 +68,7 @@ class DependabotCheckCommand extends PackageLoopingCommand { bool skipped = true; final List errors = []; - final RunState gradleState = _validateGradle(package); + final RunState gradleState = _validateDependabotGradleCoverage(package); skipped = skipped && gradleState == RunState.skipped; if (gradleState == RunState.failed) { printError('${indentation}Missing Gradle coverage.'); @@ -85,11 +85,12 @@ class DependabotCheckCommand extends PackageLoopingCommand { : PackageResult.fail(errors); } - /// Returns the state for the Gradle ecosystem coverage of [package]: + /// Returns the state for the Dependabot coverage of the Gradle ecosystem for + /// [package]: /// - succeeded if it includes gradle and is covered. /// - failed if it includes gradle and is not covered. /// - skipped if it doesn't include gradle. - RunState _validateGradle(RepositoryPackage package) { + RunState _validateDependabotGradleCoverage(RepositoryPackage package) { final Directory androidDir = package.platformDirectory(FlutterPlatform.android); final Directory appDir = androidDir.childDirectory('app');