From 4a8595961a6384c4a1262d5e0529ee77a81ad411 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 29 Apr 2026 16:07:14 -0400 Subject: [PATCH 01/18] Rename repo-package-info to create the start of the new command --- AGENTS.md | 2 +- script/tool/lib/src/main.dart | 14 +- ...eck_command.dart => validate_command.dart} | 24 +- .../repo_package_info_check_command_test.dart | 952 ----------------- script/tool/test/validate_command_test.dart | 960 ++++++++++++++++++ 5 files changed, 980 insertions(+), 972 deletions(-) rename script/tool/lib/src/{repo_package_info_check_command.dart => validate_command.dart} (73%) delete mode 100644 script/tool/test/repo_package_info_check_command_test.dart create mode 100644 script/tool/test/validate_command_test.dart diff --git a/AGENTS.md b/AGENTS.md index 92d0f90a74b0..52580b5da637 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,7 +94,7 @@ dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart update-dependency dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart readme-check --packages dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart version-check --packages dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart license-check - dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart repo-package-info-check + dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart validate ``` ### Specialized Workflows diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index fedffe01538b..79676439fea3 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -33,11 +33,11 @@ import 'publish_command.dart'; import 'pubspec_check_command.dart'; import 'readme_check_command.dart'; import 'remove_dev_dependencies_command.dart'; -import 'repo_package_info_check_command.dart'; import 'update_dependency_command.dart'; import 'update_excerpts_command.dart'; import 'update_min_sdk_command.dart'; import 'update_release_info_command.dart'; +import 'validate_command.dart'; import 'version_check_command.dart'; void main(List args) { @@ -64,9 +64,11 @@ void main(List args) { 'Productivity utils for hosting multiple plugins within one repository.', ) ..addCommand(AnalyzeCommand(packagesDir)) + ..addCommand(BranchesForBatchReleaseCommand(packagesDir)) ..addCommand(BuildExamplesCommand(packagesDir)) ..addCommand(CreateAllPackagesAppCommand(packagesDir)) ..addCommand(CustomTestCommand(packagesDir)) + ..addCommand(DartTestCommand(packagesDir)) ..addCommand(DependabotCheckCommand(packagesDir)) ..addCommand(DriveExamplesCommand(packagesDir)) ..addCommand(FederationSafetyCheckCommand(packagesDir)) @@ -76,23 +78,21 @@ void main(List args) { ..addCommand(FormatCommand(packagesDir)) ..addCommand(GradleCheckCommand(packagesDir)) ..addCommand(LicenseCheckCommand(packagesDir)) - ..addCommand(PodspecCheckCommand(packagesDir)) ..addCommand(ListCommand(packagesDir)) - ..addCommand(NativeTestCommand(packagesDir)) ..addCommand(MakeDepsPathBasedCommand(packagesDir)) + ..addCommand(NativeTestCommand(packagesDir)) + ..addCommand(PodspecCheckCommand(packagesDir)) ..addCommand(PublishCheckCommand(packagesDir)) ..addCommand(PublishCommand(packagesDir)) ..addCommand(PubspecCheckCommand(packagesDir)) ..addCommand(ReadmeCheckCommand(packagesDir)) ..addCommand(RemoveDevDependenciesCommand(packagesDir)) - ..addCommand(RepoPackageInfoCheckCommand(packagesDir)) - ..addCommand(DartTestCommand(packagesDir)) ..addCommand(UpdateDependencyCommand(packagesDir)) ..addCommand(UpdateExcerptsCommand(packagesDir)) ..addCommand(UpdateMinSdkCommand(packagesDir)) ..addCommand(UpdateReleaseInfoCommand(packagesDir)) - ..addCommand(VersionCheckCommand(packagesDir)) - ..addCommand(BranchesForBatchReleaseCommand(packagesDir)); + ..addCommand(ValidateCommand(packagesDir)) + ..addCommand(VersionCheckCommand(packagesDir)); commandRunner.run(args).catchError((Object e) { final toolExit = e as ToolExit; diff --git a/script/tool/lib/src/repo_package_info_check_command.dart b/script/tool/lib/src/validate_command.dart similarity index 73% rename from script/tool/lib/src/repo_package_info_check_command.dart rename to script/tool/lib/src/validate_command.dart index 21d6b02ad590..d112eefc98c5 100644 --- a/script/tool/lib/src/repo_package_info_check_command.dart +++ b/script/tool/lib/src/validate_command.dart @@ -8,11 +8,15 @@ import 'common/package_looping_command.dart'; import 'common/repository_package.dart'; import 'validators/repo_info_validator.dart'; -/// A command to verify repository-level metadata about packages, such as -/// repo README and auto-label entries. -class RepoPackageInfoCheckCommand extends PackageLoopingCommand { +/// A command to validate that packages follow various team conventions, +/// guidelines, and best practices. +/// +/// This includes: +/// - repository-level metadata about packages, such as repo README and +/// auto-label entries. +class ValidateCommand extends PackageLoopingCommand { /// Creates Dependabot check command instance. - RepoPackageInfoCheckCommand(super.packagesDir, {super.gitDir}); + ValidateCommand(super.packagesDir, {super.gitDir}); late Directory _repoRoot; @@ -24,14 +28,10 @@ class RepoPackageInfoCheckCommand extends PackageLoopingCommand { final Set _autoLabeledPackages = {}; @override - final String name = 'repo-package-info-check'; + final String name = 'validate'; @override - List get aliases => ['check-repo-package-info']; - - @override - final String description = - 'Checks that all packages are listed correctly in repo metadata.'; + final String description = 'Checks that packages follow team guidelines.'; @override final bool hasLongOutput = false; @@ -40,7 +40,7 @@ class RepoPackageInfoCheckCommand extends PackageLoopingCommand { Future initializeRun() async { _repoRoot = packagesDir.fileSystem.directory((await gitDir).path); - // Extract all of the README.md table entries. + // Extract all of the repo-level README.md table entries. _readmeTableEntries.addAll( RepoInfoValidator.loadReadmeTableEntries( repoRoot: _repoRoot, @@ -48,7 +48,7 @@ class RepoPackageInfoCheckCommand extends PackageLoopingCommand { thirdPartyPackagesDir: thirdPartyPackagesDir, ), ); - // Extract all of the lebeler.yml package entries. + // Extract all of the labeler.yml package entries. _autoLabeledPackages.addAll( RepoInfoValidator.loadAutoLabeledPackages(repoRoot: _repoRoot), ); diff --git a/script/tool/test/repo_package_info_check_command_test.dart b/script/tool/test/repo_package_info_check_command_test.dart deleted file mode 100644 index 0395375d4280..000000000000 --- a/script/tool/test/repo_package_info_check_command_test.dart +++ /dev/null @@ -1,952 +0,0 @@ -// Copyright 2013 The Flutter Authors -// 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:flutter_plugin_tools/src/common/core.dart'; -import 'package:flutter_plugin_tools/src/repo_package_info_check_command.dart'; -import 'package:git/git.dart'; -import 'package:test/test.dart'; - -import 'mocks.dart'; -import 'util.dart'; - -void main() { - late CommandRunner runner; - late Directory root; - late Directory packagesDir; - late RecordingProcessRunner gitProcessRunner; - - setUp(() { - final GitDir gitDir; - (:packagesDir, processRunner: _, :gitProcessRunner, :gitDir) = - configureBaseCommandMocks(); - root = packagesDir.fileSystem.currentDirectory; - - final command = RepoPackageInfoCheckCommand(packagesDir, gitDir: gitDir); - runner = CommandRunner( - 'dependabot_test', - 'Test for $RepoPackageInfoCheckCommand', - ); - runner.addCommand(command); - - // Default to failing these checks so that tests of non-batch-release packages - // (the default) don't fail due to "unexpected" branches/labels being found. - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [FakeProcessInfo(MockProcess(exitCode: 1))]; - }); - - String readmeTableHeader() { - return ''' -| Package | Pub | Points | Popularity | Issues | Pull requests | -|---------|-----|--------|------------|--------|---------------| -'''; - } - - String readmeTableEntry(String packageName) { - final String encodedTag = Uri.encodeComponent('p: $packageName'); - return '| [$packageName](./packages/$packageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - } - - void writeAutoLabelerYaml(List packages) { - final File labelerYaml = root - .childDirectory('.github') - .childFile('labeler.yml'); - labelerYaml.createSync(recursive: true); - labelerYaml.writeAsStringSync( - packages - .map((RepositoryPackage p) { - final bool isThirdParty = p.path.contains('third_party/'); - return ''' --p: ${p.directory.basename} - - changed-files: - - any-glob-to-any-file: - - ${isThirdParty ? 'third_party/' : ''}packages/${p.directory.basename}/**/* -'''; - }) - .join('\n\n'), - ); - } - - test('passes for correct coverage', () async { - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml(packages); - - final List output = await runCapturingPrint(runner, [ - 'repo-package-info-check', - ]); - - expect( - output, - containsAllInOrder([contains('Ran for 1 package(s)')]), - ); - }); - - test( - 'passes for federated plugins with only app-facing package listed', - () async { - const pluginName = 'foo'; - final Directory pluginDir = packagesDir.childDirectory(pluginName); - final packages = [ - createFakePlugin(pluginName, pluginDir), - createFakePlugin('${pluginName}_platform_interface', pluginDir), - createFakePlugin('${pluginName}_android', pluginDir), - createFakePlugin('${pluginName}_ios', pluginDir), - ]; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry(pluginName)} -'''); - writeAutoLabelerYaml([packages.first]); - writeAutoLabelerYaml([packages.first]); - - // 4 packages * 2 checks (git, gh) = 8 calls. - // Default mocks in setUp cover 1 call each. We need 3 more each. - gitProcessRunner.mockProcessesForExecutable['git-ls-remote']! - .addAll([ - FakeProcessInfo(MockProcess(exitCode: 1)), - FakeProcessInfo(MockProcess(exitCode: 1)), - FakeProcessInfo(MockProcess(exitCode: 1)), - ]); - - final List output = await runCapturingPrint(runner, [ - 'repo-package-info-check', - ]); - - expect( - output, - containsAllInOrder([contains('Ran for 4 package(s)')]), - ); - }, - ); - - test('fails for unexpected README table entry', () async { - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('another_package')} -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains('Unknown package "another_package" in root README.md table'), - ]), - ); - }); - - test('fails for missing README table entry', () async { - final packages = [ - createFakePackage('a_package', packagesDir), - createFakePackage('another_package', packagesDir), - ]; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('another_package')} -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains('Missing repo root README.md table entry'), - contains( - 'a_package:\n' - ' Missing repo root README.md table entry', - ), - ]), - ); - }); - - test('fails for unexpected format in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$packageName/) | ' - 'Some random text | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains('Invalid repo root README.md table entry: "Some random text"'), - contains( - 'a_package:\n' - ' Invalid root README.md table entry', - ), - ]), - ); - }); - - test('fails for incorrect source link in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - const incorrectPackageName = 'a_pakage'; - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$incorrectPackageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Incorrect link in root README.md table: "./packages/$incorrectPackageName/"', - ), - contains( - 'a_package:\n' - ' Incorrect link in root README.md table', - ), - ]), - ); - }); - - test('fails for incorrect packages/* link in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - const incorrectPackageName = 'a_pakage'; - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$packageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$incorrectPackageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Incorrect link in root README.md table: "https://pub.dev/packages/$incorrectPackageName/score"', - ), - contains( - 'a_package:\n' - ' Incorrect link in root README.md table', - ), - ]), - ); - }); - - test('fails for incorrect labels/* link in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - final String incorrectTag = Uri.encodeComponent('p: a_pakage'); - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$packageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$incorrectTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Incorrect link in root README.md table: "https://github.com/flutter/flutter/labels/$incorrectTag"', - ), - contains( - 'a_package:\n' - ' Incorrect link in root README.md table', - ), - ]), - ); - }); - - test('fails for incorrect packages/* anchor in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - const incorrectPackageName = 'a_pakage'; - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$packageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$incorrectPackageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Incorrect anchor in root README.md table: "![pub points](https://img.shields.io/pub/points/$incorrectPackageName)"', - ), - contains( - 'a_package:\n' - ' Incorrect anchor in root README.md table', - ), - ]), - ); - }); - - test('fails for incorrect tag query anchor in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - final String incorrectTag = Uri.encodeComponent('p: a_pakage'); - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$packageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$incorrectTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Incorrect anchor in root README.md table: "![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$incorrectTag?label=)', - ), - contains( - 'a_package:\n' - ' Incorrect anchor in root README.md table', - ), - ]), - ); - }); - - test('fails for missing auto-labeler entry', () async { - const packageName = 'a_package'; - createFakePackage('a_package', packagesDir); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry(packageName)} -'''); - writeAutoLabelerYaml([]); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains('Missing a rule in .github/labeler.yml.'), - contains( - 'a_package:\n' - ' Missing auto-labeler entry', - ), - ]), - ); - }); - - group('ci_config check', () { - test('control test', () async { - final RepositoryPackage package = createFakePackage( - 'a_package', - packagesDir, - ); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml([package]); - - package.ciConfigFile.writeAsStringSync(''' -release: - batch: false - '''); - - final List output = await runCapturingPrint(runner, [ - 'repo-package-info-check', - ]); - - expect(output, containsAll([contains('No issues found!')])); - }); - - test('missing ci_config file is ok', () async { - final RepositoryPackage package = createFakePackage( - 'a_package', - packagesDir, - ); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml([package]); - - final List output = await runCapturingPrint(runner, [ - 'repo-package-info-check', - ]); - - expect( - output, - containsAllInOrder([contains('No issues found!')]), - ); - }); - - test('fails for unknown key', () async { - final RepositoryPackage package = createFakePackage( - 'a_package', - packagesDir, - ); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml([package]); - package.ciConfigFile.writeAsStringSync(''' -something: true - '''); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains('Unknown key `something` in config, the possible keys are'), - ]), - ); - }); - - test('fails for invalid value type for batch property in release', () async { - final RepositoryPackage package = createFakePackage( - 'a_package', - packagesDir, - ); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml([package]); - package.ciConfigFile.writeAsStringSync(''' -release: - batch: 1 - '''); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Invalid value `1` for key `release.batch`, the possible values are [true, false]', - ), - ]), - ); - }); - }); - - group('release strategy check', () { - RepositoryPackage setupReleaseStrategyTest() { - final RepositoryPackage package = createFakePackage( - 'a_package', - packagesDir, - ); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml([package]); - return package; - } - - void writeBatchConfig(RepositoryPackage package) { - package.ciConfigFile.writeAsStringSync(''' -release: - batch: true -'''); - } - - void writeWorkflowFiles({ - bool validBatchFile = true, - bool validReleaseFromBranches = true, - bool validSyncRelease = true, - }) { - final Directory workflowDir = root - .childDirectory('.github') - .childDirectory('workflows'); - workflowDir.createSync(recursive: true); - - if (validBatchFile) { - workflowDir.childFile('a_package_batch.yml').writeAsStringSync(''' -name: Batch Release -on: - schedule: - - cron: "0 8 * * 1" -jobs: - dispatch_release_pr: - runs-on: ubuntu-latest - steps: - - name: Repository Dispatch - uses: peter-evans/repository-dispatch@5fc4efd1a4797ddb68ffd0714a238564e4cc0e6f - with: - event-type: batch-release-pr - client-payload: '{"package": "a_package"}' -'''); - } - - if (validReleaseFromBranches) { - workflowDir.childFile('release_from_branches.yml').writeAsStringSync(''' -on: - push: - branches: - - 'release-a_package-*' -'''); - } - - if (validSyncRelease) { - workflowDir.childFile('sync_release_pr.yml').writeAsStringSync(''' -on: - push: - branches: - - 'release-a_package-*' -'''); - } - } - - test( - 'ignores non-batch release packages if they have no artifacts', - () async { - setupReleaseStrategyTest(); - // No config, so batch is false by default. - - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [ - FakeProcessInfo( - MockProcess(exitCode: 1), - ), // git ls-remote fails (branch doesn't exist) - ]; - - final List output = await runCapturingPrint(runner, [ - 'repo-package-info-check', - ]); - - expect( - output, - containsAllInOrder([contains('No issues found!')]), - ); - }, - ); - - test('fails if non-batch package has batch artifacts', () async { - setupReleaseStrategyTest(); - // batch defaults to false - writeWorkflowFiles(); - - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [ - FakeProcessInfo( - MockProcess(), - ), // git ls-remote succeeds (branch exists) - ]; - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - contains( - contains( - 'Unexpected batch workflow file: .github/workflows/a_package_batch.yml', - ), - ), - ); - expect( - output, - contains( - contains( - 'Unexpected trigger for release-a_package-* in .github/workflows/release_from_branches.yml', - ), - ), - ); - expect( - output, - contains( - contains( - 'Unexpected trigger for release-a_package-* in .github/workflows/sync_release_pr.yml', - ), - ), - ); - }); - - test('fails if batch package has pre-release version', () async { - final RepositoryPackage package = setupReleaseStrategyTest(); - writeBatchConfig(package); - writeWorkflowFiles(); - package.pubspecFile.writeAsStringSync(''' -name: a_package -version: 1.0.0-wip -'''); - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [ - FakeProcessInfo(MockProcess()), // git ls-remote succeeds - ]; - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - contains( - contains( - 'Batch release packages must not have a pre-release version.', - ), - ), - ); - }); - - test('fails if batch workflow file is missing', () async { - final RepositoryPackage package = setupReleaseStrategyTest(); - writeBatchConfig(package); - // Don't write workflow files. - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - contains( - contains( - 'Missing batch workflow file: .github/workflows/a_package_batch.yml', - ), - ), - ); - }); - - test('fails if batch workflow content is invalid', () async { - final RepositoryPackage package = setupReleaseStrategyTest(); - writeBatchConfig(package); - final Directory workflowDir = root - .childDirectory('.github') - .childDirectory('workflows'); - workflowDir.createSync(recursive: true); - workflowDir.childFile('a_package_batch.yml').writeAsStringSync(''' -name: Batch Release -jobs: - dispatch_release_pr: - steps: - - uses: peter-evans/repository-dispatch@5fc4efd1a4797ddb68ffd0714a238564e4cc0e6f - with: - event-type: something-else - client-payload: '{"package": "a_package"}' -'''); - // Write other files to be valid so we focus on this error - workflowDir - .childFile('release_from_branches.yml') - .writeAsStringSync("- 'release-a_package-*'"); - workflowDir - .childFile('sync_release_pr.yml') - .writeAsStringSync("- 'release-a_package-*'"); - - // Mock successful git and gh calls - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [FakeProcessInfo(MockProcess())]; - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - contains( - contains( - 'Invalid batch workflow content in a_package_batch.yml. ' - 'Must contain a step using peter-evans/repository-dispatch with:\n' - ' event-type: batch-release-pr\n' - ' client-payload: \'{"package": "a_package"}\'', - ), - ), - ); - }); - - test('fails if global workflows are missing triggers', () async { - final RepositoryPackage package = setupReleaseStrategyTest(); - writeBatchConfig(package); - writeWorkflowFiles( - validReleaseFromBranches: false, - validSyncRelease: false, - ); - // Create files but without correct content - final Directory workflowDir = root - .childDirectory('.github') - .childDirectory('workflows'); - workflowDir - .childFile('release_from_branches.yml') - .writeAsStringSync('name: something'); - workflowDir - .childFile('sync_release_pr.yml') - .writeAsStringSync('name: something'); - - gitProcessRunner.mockProcessesForExecutable['git'] = [ - FakeProcessInfo(MockProcess()), - ]; - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - contains( - contains( - 'Missing trigger for release-a_package-* in .github/workflows/release_from_branches.yml', - ), - ), - ); - expect( - output, - contains( - contains( - 'Missing trigger for release-a_package-* in .github/workflows/sync_release_pr.yml', - ), - ), - ); - }); - - test('passes if all checks pass', () async { - final RepositoryPackage package = setupReleaseStrategyTest(); - writeBatchConfig(package); - writeWorkflowFiles(); - - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [FakeProcessInfo(MockProcess())]; - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['repo-package-info-check'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - if (commandError != null) { - print('ERROR: Command failed in "passes if all checks pass"'); - print('Output:\n${output.join('\n')}'); - } - - expect(commandError, isNull); - expect( - output, - containsAllInOrder([contains('No issues found!')]), - ); - }); - }); -} diff --git a/script/tool/test/validate_command_test.dart b/script/tool/test/validate_command_test.dart new file mode 100644 index 000000000000..611865dc18bc --- /dev/null +++ b/script/tool/test/validate_command_test.dart @@ -0,0 +1,960 @@ +// Copyright 2013 The Flutter Authors +// 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:flutter_plugin_tools/src/common/core.dart'; +import 'package:flutter_plugin_tools/src/validate_command.dart'; +import 'package:git/git.dart'; +import 'package:test/test.dart'; + +import 'mocks.dart'; +import 'util.dart'; + +void main() { + late CommandRunner runner; + late Directory root; + late Directory packagesDir; + late RecordingProcessRunner gitProcessRunner; + + setUp(() { + final GitDir gitDir; + (:packagesDir, processRunner: _, :gitProcessRunner, :gitDir) = + configureBaseCommandMocks(); + root = packagesDir.fileSystem.currentDirectory; + + final command = ValidateCommand(packagesDir, gitDir: gitDir); + runner = CommandRunner('validate_test', 'Test for $ValidateCommand'); + runner.addCommand(command); + + // Default to failing these checks so that tests of non-batch-release packages + // (the default) don't fail due to "unexpected" branches/labels being found. + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [FakeProcessInfo(MockProcess(exitCode: 1))]; + }); + + String readmeTableHeader() { + return ''' +| Package | Pub | Points | Popularity | Issues | Pull requests | +|---------|-----|--------|------------|--------|---------------| +'''; + } + + String readmeTableEntry(String packageName) { + final String encodedTag = Uri.encodeComponent('p: $packageName'); + return '| [$packageName](./packages/$packageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + } + + void writeAutoLabelerYaml(List packages) { + final File labelerYaml = root + .childDirectory('.github') + .childFile('labeler.yml'); + labelerYaml.createSync(recursive: true); + labelerYaml.writeAsStringSync( + packages + .map((RepositoryPackage p) { + final bool isThirdParty = p.path.contains('third_party/'); + return ''' +-p: ${p.directory.basename} + - changed-files: + - any-glob-to-any-file: + - ${isThirdParty ? 'third_party/' : ''}packages/${p.directory.basename}/**/* +'''; + }) + .join('\n\n'), + ); + } + + group('repo info validator', () { + test('passes for correct coverage', () async { + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml(packages); + + final List output = await runCapturingPrint(runner, [ + 'validate', + ]); + + expect( + output, + containsAllInOrder([contains('Ran for 1 package(s)')]), + ); + }); + + test( + 'passes for federated plugins with only app-facing package listed', + () async { + const pluginName = 'foo'; + final Directory pluginDir = packagesDir.childDirectory(pluginName); + final packages = [ + createFakePlugin(pluginName, pluginDir), + createFakePlugin('${pluginName}_platform_interface', pluginDir), + createFakePlugin('${pluginName}_android', pluginDir), + createFakePlugin('${pluginName}_ios', pluginDir), + ]; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry(pluginName)} +'''); + writeAutoLabelerYaml([packages.first]); + writeAutoLabelerYaml([packages.first]); + + // 4 packages * 2 checks (git, gh) = 8 calls. + // Default mocks in setUp cover 1 call each. We need 3 more each. + gitProcessRunner.mockProcessesForExecutable['git-ls-remote']! + .addAll([ + FakeProcessInfo(MockProcess(exitCode: 1)), + FakeProcessInfo(MockProcess(exitCode: 1)), + FakeProcessInfo(MockProcess(exitCode: 1)), + ]); + + final List output = await runCapturingPrint(runner, [ + 'validate', + ]); + + expect( + output, + containsAllInOrder([contains('Ran for 4 package(s)')]), + ); + }, + ); + + test('fails for unexpected README table entry', () async { + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('another_package')} +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Unknown package "another_package" in root README.md table'), + ]), + ); + }); + + test('fails for missing README table entry', () async { + final packages = [ + createFakePackage('a_package', packagesDir), + createFakePackage('another_package', packagesDir), + ]; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('another_package')} +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Missing repo root README.md table entry'), + contains( + 'a_package:\n' + ' Missing repo root README.md table entry', + ), + ]), + ); + }); + + test('fails for unexpected format in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$packageName/) | ' + 'Some random text | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Invalid repo root README.md table entry: "Some random text"', + ), + contains( + 'a_package:\n' + ' Invalid root README.md table entry', + ), + ]), + ); + }); + + test('fails for incorrect source link in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + const incorrectPackageName = 'a_pakage'; + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$incorrectPackageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Incorrect link in root README.md table: "./packages/$incorrectPackageName/"', + ), + contains( + 'a_package:\n' + ' Incorrect link in root README.md table', + ), + ]), + ); + }); + + test('fails for incorrect packages/* link in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + const incorrectPackageName = 'a_pakage'; + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$packageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$incorrectPackageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Incorrect link in root README.md table: "https://pub.dev/packages/$incorrectPackageName/score"', + ), + contains( + 'a_package:\n' + ' Incorrect link in root README.md table', + ), + ]), + ); + }); + + test('fails for incorrect labels/* link in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + final String incorrectTag = Uri.encodeComponent('p: a_pakage'); + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$packageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$incorrectTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Incorrect link in root README.md table: "https://github.com/flutter/flutter/labels/$incorrectTag"', + ), + contains( + 'a_package:\n' + ' Incorrect link in root README.md table', + ), + ]), + ); + }); + + test('fails for incorrect packages/* anchor in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + const incorrectPackageName = 'a_pakage'; + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$packageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$incorrectPackageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Incorrect anchor in root README.md table: "![pub points](https://img.shields.io/pub/points/$incorrectPackageName)"', + ), + contains( + 'a_package:\n' + ' Incorrect anchor in root README.md table', + ), + ]), + ); + }); + + test('fails for incorrect tag query anchor in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + final String incorrectTag = Uri.encodeComponent('p: a_pakage'); + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$packageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$incorrectTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Incorrect anchor in root README.md table: "![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$incorrectTag?label=)', + ), + contains( + 'a_package:\n' + ' Incorrect anchor in root README.md table', + ), + ]), + ); + }); + + test('fails for missing auto-labeler entry', () async { + const packageName = 'a_package'; + createFakePackage('a_package', packagesDir); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry(packageName)} +'''); + writeAutoLabelerYaml([]); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Missing a rule in .github/labeler.yml.'), + contains( + 'a_package:\n' + ' Missing auto-labeler entry', + ), + ]), + ); + }); + + group('ci_config check', () { + test('control test', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + + package.ciConfigFile.writeAsStringSync(''' +release: + batch: false + '''); + + final List output = await runCapturingPrint(runner, [ + 'validate', + ]); + + expect(output, containsAll([contains('No issues found!')])); + }); + + test('missing ci_config file is ok', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + + final List output = await runCapturingPrint(runner, [ + 'validate', + ]); + + expect( + output, + containsAllInOrder([contains('No issues found!')]), + ); + }); + + test('fails for unknown key', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + package.ciConfigFile.writeAsStringSync(''' +something: true + '''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Unknown key `something` in config, the possible keys are', + ), + ]), + ); + }); + + test( + 'fails for invalid value type for batch property in release', + () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + package.ciConfigFile.writeAsStringSync(''' +release: + batch: 1 + '''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Invalid value `1` for key `release.batch`, the possible values are [true, false]', + ), + ]), + ); + }, + ); + }); + + group('release strategy check', () { + RepositoryPackage setupReleaseStrategyTest() { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + return package; + } + + void writeBatchConfig(RepositoryPackage package) { + package.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + } + + void writeWorkflowFiles({ + bool validBatchFile = true, + bool validReleaseFromBranches = true, + bool validSyncRelease = true, + }) { + final Directory workflowDir = root + .childDirectory('.github') + .childDirectory('workflows'); + workflowDir.createSync(recursive: true); + + if (validBatchFile) { + workflowDir.childFile('a_package_batch.yml').writeAsStringSync(''' +name: Batch Release +on: + schedule: + - cron: "0 8 * * 1" +jobs: + dispatch_release_pr: + runs-on: ubuntu-latest + steps: + - name: Repository Dispatch + uses: peter-evans/repository-dispatch@5fc4efd1a4797ddb68ffd0714a238564e4cc0e6f + with: + event-type: batch-release-pr + client-payload: '{"package": "a_package"}' +'''); + } + + if (validReleaseFromBranches) { + workflowDir.childFile('release_from_branches.yml').writeAsStringSync( + ''' +on: + push: + branches: + - 'release-a_package-*' +''', + ); + } + + if (validSyncRelease) { + workflowDir.childFile('sync_release_pr.yml').writeAsStringSync(''' +on: + push: + branches: + - 'release-a_package-*' +'''); + } + } + + test( + 'ignores non-batch release packages if they have no artifacts', + () async { + setupReleaseStrategyTest(); + // No config, so batch is false by default. + + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [ + FakeProcessInfo( + MockProcess(exitCode: 1), + ), // git ls-remote fails (branch doesn't exist) + ]; + + final List output = await runCapturingPrint(runner, [ + 'validate', + ]); + + expect( + output, + containsAllInOrder([contains('No issues found!')]), + ); + }, + ); + + test('fails if non-batch package has batch artifacts', () async { + setupReleaseStrategyTest(); + // batch defaults to false + writeWorkflowFiles(); + + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [ + FakeProcessInfo( + MockProcess(), + ), // git ls-remote succeeds (branch exists) + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + contains( + contains( + 'Unexpected batch workflow file: .github/workflows/a_package_batch.yml', + ), + ), + ); + expect( + output, + contains( + contains( + 'Unexpected trigger for release-a_package-* in .github/workflows/release_from_branches.yml', + ), + ), + ); + expect( + output, + contains( + contains( + 'Unexpected trigger for release-a_package-* in .github/workflows/sync_release_pr.yml', + ), + ), + ); + }); + + test('fails if batch package has pre-release version', () async { + final RepositoryPackage package = setupReleaseStrategyTest(); + writeBatchConfig(package); + writeWorkflowFiles(); + package.pubspecFile.writeAsStringSync(''' +name: a_package +version: 1.0.0-wip +'''); + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [ + FakeProcessInfo(MockProcess()), // git ls-remote succeeds + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + contains( + contains( + 'Batch release packages must not have a pre-release version.', + ), + ), + ); + }); + + test('fails if batch workflow file is missing', () async { + final RepositoryPackage package = setupReleaseStrategyTest(); + writeBatchConfig(package); + // Don't write workflow files. + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + contains( + contains( + 'Missing batch workflow file: .github/workflows/a_package_batch.yml', + ), + ), + ); + }); + + test('fails if batch workflow content is invalid', () async { + final RepositoryPackage package = setupReleaseStrategyTest(); + writeBatchConfig(package); + final Directory workflowDir = root + .childDirectory('.github') + .childDirectory('workflows'); + workflowDir.createSync(recursive: true); + workflowDir.childFile('a_package_batch.yml').writeAsStringSync(''' +name: Batch Release +jobs: + dispatch_release_pr: + steps: + - uses: peter-evans/repository-dispatch@5fc4efd1a4797ddb68ffd0714a238564e4cc0e6f + with: + event-type: something-else + client-payload: '{"package": "a_package"}' +'''); + // Write other files to be valid so we focus on this error + workflowDir + .childFile('release_from_branches.yml') + .writeAsStringSync("- 'release-a_package-*'"); + workflowDir + .childFile('sync_release_pr.yml') + .writeAsStringSync("- 'release-a_package-*'"); + + // Mock successful git and gh calls + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [FakeProcessInfo(MockProcess())]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + contains( + contains( + 'Invalid batch workflow content in a_package_batch.yml. ' + 'Must contain a step using peter-evans/repository-dispatch with:\n' + ' event-type: batch-release-pr\n' + ' client-payload: \'{"package": "a_package"}\'', + ), + ), + ); + }); + + test('fails if global workflows are missing triggers', () async { + final RepositoryPackage package = setupReleaseStrategyTest(); + writeBatchConfig(package); + writeWorkflowFiles( + validReleaseFromBranches: false, + validSyncRelease: false, + ); + // Create files but without correct content + final Directory workflowDir = root + .childDirectory('.github') + .childDirectory('workflows'); + workflowDir + .childFile('release_from_branches.yml') + .writeAsStringSync('name: something'); + workflowDir + .childFile('sync_release_pr.yml') + .writeAsStringSync('name: something'); + + gitProcessRunner.mockProcessesForExecutable['git'] = [ + FakeProcessInfo(MockProcess()), + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + contains( + contains( + 'Missing trigger for release-a_package-* in .github/workflows/release_from_branches.yml', + ), + ), + ); + expect( + output, + contains( + contains( + 'Missing trigger for release-a_package-* in .github/workflows/sync_release_pr.yml', + ), + ), + ); + }); + + test('passes if all checks pass', () async { + final RepositoryPackage package = setupReleaseStrategyTest(); + writeBatchConfig(package); + writeWorkflowFiles(); + + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [FakeProcessInfo(MockProcess())]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + if (commandError != null) { + print('ERROR: Command failed in "passes if all checks pass"'); + print('Output:\n${output.join('\n')}'); + } + + expect(commandError, isNull); + expect( + output, + containsAllInOrder([contains('No issues found!')]), + ); + }); + }); + }); +} From 833a8a37f38c564da3adbc118e00d8dfdffced4b Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 29 Apr 2026 16:22:35 -0400 Subject: [PATCH 02/18] Make it an all-package command, without behavior change --- .../lib/src/common/repository_package.dart | 4 ++++ script/tool/lib/src/validate_command.dart | 23 +++++++++++++++---- script/tool/test/validate_command_test.dart | 10 +++++--- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/script/tool/lib/src/common/repository_package.dart b/script/tool/lib/src/common/repository_package.dart index 6e12802ba029..f495f5d41c6b 100644 --- a/script/tool/lib/src/common/repository_package.dart +++ b/script/tool/lib/src/common/repository_package.dart @@ -166,6 +166,10 @@ class RepositoryPackage { isFederated && !isPlatformInterface && directory.basename != directory.parent.basename; + + /// True if this appears to be a top-level package, according to repository + /// conventions. + bool get isTopLevel => getEnclosingPackage() == null; /// True if this appears to be an example package, according to package /// conventions. diff --git a/script/tool/lib/src/validate_command.dart b/script/tool/lib/src/validate_command.dart index d112eefc98c5..939b142ebee3 100644 --- a/script/tool/lib/src/validate_command.dart +++ b/script/tool/lib/src/validate_command.dart @@ -33,6 +33,10 @@ class ValidateCommand extends PackageLoopingCommand { @override final String description = 'Checks that packages follow team guidelines.'; + @override + final PackageLoopingType packageLoopingType = + PackageLoopingType.includeAllSubpackages; + @override final bool hasLongOutput = false; @@ -56,16 +60,25 @@ class ValidateCommand extends PackageLoopingCommand { @override Future runForPackage(RepositoryPackage package) async { + final List errors = []; + + if (package.isTopLevel) { + errors.addAll(await _validateRepoInfo(package)); + } + + return errors.isEmpty + ? PackageResult.success() + : PackageResult.fail(errors); + } + + /// Runs repo-level checks. + Future> _validateRepoInfo(RepositoryPackage package) async { final validator = RepoInfoValidator( readmeTableEntries: _readmeTableEntries, autoLabeledPackages: _autoLabeledPackages, gitDir: await gitDir, indentation: indentation, ); - final List errors = await validator.validatePackage(package); - - return errors.isEmpty - ? PackageResult.success() - : PackageResult.fail(errors); + return validator.validatePackage(package); } } diff --git a/script/tool/test/validate_command_test.dart b/script/tool/test/validate_command_test.dart index 611865dc18bc..fdf8d3f301b5 100644 --- a/script/tool/test/validate_command_test.dart +++ b/script/tool/test/validate_command_test.dart @@ -89,7 +89,7 @@ ${readmeTableEntry('a_package')} expect( output, - containsAllInOrder([contains('Ran for 1 package(s)')]), + containsAllInOrder([contains('Running for a_package')]), ); }); @@ -110,7 +110,6 @@ ${readmeTableHeader()} ${readmeTableEntry(pluginName)} '''); writeAutoLabelerYaml([packages.first]); - writeAutoLabelerYaml([packages.first]); // 4 packages * 2 checks (git, gh) = 8 calls. // Default mocks in setUp cover 1 call each. We need 3 more each. @@ -127,7 +126,12 @@ ${readmeTableEntry(pluginName)} expect( output, - containsAllInOrder([contains('Ran for 4 package(s)')]), + containsAllInOrder([ + contains('Running for foo'), + contains('Running for foo_android'), + contains('Running for foo_ios'), + contains('Running for foo_platform_interface'), + ]), ); }, ); From 0b57fd8befedc045193efccdd7e8d0be9bc0bed5 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 29 Apr 2026 16:51:29 -0400 Subject: [PATCH 03/18] Pre-adjust some commands to run on all packages --- script/tool/lib/src/readme_check_command.dart | 20 ++++++++-- .../version_and_changelog_validator.dart | 20 +++++++++- .../tool/lib/src/version_check_command.dart | 40 +++++++++---------- 3 files changed, 55 insertions(+), 25 deletions(-) diff --git a/script/tool/lib/src/readme_check_command.dart b/script/tool/lib/src/readme_check_command.dart index 452579055992..fc3f18b5888f 100644 --- a/script/tool/lib/src/readme_check_command.dart +++ b/script/tool/lib/src/readme_check_command.dart @@ -37,11 +37,27 @@ class ReadmeCheckCommand extends PackageLoopingCommand { final String description = 'Checks that READMEs follow repository conventions.'; + @override + final PackageLoopingType packageLoopingType = + PackageLoopingType.includeAllSubpackages; + @override bool get hasLongOutput => false; @override Future runForPackage(RepositoryPackage package) async { + final List errors = []; + + if (package.isTopLevel) { + errors.addAll(await _validateReadme(package)); + } + + return errors.isEmpty + ? PackageResult.success() + : PackageResult.fail(errors); + } + + Future> _validateReadme(RepositoryPackage package) async { final validator = ReadmeValidator( path: path, indentation: indentation, @@ -78,8 +94,6 @@ class ReadmeCheckCommand extends PackageLoopingCommand { ); } - return errors.isEmpty - ? PackageResult.success() - : PackageResult.fail(errors); + return errors; } } diff --git a/script/tool/lib/src/validators/version_and_changelog_validator.dart b/script/tool/lib/src/validators/version_and_changelog_validator.dart index 0368cc72348f..12a885bb3beb 100644 --- a/script/tool/lib/src/validators/version_and_changelog_validator.dart +++ b/script/tool/lib/src/validators/version_and_changelog_validator.dart @@ -120,7 +120,15 @@ class VersionAndChangelogValidator { required bool checkForMissingChanges, required bool ignorePlatformInterfaceBreaks, }) async { - final Pubspec pubspec = package.parsePubspec(); + final Pubspec? pubspec = _tryParsePubspec(package); + if (pubspec == null) { + // No remaining checks make sense, so fail immediately. + return ['Invalid pubspec.yaml.']; + } + + if (pubspec.publishTo == 'none') { + return []; + } final Version? currentPubspecVersion = pubspec.version; if (currentPubspecVersion == null) { @@ -656,6 +664,16 @@ ${_indentation}The first version listed in CHANGELOG.md is $fromChangeLog. platformContext: _path, ); } + + Pubspec? _tryParsePubspec(RepositoryPackage package) { + try { + final Pubspec pubspec = package.parsePubspec(); + return pubspec; + } on Exception catch (exception) { + printError('${_indentation}Failed to parse `pubspec.yaml`: $exception}'); + return null; + } + } } /// The state of a package's version relative to the comparison base. diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index e359cb25f6d6..a4ec2114f805 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -5,7 +5,6 @@ import 'package:file/file.dart'; import 'common/git_version_finder.dart'; -import 'common/output_utils.dart'; import 'common/package_looping_command.dart'; import 'common/repository_package.dart'; import 'validators/version_and_changelog_validator.dart'; @@ -71,6 +70,10 @@ class VersionCheckCommand extends PackageLoopingCommand { 'Also checks if the latest version in CHANGELOG matches the version in pubspec.\n\n' 'This command requires "flutter" to be in your path.'; + @override + final PackageLoopingType packageLoopingType = + PackageLoopingType.includeAllSubpackages; + @override bool get hasLongOutput => false; @@ -81,15 +84,23 @@ class VersionCheckCommand extends PackageLoopingCommand { @override Future runForPackage(RepositoryPackage package) async { - final Pubspec? pubspec = _tryParsePubspec(package); - if (pubspec == null) { - // No remaining checks make sense, so fail immediately. - return PackageResult.fail(['Invalid pubspec.yaml.']); + if (!package.isTopLevel) { + return PackageResult.skip('Not a top-level package.'); } + final List errors = []; - if (pubspec.publishTo == 'none') { - return PackageResult.skip('Found "publish_to: none".'); + if (package.isTopLevel) { + errors.addAll(await _validateVersionAndChangelog(package)); } + + return errors.isEmpty + ? PackageResult.success() + : PackageResult.fail(errors); + } + + Future> _validateVersionAndChangelog( + RepositoryPackage package, + ) async { final Directory repoRoot = packagesDir.fileSystem.directory( (await gitDir).path, ); @@ -104,24 +115,11 @@ class VersionCheckCommand extends PackageLoopingCommand { prLabels: _prLabels, ); - final List errors = await validator.validateChangelogAndVersion( + return validator.validateChangelogAndVersion( package, checkForMissingChanges: getBoolArg(_checkForMissingChanges), ignorePlatformInterfaceBreaks: getBoolArg(_ignorePlatformInterfaceBreaks), ); - return errors.isEmpty - ? PackageResult.success() - : PackageResult.fail(errors); - } - - Pubspec? _tryParsePubspec(RepositoryPackage package) { - try { - final Pubspec pubspec = package.parsePubspec(); - return pubspec; - } on Exception catch (exception) { - printError('${indentation}Failed to parse `pubspec.yaml`: $exception}'); - return null; - } } /// Returns the labels associated with this PR, if any, or an empty set From 7d394a497b6b9db9c4db99a1a197a6305f650c77 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 30 Apr 2026 09:58:36 -0400 Subject: [PATCH 04/18] Fold in dependabot, and change test structure to avoid test setup combinatorics --- .ci/targets/repo_checks.yaml | 4 +- .../lib/src/dependabot_check_command.dart | 64 -- script/tool/lib/src/main.dart | 2 - script/tool/lib/src/validate_command.dart | 82 +- ... => validate_command_dependabot_test.dart} | 22 +- .../test/validate_command_repo_info_test.dart | 960 +++++++++++++++++ script/tool/test/validate_command_test.dart | 964 ------------------ 7 files changed, 1042 insertions(+), 1056 deletions(-) delete mode 100644 script/tool/lib/src/dependabot_check_command.dart rename script/tool/test/{dependabot_check_command_test.dart => validate_command_dependabot_test.dart} (93%) create mode 100644 script/tool/test/validate_command_repo_info_test.dart delete mode 100644 script/tool/test/validate_command_test.dart diff --git a/.ci/targets/repo_checks.yaml b/.ci/targets/repo_checks.yaml index 924468351134..20b68063066c 100644 --- a/.ci/targets/repo_checks.yaml +++ b/.ci/targets/repo_checks.yaml @@ -56,9 +56,9 @@ tasks: script: .ci/scripts/tool_runner.sh args: ["check-repo-package-info"] always: true - - name: Dependabot coverage validation + - name: Repository standards validation script: .ci/scripts/tool_runner.sh - args: ["dependabot-check"] + args: ["validate"] always: true - name: CHANGELOG and version validation script: .ci/scripts/check_version.sh diff --git a/script/tool/lib/src/dependabot_check_command.dart b/script/tool/lib/src/dependabot_check_command.dart deleted file mode 100644 index e2b2bf5d8ec6..000000000000 --- a/script/tool/lib/src/dependabot_check_command.dart +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2013 The Flutter Authors -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:file/file.dart'; - -import 'common/package_looping_command.dart'; -import 'common/repository_package.dart'; -import 'validators/dependabot_validator.dart'; - -/// A command to verify Dependabot configuration coverage of packages. -class DependabotCheckCommand extends PackageLoopingCommand { - /// Creates Dependabot check command instance. - DependabotCheckCommand(super.packagesDir, {super.gitDir}); - - late Directory _repoRoot; - - // The set of directories covered by the repo's Dependabot configuration. - late DependabotCoverage _coverage; - - @override - final String name = 'dependabot-check'; - - @override - List get aliases => ['check-dependabot']; - - @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); - - _coverage = DependabotValidator.loadConfig(repoRoot: _repoRoot); - } - - @override - Future runForPackage(RepositoryPackage package) async { - final validator = DependabotValidator( - coverage: _coverage, - path: path, - repoRoot: _repoRoot, - indentation: indentation, - ); - - final errors = []; - - errors.addAll(validator.validateDependabotCoverage(package)); - - // TODO(stuartmorgan): Add other ecosystem checks here as more are enabled. - - return errors.isEmpty - ? PackageResult.success() - : PackageResult.fail(errors); - } -} diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index 79676439fea3..95351382055e 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -15,7 +15,6 @@ import 'common/core.dart'; import 'create_all_packages_app_command.dart'; import 'custom_test_command.dart'; import 'dart_test_command.dart'; -import 'dependabot_check_command.dart'; import 'drive_examples_command.dart'; import 'federation_safety_check_command.dart'; import 'fetch_deps_command.dart'; @@ -69,7 +68,6 @@ void main(List args) { ..addCommand(CreateAllPackagesAppCommand(packagesDir)) ..addCommand(CustomTestCommand(packagesDir)) ..addCommand(DartTestCommand(packagesDir)) - ..addCommand(DependabotCheckCommand(packagesDir)) ..addCommand(DriveExamplesCommand(packagesDir)) ..addCommand(FederationSafetyCheckCommand(packagesDir)) ..addCommand(FetchDepsCommand(packagesDir)) diff --git a/script/tool/lib/src/validate_command.dart b/script/tool/lib/src/validate_command.dart index 939b142ebee3..988de4637442 100644 --- a/script/tool/lib/src/validate_command.dart +++ b/script/tool/lib/src/validate_command.dart @@ -3,20 +3,39 @@ // found in the LICENSE file. import 'package:file/file.dart'; +import 'package:meta/meta.dart'; import 'common/package_looping_command.dart'; import 'common/repository_package.dart'; +import 'validators/dependabot_validator.dart'; import 'validators/repo_info_validator.dart'; +/// The set of possible validators. +/// +/// Exposed for testing so that unit tests can target a single validator's +/// behavior via the command without having to set everything required for +/// every other validator to pass. +/// +/// This is done instead of testing validators directly to ensure that testing +/// includes things like command line parsing and run initialization. +@visibleForTesting +enum Validator { dependabot, repoInfo } + /// A command to validate that packages follow various team conventions, /// guidelines, and best practices. /// /// This includes: /// - repository-level metadata about packages, such as repo README and -/// auto-label entries. +/// auto-label entries +/// - dependabot configuration coverage class ValidateCommand extends PackageLoopingCommand { /// Creates Dependabot check command instance. - ValidateCommand(super.packagesDir, {super.gitDir}); + ValidateCommand(super.packagesDir, {this.targetedValidators, super.gitDir}); + + /// The validators to run. + /// + /// If null, all validators are run. + final Set? targetedValidators; late Directory _repoRoot; @@ -27,6 +46,9 @@ class ValidateCommand extends PackageLoopingCommand { /// Packages with entries in labeler.yml. final Set _autoLabeledPackages = {}; + // The set of directories covered by the repo's Dependabot configuration. + late DependabotCoverage _dependabotCoverage; + @override final String name = 'validate'; @@ -44,18 +66,23 @@ class ValidateCommand extends PackageLoopingCommand { Future initializeRun() async { _repoRoot = packagesDir.fileSystem.directory((await gitDir).path); - // Extract all of the repo-level README.md table entries. - _readmeTableEntries.addAll( - RepoInfoValidator.loadReadmeTableEntries( - repoRoot: _repoRoot, - packagesDir: packagesDir, - thirdPartyPackagesDir: thirdPartyPackagesDir, - ), - ); - // Extract all of the labeler.yml package entries. - _autoLabeledPackages.addAll( - RepoInfoValidator.loadAutoLabeledPackages(repoRoot: _repoRoot), - ); + if (_shouldRun(Validator.repoInfo)) { + // Extract all of the repo-level README.md table entries. + _readmeTableEntries.addAll( + RepoInfoValidator.loadReadmeTableEntries( + repoRoot: _repoRoot, + packagesDir: packagesDir, + thirdPartyPackagesDir: thirdPartyPackagesDir, + ), + ); + // Extract all of the labeler.yml package entries. + _autoLabeledPackages.addAll( + RepoInfoValidator.loadAutoLabeledPackages(repoRoot: _repoRoot), + ); + } + if (_shouldRun(Validator.dependabot)) { + _dependabotCoverage = DependabotValidator.loadConfig(repoRoot: _repoRoot); + } } @override @@ -63,7 +90,12 @@ class ValidateCommand extends PackageLoopingCommand { final List errors = []; if (package.isTopLevel) { - errors.addAll(await _validateRepoInfo(package)); + if (_shouldRun(Validator.repoInfo)) { + errors.addAll(await _validateRepoInfo(package)); + } + } + if (_shouldRun(Validator.dependabot)) { + errors.addAll(await _validateDependabot(package)); } return errors.isEmpty @@ -81,4 +113,24 @@ class ValidateCommand extends PackageLoopingCommand { ); return validator.validatePackage(package); } + + Future> _validateDependabot(RepositoryPackage package) async { + final validator = DependabotValidator( + coverage: _dependabotCoverage, + path: path, + repoRoot: _repoRoot, + indentation: indentation, + ); + + final errors = []; + + errors.addAll(validator.validateDependabotCoverage(package)); + + // TODO(stuartmorgan): Add other ecosystem checks here as more are enabled. + + return errors; + } + + bool _shouldRun(Validator validator) => + targetedValidators?.contains(validator) ?? true; } diff --git a/script/tool/test/dependabot_check_command_test.dart b/script/tool/test/validate_command_dependabot_test.dart similarity index 93% rename from script/tool/test/dependabot_check_command_test.dart rename to script/tool/test/validate_command_dependabot_test.dart index 52558fb38679..58cbfaaa23cb 100644 --- a/script/tool/test/dependabot_check_command_test.dart +++ b/script/tool/test/validate_command_dependabot_test.dart @@ -5,7 +5,7 @@ import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:flutter_plugin_tools/src/common/core.dart'; -import 'package:flutter_plugin_tools/src/dependabot_check_command.dart'; +import 'package:flutter_plugin_tools/src/validate_command.dart'; import 'package:git/git.dart'; import 'package:test/test.dart'; @@ -22,10 +22,14 @@ void main() { configureBaseCommandMocks(); root = packagesDir.parent; - final command = DependabotCheckCommand(packagesDir, gitDir: gitDir); + final command = ValidateCommand( + packagesDir, + gitDir: gitDir, + targetedValidators: {Validator.dependabot}, + ); runner = CommandRunner( - 'dependabot_test', - 'Test for $DependabotCheckCommand', + 'validate_dependabot_test', + 'Test for validating dependabot coverage', ); runner.addCommand(command); }); @@ -73,7 +77,7 @@ $gradleEntries createFakePackage('a_package', packagesDir); final List output = await runCapturingPrint(runner, [ - 'dependabot-check', + 'validate', ]); expect(output, containsAllInOrder([contains('No issues found!')])); @@ -94,7 +98,7 @@ $gradleEntries Error? commandError; final List output = await runCapturingPrint( runner, - ['dependabot-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -124,7 +128,7 @@ $gradleEntries Error? commandError; final List output = await runCapturingPrint( runner, - ['dependabot-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -164,7 +168,7 @@ $gradleEntries .createSync(recursive: true); final List output = await runCapturingPrint(runner, [ - 'dependabot-check', + 'validate', ]); expect( @@ -197,7 +201,7 @@ $gradleEntries .createSync(recursive: true); final List output = await runCapturingPrint(runner, [ - 'dependabot-check', + 'validate', ]); expect( diff --git a/script/tool/test/validate_command_repo_info_test.dart b/script/tool/test/validate_command_repo_info_test.dart new file mode 100644 index 000000000000..b796e0d19a42 --- /dev/null +++ b/script/tool/test/validate_command_repo_info_test.dart @@ -0,0 +1,960 @@ +// Copyright 2013 The Flutter Authors +// 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:flutter_plugin_tools/src/common/core.dart'; +import 'package:flutter_plugin_tools/src/validate_command.dart'; +import 'package:git/git.dart'; +import 'package:test/test.dart'; + +import 'mocks.dart'; +import 'util.dart'; + +void main() { + late CommandRunner runner; + late Directory root; + late Directory packagesDir; + late RecordingProcessRunner gitProcessRunner; + + setUp(() { + final GitDir gitDir; + (:packagesDir, processRunner: _, :gitProcessRunner, :gitDir) = + configureBaseCommandMocks(); + root = packagesDir.fileSystem.currentDirectory; + + final command = ValidateCommand( + packagesDir, + gitDir: gitDir, + targetedValidators: {Validator.repoInfo}, + ); + runner = CommandRunner( + 'validate_repo_info_test', + 'Test for validating repo info', + ); + runner.addCommand(command); + + // Default to failing these checks so that tests of non-batch-release packages + // (the default) don't fail due to "unexpected" branches/labels being found. + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [FakeProcessInfo(MockProcess(exitCode: 1))]; + }); + + String readmeTableHeader() { + return ''' +| Package | Pub | Points | Popularity | Issues | Pull requests | +|---------|-----|--------|------------|--------|---------------| +'''; + } + + String readmeTableEntry(String packageName) { + final String encodedTag = Uri.encodeComponent('p: $packageName'); + return '| [$packageName](./packages/$packageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + } + + void writeAutoLabelerYaml(List packages) { + final File labelerYaml = root + .childDirectory('.github') + .childFile('labeler.yml'); + labelerYaml.createSync(recursive: true); + labelerYaml.writeAsStringSync( + packages + .map((RepositoryPackage p) { + final bool isThirdParty = p.path.contains('third_party/'); + return ''' +-p: ${p.directory.basename} + - changed-files: + - any-glob-to-any-file: + - ${isThirdParty ? 'third_party/' : ''}packages/${p.directory.basename}/**/* +'''; + }) + .join('\n\n'), + ); + } + + test('passes for correct coverage', () async { + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml(packages); + + final List output = await runCapturingPrint(runner, [ + 'validate', + ]); + + expect( + output, + containsAllInOrder([contains('Running for a_package')]), + ); + }); + + test( + 'passes for federated plugins with only app-facing package listed', + () async { + const pluginName = 'foo'; + final Directory pluginDir = packagesDir.childDirectory(pluginName); + final packages = [ + createFakePlugin(pluginName, pluginDir), + createFakePlugin('${pluginName}_platform_interface', pluginDir), + createFakePlugin('${pluginName}_android', pluginDir), + createFakePlugin('${pluginName}_ios', pluginDir), + ]; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry(pluginName)} +'''); + writeAutoLabelerYaml([packages.first]); + + // 4 packages * 2 checks (git, gh) = 8 calls. + // Default mocks in setUp cover 1 call each. We need 3 more each. + gitProcessRunner.mockProcessesForExecutable['git-ls-remote']! + .addAll([ + FakeProcessInfo(MockProcess(exitCode: 1)), + FakeProcessInfo(MockProcess(exitCode: 1)), + FakeProcessInfo(MockProcess(exitCode: 1)), + ]); + + final List output = await runCapturingPrint(runner, [ + 'validate', + ]); + + expect( + output, + containsAllInOrder([ + contains('Running for foo'), + contains('Running for foo_android'), + contains('Running for foo_ios'), + contains('Running for foo_platform_interface'), + ]), + ); + }, + ); + + test('fails for unexpected README table entry', () async { + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('another_package')} +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Unknown package "another_package" in root README.md table'), + ]), + ); + }); + + test('fails for missing README table entry', () async { + final packages = [ + createFakePackage('a_package', packagesDir), + createFakePackage('another_package', packagesDir), + ]; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('another_package')} +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Missing repo root README.md table entry'), + contains( + 'a_package:\n' + ' Missing repo root README.md table entry', + ), + ]), + ); + }); + + test('fails for unexpected format in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$packageName/) | ' + 'Some random text | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Invalid repo root README.md table entry: "Some random text"'), + contains( + 'a_package:\n' + ' Invalid root README.md table entry', + ), + ]), + ); + }); + + test('fails for incorrect source link in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + const incorrectPackageName = 'a_pakage'; + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$incorrectPackageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Incorrect link in root README.md table: "./packages/$incorrectPackageName/"', + ), + contains( + 'a_package:\n' + ' Incorrect link in root README.md table', + ), + ]), + ); + }); + + test('fails for incorrect packages/* link in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + const incorrectPackageName = 'a_pakage'; + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$packageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$incorrectPackageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Incorrect link in root README.md table: "https://pub.dev/packages/$incorrectPackageName/score"', + ), + contains( + 'a_package:\n' + ' Incorrect link in root README.md table', + ), + ]), + ); + }); + + test('fails for incorrect labels/* link in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + final String incorrectTag = Uri.encodeComponent('p: a_pakage'); + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$packageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$incorrectTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Incorrect link in root README.md table: "https://github.com/flutter/flutter/labels/$incorrectTag"', + ), + contains( + 'a_package:\n' + ' Incorrect link in root README.md table', + ), + ]), + ); + }); + + test('fails for incorrect packages/* anchor in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + const incorrectPackageName = 'a_pakage'; + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$packageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$incorrectPackageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Incorrect anchor in root README.md table: "![pub points](https://img.shields.io/pub/points/$incorrectPackageName)"', + ), + contains( + 'a_package:\n' + ' Incorrect anchor in root README.md table', + ), + ]), + ); + }); + + test('fails for incorrect tag query anchor in README table entry', () async { + const packageName = 'a_package'; + final String encodedTag = Uri.encodeComponent('p: $packageName'); + final String incorrectTag = Uri.encodeComponent('p: a_pakage'); + final packages = [ + createFakePackage('a_package', packagesDir), + ]; + + final entry = + '| [$packageName](./packages/$packageName/) | ' + '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' + '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' + '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$incorrectTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' + '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +$entry +'''); + writeAutoLabelerYaml(packages); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Incorrect anchor in root README.md table: "![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$incorrectTag?label=)', + ), + contains( + 'a_package:\n' + ' Incorrect anchor in root README.md table', + ), + ]), + ); + }); + + test('fails for missing auto-labeler entry', () async { + const packageName = 'a_package'; + createFakePackage('a_package', packagesDir); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry(packageName)} +'''); + writeAutoLabelerYaml([]); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Missing a rule in .github/labeler.yml.'), + contains( + 'a_package:\n' + ' Missing auto-labeler entry', + ), + ]), + ); + }); + + group('ci_config check', () { + test('control test', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + + package.ciConfigFile.writeAsStringSync(''' +release: + batch: false + '''); + + final List output = await runCapturingPrint(runner, [ + 'validate', + ]); + + expect(output, containsAll([contains('No issues found!')])); + }); + + test('missing ci_config file is ok', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + + final List output = await runCapturingPrint(runner, [ + 'validate', + ]); + + expect( + output, + containsAllInOrder([contains('No issues found!')]), + ); + }); + + test('fails for unknown key', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + package.ciConfigFile.writeAsStringSync(''' +something: true + '''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Unknown key `something` in config, the possible keys are'), + ]), + ); + }); + + test('fails for invalid value type for batch property in release', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + package.ciConfigFile.writeAsStringSync(''' +release: + batch: 1 + '''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Invalid value `1` for key `release.batch`, the possible values are [true, false]', + ), + ]), + ); + }); + }); + + group('release strategy check', () { + RepositoryPackage setupReleaseStrategyTest() { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); + + root.childFile('README.md').writeAsStringSync(''' +${readmeTableHeader()} +${readmeTableEntry('a_package')} +'''); + writeAutoLabelerYaml([package]); + return package; + } + + void writeBatchConfig(RepositoryPackage package) { + package.ciConfigFile.writeAsStringSync(''' +release: + batch: true +'''); + } + + void writeWorkflowFiles({ + bool validBatchFile = true, + bool validReleaseFromBranches = true, + bool validSyncRelease = true, + }) { + final Directory workflowDir = root + .childDirectory('.github') + .childDirectory('workflows'); + workflowDir.createSync(recursive: true); + + if (validBatchFile) { + workflowDir.childFile('a_package_batch.yml').writeAsStringSync(''' +name: Batch Release +on: + schedule: + - cron: "0 8 * * 1" +jobs: + dispatch_release_pr: + runs-on: ubuntu-latest + steps: + - name: Repository Dispatch + uses: peter-evans/repository-dispatch@5fc4efd1a4797ddb68ffd0714a238564e4cc0e6f + with: + event-type: batch-release-pr + client-payload: '{"package": "a_package"}' +'''); + } + + if (validReleaseFromBranches) { + workflowDir.childFile('release_from_branches.yml').writeAsStringSync(''' +on: + push: + branches: + - 'release-a_package-*' +'''); + } + + if (validSyncRelease) { + workflowDir.childFile('sync_release_pr.yml').writeAsStringSync(''' +on: + push: + branches: + - 'release-a_package-*' +'''); + } + } + + test( + 'ignores non-batch release packages if they have no artifacts', + () async { + setupReleaseStrategyTest(); + // No config, so batch is false by default. + + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [ + FakeProcessInfo( + MockProcess(exitCode: 1), + ), // git ls-remote fails (branch doesn't exist) + ]; + + final List output = await runCapturingPrint(runner, [ + 'validate', + ]); + + expect( + output, + containsAllInOrder([contains('No issues found!')]), + ); + }, + ); + + test('fails if non-batch package has batch artifacts', () async { + setupReleaseStrategyTest(); + // batch defaults to false + writeWorkflowFiles(); + + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [ + FakeProcessInfo( + MockProcess(), + ), // git ls-remote succeeds (branch exists) + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + contains( + contains( + 'Unexpected batch workflow file: .github/workflows/a_package_batch.yml', + ), + ), + ); + expect( + output, + contains( + contains( + 'Unexpected trigger for release-a_package-* in .github/workflows/release_from_branches.yml', + ), + ), + ); + expect( + output, + contains( + contains( + 'Unexpected trigger for release-a_package-* in .github/workflows/sync_release_pr.yml', + ), + ), + ); + }); + + test('fails if batch package has pre-release version', () async { + final RepositoryPackage package = setupReleaseStrategyTest(); + writeBatchConfig(package); + writeWorkflowFiles(); + package.pubspecFile.writeAsStringSync(''' +name: a_package +version: 1.0.0-wip +'''); + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [ + FakeProcessInfo(MockProcess()), // git ls-remote succeeds + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + contains( + contains( + 'Batch release packages must not have a pre-release version.', + ), + ), + ); + }); + + test('fails if batch workflow file is missing', () async { + final RepositoryPackage package = setupReleaseStrategyTest(); + writeBatchConfig(package); + // Don't write workflow files. + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + contains( + contains( + 'Missing batch workflow file: .github/workflows/a_package_batch.yml', + ), + ), + ); + }); + + test('fails if batch workflow content is invalid', () async { + final RepositoryPackage package = setupReleaseStrategyTest(); + writeBatchConfig(package); + final Directory workflowDir = root + .childDirectory('.github') + .childDirectory('workflows'); + workflowDir.createSync(recursive: true); + workflowDir.childFile('a_package_batch.yml').writeAsStringSync(''' +name: Batch Release +jobs: + dispatch_release_pr: + steps: + - uses: peter-evans/repository-dispatch@5fc4efd1a4797ddb68ffd0714a238564e4cc0e6f + with: + event-type: something-else + client-payload: '{"package": "a_package"}' +'''); + // Write other files to be valid so we focus on this error + workflowDir + .childFile('release_from_branches.yml') + .writeAsStringSync("- 'release-a_package-*'"); + workflowDir + .childFile('sync_release_pr.yml') + .writeAsStringSync("- 'release-a_package-*'"); + + // Mock successful git and gh calls + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [FakeProcessInfo(MockProcess())]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + contains( + contains( + 'Invalid batch workflow content in a_package_batch.yml. ' + 'Must contain a step using peter-evans/repository-dispatch with:\n' + ' event-type: batch-release-pr\n' + ' client-payload: \'{"package": "a_package"}\'', + ), + ), + ); + }); + + test('fails if global workflows are missing triggers', () async { + final RepositoryPackage package = setupReleaseStrategyTest(); + writeBatchConfig(package); + writeWorkflowFiles( + validReleaseFromBranches: false, + validSyncRelease: false, + ); + // Create files but without correct content + final Directory workflowDir = root + .childDirectory('.github') + .childDirectory('workflows'); + workflowDir + .childFile('release_from_branches.yml') + .writeAsStringSync('name: something'); + workflowDir + .childFile('sync_release_pr.yml') + .writeAsStringSync('name: something'); + + gitProcessRunner.mockProcessesForExecutable['git'] = [ + FakeProcessInfo(MockProcess()), + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + contains( + contains( + 'Missing trigger for release-a_package-* in .github/workflows/release_from_branches.yml', + ), + ), + ); + expect( + output, + contains( + contains( + 'Missing trigger for release-a_package-* in .github/workflows/sync_release_pr.yml', + ), + ), + ); + }); + + test('passes if all checks pass', () async { + final RepositoryPackage package = setupReleaseStrategyTest(); + writeBatchConfig(package); + writeWorkflowFiles(); + + gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = + [FakeProcessInfo(MockProcess())]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['validate'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + if (commandError != null) { + print('ERROR: Command failed in "passes if all checks pass"'); + print('Output:\n${output.join('\n')}'); + } + + expect(commandError, isNull); + expect( + output, + containsAllInOrder([contains('No issues found!')]), + ); + }); + }); +} diff --git a/script/tool/test/validate_command_test.dart b/script/tool/test/validate_command_test.dart deleted file mode 100644 index fdf8d3f301b5..000000000000 --- a/script/tool/test/validate_command_test.dart +++ /dev/null @@ -1,964 +0,0 @@ -// Copyright 2013 The Flutter Authors -// 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:flutter_plugin_tools/src/common/core.dart'; -import 'package:flutter_plugin_tools/src/validate_command.dart'; -import 'package:git/git.dart'; -import 'package:test/test.dart'; - -import 'mocks.dart'; -import 'util.dart'; - -void main() { - late CommandRunner runner; - late Directory root; - late Directory packagesDir; - late RecordingProcessRunner gitProcessRunner; - - setUp(() { - final GitDir gitDir; - (:packagesDir, processRunner: _, :gitProcessRunner, :gitDir) = - configureBaseCommandMocks(); - root = packagesDir.fileSystem.currentDirectory; - - final command = ValidateCommand(packagesDir, gitDir: gitDir); - runner = CommandRunner('validate_test', 'Test for $ValidateCommand'); - runner.addCommand(command); - - // Default to failing these checks so that tests of non-batch-release packages - // (the default) don't fail due to "unexpected" branches/labels being found. - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [FakeProcessInfo(MockProcess(exitCode: 1))]; - }); - - String readmeTableHeader() { - return ''' -| Package | Pub | Points | Popularity | Issues | Pull requests | -|---------|-----|--------|------------|--------|---------------| -'''; - } - - String readmeTableEntry(String packageName) { - final String encodedTag = Uri.encodeComponent('p: $packageName'); - return '| [$packageName](./packages/$packageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - } - - void writeAutoLabelerYaml(List packages) { - final File labelerYaml = root - .childDirectory('.github') - .childFile('labeler.yml'); - labelerYaml.createSync(recursive: true); - labelerYaml.writeAsStringSync( - packages - .map((RepositoryPackage p) { - final bool isThirdParty = p.path.contains('third_party/'); - return ''' --p: ${p.directory.basename} - - changed-files: - - any-glob-to-any-file: - - ${isThirdParty ? 'third_party/' : ''}packages/${p.directory.basename}/**/* -'''; - }) - .join('\n\n'), - ); - } - - group('repo info validator', () { - test('passes for correct coverage', () async { - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml(packages); - - final List output = await runCapturingPrint(runner, [ - 'validate', - ]); - - expect( - output, - containsAllInOrder([contains('Running for a_package')]), - ); - }); - - test( - 'passes for federated plugins with only app-facing package listed', - () async { - const pluginName = 'foo'; - final Directory pluginDir = packagesDir.childDirectory(pluginName); - final packages = [ - createFakePlugin(pluginName, pluginDir), - createFakePlugin('${pluginName}_platform_interface', pluginDir), - createFakePlugin('${pluginName}_android', pluginDir), - createFakePlugin('${pluginName}_ios', pluginDir), - ]; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry(pluginName)} -'''); - writeAutoLabelerYaml([packages.first]); - - // 4 packages * 2 checks (git, gh) = 8 calls. - // Default mocks in setUp cover 1 call each. We need 3 more each. - gitProcessRunner.mockProcessesForExecutable['git-ls-remote']! - .addAll([ - FakeProcessInfo(MockProcess(exitCode: 1)), - FakeProcessInfo(MockProcess(exitCode: 1)), - FakeProcessInfo(MockProcess(exitCode: 1)), - ]); - - final List output = await runCapturingPrint(runner, [ - 'validate', - ]); - - expect( - output, - containsAllInOrder([ - contains('Running for foo'), - contains('Running for foo_android'), - contains('Running for foo_ios'), - contains('Running for foo_platform_interface'), - ]), - ); - }, - ); - - test('fails for unexpected README table entry', () async { - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('another_package')} -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains('Unknown package "another_package" in root README.md table'), - ]), - ); - }); - - test('fails for missing README table entry', () async { - final packages = [ - createFakePackage('a_package', packagesDir), - createFakePackage('another_package', packagesDir), - ]; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('another_package')} -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains('Missing repo root README.md table entry'), - contains( - 'a_package:\n' - ' Missing repo root README.md table entry', - ), - ]), - ); - }); - - test('fails for unexpected format in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$packageName/) | ' - 'Some random text | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Invalid repo root README.md table entry: "Some random text"', - ), - contains( - 'a_package:\n' - ' Invalid root README.md table entry', - ), - ]), - ); - }); - - test('fails for incorrect source link in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - const incorrectPackageName = 'a_pakage'; - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$incorrectPackageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Incorrect link in root README.md table: "./packages/$incorrectPackageName/"', - ), - contains( - 'a_package:\n' - ' Incorrect link in root README.md table', - ), - ]), - ); - }); - - test('fails for incorrect packages/* link in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - const incorrectPackageName = 'a_pakage'; - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$packageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$incorrectPackageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Incorrect link in root README.md table: "https://pub.dev/packages/$incorrectPackageName/score"', - ), - contains( - 'a_package:\n' - ' Incorrect link in root README.md table', - ), - ]), - ); - }); - - test('fails for incorrect labels/* link in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - final String incorrectTag = Uri.encodeComponent('p: a_pakage'); - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$packageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$incorrectTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Incorrect link in root README.md table: "https://github.com/flutter/flutter/labels/$incorrectTag"', - ), - contains( - 'a_package:\n' - ' Incorrect link in root README.md table', - ), - ]), - ); - }); - - test('fails for incorrect packages/* anchor in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - const incorrectPackageName = 'a_pakage'; - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$packageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$incorrectPackageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$encodedTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Incorrect anchor in root README.md table: "![pub points](https://img.shields.io/pub/points/$incorrectPackageName)"', - ), - contains( - 'a_package:\n' - ' Incorrect anchor in root README.md table', - ), - ]), - ); - }); - - test('fails for incorrect tag query anchor in README table entry', () async { - const packageName = 'a_package'; - final String encodedTag = Uri.encodeComponent('p: $packageName'); - final String incorrectTag = Uri.encodeComponent('p: a_pakage'); - final packages = [ - createFakePackage('a_package', packagesDir), - ]; - - final entry = - '| [$packageName](./packages/$packageName/) | ' - '[![pub package](https://img.shields.io/pub/v/$packageName.svg)](https://pub.dev/packages/$packageName) | ' - '[![pub points](https://img.shields.io/pub/points/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![popularity](https://img.shields.io/pub/popularity/$packageName)](https://pub.dev/packages/$packageName/score) | ' - '[![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$incorrectTag?label=)](https://github.com/flutter/flutter/labels/$encodedTag) | ' - '[![GitHub pull requests by-label](https://img.shields.io/github/issues-pr/flutter/packages/$encodedTag?label=)](https://github.com/flutter/packages/labels/$encodedTag) |'; - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -$entry -'''); - writeAutoLabelerYaml(packages); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Incorrect anchor in root README.md table: "![GitHub issues by-label](https://img.shields.io/github/issues/flutter/flutter/$incorrectTag?label=)', - ), - contains( - 'a_package:\n' - ' Incorrect anchor in root README.md table', - ), - ]), - ); - }); - - test('fails for missing auto-labeler entry', () async { - const packageName = 'a_package'; - createFakePackage('a_package', packagesDir); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry(packageName)} -'''); - writeAutoLabelerYaml([]); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains('Missing a rule in .github/labeler.yml.'), - contains( - 'a_package:\n' - ' Missing auto-labeler entry', - ), - ]), - ); - }); - - group('ci_config check', () { - test('control test', () async { - final RepositoryPackage package = createFakePackage( - 'a_package', - packagesDir, - ); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml([package]); - - package.ciConfigFile.writeAsStringSync(''' -release: - batch: false - '''); - - final List output = await runCapturingPrint(runner, [ - 'validate', - ]); - - expect(output, containsAll([contains('No issues found!')])); - }); - - test('missing ci_config file is ok', () async { - final RepositoryPackage package = createFakePackage( - 'a_package', - packagesDir, - ); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml([package]); - - final List output = await runCapturingPrint(runner, [ - 'validate', - ]); - - expect( - output, - containsAllInOrder([contains('No issues found!')]), - ); - }); - - test('fails for unknown key', () async { - final RepositoryPackage package = createFakePackage( - 'a_package', - packagesDir, - ); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml([package]); - package.ciConfigFile.writeAsStringSync(''' -something: true - '''); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Unknown key `something` in config, the possible keys are', - ), - ]), - ); - }); - - test( - 'fails for invalid value type for batch property in release', - () async { - final RepositoryPackage package = createFakePackage( - 'a_package', - packagesDir, - ); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml([package]); - package.ciConfigFile.writeAsStringSync(''' -release: - batch: 1 - '''); - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder([ - contains( - 'Invalid value `1` for key `release.batch`, the possible values are [true, false]', - ), - ]), - ); - }, - ); - }); - - group('release strategy check', () { - RepositoryPackage setupReleaseStrategyTest() { - final RepositoryPackage package = createFakePackage( - 'a_package', - packagesDir, - ); - - root.childFile('README.md').writeAsStringSync(''' -${readmeTableHeader()} -${readmeTableEntry('a_package')} -'''); - writeAutoLabelerYaml([package]); - return package; - } - - void writeBatchConfig(RepositoryPackage package) { - package.ciConfigFile.writeAsStringSync(''' -release: - batch: true -'''); - } - - void writeWorkflowFiles({ - bool validBatchFile = true, - bool validReleaseFromBranches = true, - bool validSyncRelease = true, - }) { - final Directory workflowDir = root - .childDirectory('.github') - .childDirectory('workflows'); - workflowDir.createSync(recursive: true); - - if (validBatchFile) { - workflowDir.childFile('a_package_batch.yml').writeAsStringSync(''' -name: Batch Release -on: - schedule: - - cron: "0 8 * * 1" -jobs: - dispatch_release_pr: - runs-on: ubuntu-latest - steps: - - name: Repository Dispatch - uses: peter-evans/repository-dispatch@5fc4efd1a4797ddb68ffd0714a238564e4cc0e6f - with: - event-type: batch-release-pr - client-payload: '{"package": "a_package"}' -'''); - } - - if (validReleaseFromBranches) { - workflowDir.childFile('release_from_branches.yml').writeAsStringSync( - ''' -on: - push: - branches: - - 'release-a_package-*' -''', - ); - } - - if (validSyncRelease) { - workflowDir.childFile('sync_release_pr.yml').writeAsStringSync(''' -on: - push: - branches: - - 'release-a_package-*' -'''); - } - } - - test( - 'ignores non-batch release packages if they have no artifacts', - () async { - setupReleaseStrategyTest(); - // No config, so batch is false by default. - - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [ - FakeProcessInfo( - MockProcess(exitCode: 1), - ), // git ls-remote fails (branch doesn't exist) - ]; - - final List output = await runCapturingPrint(runner, [ - 'validate', - ]); - - expect( - output, - containsAllInOrder([contains('No issues found!')]), - ); - }, - ); - - test('fails if non-batch package has batch artifacts', () async { - setupReleaseStrategyTest(); - // batch defaults to false - writeWorkflowFiles(); - - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [ - FakeProcessInfo( - MockProcess(), - ), // git ls-remote succeeds (branch exists) - ]; - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - contains( - contains( - 'Unexpected batch workflow file: .github/workflows/a_package_batch.yml', - ), - ), - ); - expect( - output, - contains( - contains( - 'Unexpected trigger for release-a_package-* in .github/workflows/release_from_branches.yml', - ), - ), - ); - expect( - output, - contains( - contains( - 'Unexpected trigger for release-a_package-* in .github/workflows/sync_release_pr.yml', - ), - ), - ); - }); - - test('fails if batch package has pre-release version', () async { - final RepositoryPackage package = setupReleaseStrategyTest(); - writeBatchConfig(package); - writeWorkflowFiles(); - package.pubspecFile.writeAsStringSync(''' -name: a_package -version: 1.0.0-wip -'''); - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [ - FakeProcessInfo(MockProcess()), // git ls-remote succeeds - ]; - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - contains( - contains( - 'Batch release packages must not have a pre-release version.', - ), - ), - ); - }); - - test('fails if batch workflow file is missing', () async { - final RepositoryPackage package = setupReleaseStrategyTest(); - writeBatchConfig(package); - // Don't write workflow files. - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - contains( - contains( - 'Missing batch workflow file: .github/workflows/a_package_batch.yml', - ), - ), - ); - }); - - test('fails if batch workflow content is invalid', () async { - final RepositoryPackage package = setupReleaseStrategyTest(); - writeBatchConfig(package); - final Directory workflowDir = root - .childDirectory('.github') - .childDirectory('workflows'); - workflowDir.createSync(recursive: true); - workflowDir.childFile('a_package_batch.yml').writeAsStringSync(''' -name: Batch Release -jobs: - dispatch_release_pr: - steps: - - uses: peter-evans/repository-dispatch@5fc4efd1a4797ddb68ffd0714a238564e4cc0e6f - with: - event-type: something-else - client-payload: '{"package": "a_package"}' -'''); - // Write other files to be valid so we focus on this error - workflowDir - .childFile('release_from_branches.yml') - .writeAsStringSync("- 'release-a_package-*'"); - workflowDir - .childFile('sync_release_pr.yml') - .writeAsStringSync("- 'release-a_package-*'"); - - // Mock successful git and gh calls - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [FakeProcessInfo(MockProcess())]; - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - contains( - contains( - 'Invalid batch workflow content in a_package_batch.yml. ' - 'Must contain a step using peter-evans/repository-dispatch with:\n' - ' event-type: batch-release-pr\n' - ' client-payload: \'{"package": "a_package"}\'', - ), - ), - ); - }); - - test('fails if global workflows are missing triggers', () async { - final RepositoryPackage package = setupReleaseStrategyTest(); - writeBatchConfig(package); - writeWorkflowFiles( - validReleaseFromBranches: false, - validSyncRelease: false, - ); - // Create files but without correct content - final Directory workflowDir = root - .childDirectory('.github') - .childDirectory('workflows'); - workflowDir - .childFile('release_from_branches.yml') - .writeAsStringSync('name: something'); - workflowDir - .childFile('sync_release_pr.yml') - .writeAsStringSync('name: something'); - - gitProcessRunner.mockProcessesForExecutable['git'] = [ - FakeProcessInfo(MockProcess()), - ]; - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - expect(commandError, isA()); - expect( - output, - contains( - contains( - 'Missing trigger for release-a_package-* in .github/workflows/release_from_branches.yml', - ), - ), - ); - expect( - output, - contains( - contains( - 'Missing trigger for release-a_package-* in .github/workflows/sync_release_pr.yml', - ), - ), - ); - }); - - test('passes if all checks pass', () async { - final RepositoryPackage package = setupReleaseStrategyTest(); - writeBatchConfig(package); - writeWorkflowFiles(); - - gitProcessRunner.mockProcessesForExecutable['git-ls-remote'] = - [FakeProcessInfo(MockProcess())]; - - Error? commandError; - final List output = await runCapturingPrint( - runner, - ['validate'], - errorHandler: (Error e) { - commandError = e; - }, - ); - - if (commandError != null) { - print('ERROR: Command failed in "passes if all checks pass"'); - print('Output:\n${output.join('\n')}'); - } - - expect(commandError, isNull); - expect( - output, - containsAllInOrder([contains('No issues found!')]), - ); - }); - }); - }); -} From 653a84747027bf0211b459366d7056c21b1c45e3 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 30 Apr 2026 10:14:44 -0400 Subject: [PATCH 05/18] Fold in gradle --- .ci/targets/repo_checks.yaml | 4 - script/tool/lib/src/gradle_check_command.dart | 44 --------- script/tool/lib/src/main.dart | 2 - script/tool/lib/src/validate_command.dart | 39 ++++---- ...dart => validate_command_gradle_test.dart} | 90 ++++++++++--------- 5 files changed, 69 insertions(+), 110 deletions(-) delete mode 100644 script/tool/lib/src/gradle_check_command.dart rename script/tool/test/{gradle_check_command_test.dart => validate_command_gradle_test.dart} (96%) diff --git a/.ci/targets/repo_checks.yaml b/.ci/targets/repo_checks.yaml index 20b68063066c..39d2f0d84407 100644 --- a/.ci/targets/repo_checks.yaml +++ b/.ci/targets/repo_checks.yaml @@ -48,10 +48,6 @@ tasks: script: .ci/scripts/tool_runner.sh args: ["update-excerpts", "--fail-on-change"] always: true - - name: Gradle validation - script: .ci/scripts/tool_runner.sh - args: ["gradle-check"] - always: true - name: Repository-level package metadata validation script: .ci/scripts/tool_runner.sh args: ["check-repo-package-info"] diff --git a/script/tool/lib/src/gradle_check_command.dart b/script/tool/lib/src/gradle_check_command.dart deleted file mode 100644 index a21493d9a598..000000000000 --- a/script/tool/lib/src/gradle_check_command.dart +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2013 The Flutter Authors -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'common/package_looping_command.dart'; -import 'common/repository_package.dart'; -import 'validators/gradle_validator.dart'; - -/// A command to enforce gradle file conventions and best practices. -class GradleCheckCommand extends PackageLoopingCommand { - /// Creates an instance of the gradle check command. - GradleCheckCommand(super.packagesDir, {super.gitDir}); - - @override - final String name = 'gradle-check'; - - @override - List get aliases => ['check-gradle']; - - @override - final String description = - 'Checks that gradle files follow repository conventions.'; - - @override - bool get hasLongOutput => false; - - @override - PackageLoopingType get packageLoopingType => - PackageLoopingType.includeAllSubpackages; - - @override - Future runForPackage(RepositoryPackage package) async { - if (!package.platformDirectory(FlutterPlatform.android).existsSync()) { - return PackageResult.skip('No android/ directory.'); - } - - final validator = GradleValidator(path: path, indentation: indentation); - final List errors = validator.validateGradle(package); - - // TODO(stuartmorgan): When combining this with other commands, use the - // errors. For now they are dropped to keep the existing behavior. - return errors.isEmpty ? PackageResult.success() : PackageResult.fail(); - } -} diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index 95351382055e..e9d95916638a 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -21,7 +21,6 @@ import 'fetch_deps_command.dart'; import 'firebase_test_lab_command.dart'; import 'fix_command.dart'; import 'format_command.dart'; -import 'gradle_check_command.dart'; import 'license_check_command.dart'; import 'list_command.dart'; import 'make_deps_path_based_command.dart'; @@ -74,7 +73,6 @@ void main(List args) { ..addCommand(FirebaseTestLabCommand(packagesDir)) ..addCommand(FixCommand(packagesDir)) ..addCommand(FormatCommand(packagesDir)) - ..addCommand(GradleCheckCommand(packagesDir)) ..addCommand(LicenseCheckCommand(packagesDir)) ..addCommand(ListCommand(packagesDir)) ..addCommand(MakeDepsPathBasedCommand(packagesDir)) diff --git a/script/tool/lib/src/validate_command.dart b/script/tool/lib/src/validate_command.dart index 988de4637442..729aaa3a38f2 100644 --- a/script/tool/lib/src/validate_command.dart +++ b/script/tool/lib/src/validate_command.dart @@ -8,6 +8,7 @@ import 'package:meta/meta.dart'; import 'common/package_looping_command.dart'; import 'common/repository_package.dart'; import 'validators/dependabot_validator.dart'; +import 'validators/gradle_validator.dart'; import 'validators/repo_info_validator.dart'; /// The set of possible validators. @@ -19,7 +20,8 @@ import 'validators/repo_info_validator.dart'; /// This is done instead of testing validators directly to ensure that testing /// includes things like command line parsing and run initialization. @visibleForTesting -enum Validator { dependabot, repoInfo } +// ignore: public_member_api_docs +enum Validator { dependabot, repoInfo, gradle } /// A command to validate that packages follow various team conventions, /// guidelines, and best practices. @@ -28,6 +30,7 @@ enum Validator { dependabot, repoInfo } /// - repository-level metadata about packages, such as repo README and /// auto-label entries /// - dependabot configuration coverage +/// - gradle configurations class ValidateCommand extends PackageLoopingCommand { /// Creates Dependabot check command instance. ValidateCommand(super.packagesDir, {this.targetedValidators, super.gitDir}); @@ -87,16 +90,12 @@ class ValidateCommand extends PackageLoopingCommand { @override Future runForPackage(RepositoryPackage package) async { - final List errors = []; - - if (package.isTopLevel) { - if (_shouldRun(Validator.repoInfo)) { - errors.addAll(await _validateRepoInfo(package)); - } - } - if (_shouldRun(Validator.dependabot)) { - errors.addAll(await _validateDependabot(package)); - } + final List errors = [ + if (_shouldRun(Validator.repoInfo)) ...await _validateRepoInfo(package), + if (_shouldRun(Validator.dependabot)) + ...await _validateDependabot(package), + if (_shouldRun(Validator.gradle)) ...await _validateGradle(package), + ]; return errors.isEmpty ? PackageResult.success() @@ -105,6 +104,10 @@ class ValidateCommand extends PackageLoopingCommand { /// Runs repo-level checks. Future> _validateRepoInfo(RepositoryPackage package) async { + // Repo-level checks only apply to top-level packages. + if (!package.isTopLevel) { + return []; + } final validator = RepoInfoValidator( readmeTableEntries: _readmeTableEntries, autoLabeledPackages: _autoLabeledPackages, @@ -121,14 +124,16 @@ class ValidateCommand extends PackageLoopingCommand { repoRoot: _repoRoot, indentation: indentation, ); + return validator.validateDependabotCoverage(package); + } - final errors = []; - - errors.addAll(validator.validateDependabotCoverage(package)); - - // TODO(stuartmorgan): Add other ecosystem checks here as more are enabled. + Future> _validateGradle(RepositoryPackage package) async { + if (!package.platformDirectory(FlutterPlatform.android).existsSync()) { + return []; + } - return errors; + final validator = GradleValidator(path: path, indentation: indentation); + return validator.validateGradle(package); } bool _shouldRun(Validator validator) => diff --git a/script/tool/test/gradle_check_command_test.dart b/script/tool/test/validate_command_gradle_test.dart similarity index 96% rename from script/tool/test/gradle_check_command_test.dart rename to script/tool/test/validate_command_gradle_test.dart index c038fa189015..cb5470ec8d83 100644 --- a/script/tool/test/gradle_check_command_test.dart +++ b/script/tool/test/validate_command_gradle_test.dart @@ -6,7 +6,7 @@ import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:flutter_plugin_tools/src/common/core.dart'; import 'package:flutter_plugin_tools/src/common/plugin_utils.dart'; -import 'package:flutter_plugin_tools/src/gradle_check_command.dart'; +import 'package:flutter_plugin_tools/src/validate_command.dart'; import 'package:flutter_plugin_tools/src/validators/gradle_validator.dart'; import 'package:git/git.dart'; import 'package:test/test.dart'; @@ -25,11 +25,15 @@ void main() { final GitDir gitDir; (:packagesDir, processRunner: _, gitProcessRunner: _, :gitDir) = configureBaseCommandMocks(); - final command = GradleCheckCommand(packagesDir, gitDir: gitDir); + final command = ValidateCommand( + packagesDir, + gitDir: gitDir, + targetedValidators: {Validator.gradle}, + ); runner = CommandRunner( - 'gradle_check_command', - 'Test for gradle_check_command', + 'validate_gradle_test', + 'Test for gradle validations', ); runner.addCommand(command); }); @@ -349,16 +353,16 @@ flutter { '''); } - test('skips when package has no Android directory', () async { + test('passes when package has no Android directory', () async { createFakePackage('a_package', packagesDir, examples: []); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( output, - containsAllInOrder([contains('Skipped 1 package(s)')]), + containsAllInOrder([contains('Running for a_package')]), ); }); @@ -376,7 +380,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -404,7 +408,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -438,7 +442,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -475,7 +479,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -508,7 +512,7 @@ flutter { writeFakeManifest(package); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -535,7 +539,7 @@ flutter { writeFakeManifest(package); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -557,7 +561,7 @@ flutter { writeFakeManifest(package); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -579,14 +583,14 @@ flutter { writeFakeManifest(example, isApp: true); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( output, containsAllInOrder([ + contains('Running for a_plugin/example'), contains('Validating android/build.gradle.kts'), - contains('Ran for 1 package(s)'), ]), ); }); @@ -608,7 +612,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -637,7 +641,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -664,7 +668,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -698,7 +702,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -729,7 +733,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -769,7 +773,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -803,7 +807,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -835,7 +839,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -878,7 +882,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -915,7 +919,7 @@ flutter { writeFakeManifest(example, isApp: true); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -948,7 +952,7 @@ flutter { writeFakeManifest(example, isApp: true); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -980,7 +984,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1016,7 +1020,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1050,7 +1054,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1092,7 +1096,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1121,7 +1125,7 @@ flutter { writeFakeManifest(example, isApp: true); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -1147,7 +1151,7 @@ flutter { writeFakeManifest(example, isApp: true); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -1173,7 +1177,7 @@ flutter { writeFakeManifest(example, isApp: true); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -1201,7 +1205,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1244,7 +1248,7 @@ flutter { writeFakeManifest(example, isApp: true); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -1277,7 +1281,7 @@ flutter { writeFakeManifest(example, isApp: true); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -1314,7 +1318,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1356,7 +1360,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1393,7 +1397,7 @@ flutter { writeFakeManifest(package); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -1418,7 +1422,7 @@ flutter { writeFakeManifest(package); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -1443,7 +1447,7 @@ flutter { writeFakeManifest(package); final List output = await runCapturingPrint(runner, [ - 'gradle-check', + 'validate', ]); expect( @@ -1470,7 +1474,7 @@ flutter { Error? commandError; final List output = await runCapturingPrint( runner, - ['gradle-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, From 73be71147045536831bf4afc5ad8be739c4614d8 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 30 Apr 2026 10:26:50 -0400 Subject: [PATCH 06/18] Remove missed repo check invocation --- .ci/targets/repo_checks.yaml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.ci/targets/repo_checks.yaml b/.ci/targets/repo_checks.yaml index 39d2f0d84407..0881e5012eb1 100644 --- a/.ci/targets/repo_checks.yaml +++ b/.ci/targets/repo_checks.yaml @@ -48,10 +48,6 @@ tasks: script: .ci/scripts/tool_runner.sh args: ["update-excerpts", "--fail-on-change"] always: true - - name: Repository-level package metadata validation - script: .ci/scripts/tool_runner.sh - args: ["check-repo-package-info"] - always: true - name: Repository standards validation script: .ci/scripts/tool_runner.sh args: ["validate"] From 25b96b7cc9372f29fec987ace5598f3740622536 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 30 Apr 2026 13:10:37 -0400 Subject: [PATCH 07/18] Change pubspec check options to config files instead of command line --- .ci/targets/repo_checks.yaml | 6 +- script/tool/lib/src/common/core.dart | 4 + script/tool/lib/src/common/file_utils.dart | 12 ++ .../tool/lib/src/common/package_command.dart | 8 +- .../tool/lib/src/pubspec_check_command.dart | 103 +++++++++++------- script/tool/lib/src/validate_command.dart | 1 + .../tool/test/pubspec_check_command_test.dart | 65 +++++++---- .../allowed_pinned_dependencies.yaml | 0 .../allowed_unpinned_dependencies.yaml | 0 tool_config/min_version.yaml | 5 + 10 files changed, 135 insertions(+), 69 deletions(-) rename script/configs/allowed_pinned_deps.yaml => tool_config/allowed_pinned_dependencies.yaml (100%) rename script/configs/allowed_unpinned_deps.yaml => tool_config/allowed_unpinned_dependencies.yaml (100%) create mode 100644 tool_config/min_version.yaml diff --git a/.ci/targets/repo_checks.yaml b/.ci/targets/repo_checks.yaml index 0881e5012eb1..96449adb2bb9 100644 --- a/.ci/targets/repo_checks.yaml +++ b/.ci/targets/repo_checks.yaml @@ -26,11 +26,7 @@ tasks: # in legacy version analysis (.ci.yaml analyze_legacy). - name: pubspec validation script: .ci/scripts/tool_runner.sh - args: - - "pubspec-check" - - "--min-min-flutter-version=3.35.0" - - "--allow-dependencies=script/configs/allowed_unpinned_deps.yaml" - - "--allow-pinned-dependencies=script/configs/allowed_pinned_deps.yaml" + args: ["pubspec-check"] always: true - name: README validation script: .ci/scripts/tool_runner.sh diff --git a/script/tool/lib/src/common/core.dart b/script/tool/lib/src/common/core.dart index bea937c550d1..e7c17d2e514d 100644 --- a/script/tool/lib/src/common/core.dart +++ b/script/tool/lib/src/common/core.dart @@ -151,3 +151,7 @@ Directory? ciLogsDirectory(Platform platform, FileSystem fileSystem) { } return logsDirectory; } + +/// The directory that contains repo-specific configuration for this tooling. +Directory toolConfigDirectory(Directory repoRoot) => + repoRoot.childDirectory('tool_config'); diff --git a/script/tool/lib/src/common/file_utils.dart b/script/tool/lib/src/common/file_utils.dart index cbd88bf89588..e9939d363c30 100644 --- a/script/tool/lib/src/common/file_utils.dart +++ b/script/tool/lib/src/common/file_utils.dart @@ -4,6 +4,7 @@ import 'package:file/file.dart'; import 'package:path/path.dart' as p; +import 'package:yaml/yaml.dart'; /// Returns a [File] created by appending all but the last item in [components] /// to [base] as subdirectories, then appending the last as a file. @@ -49,3 +50,14 @@ String relativePosixPath( platformContext.relative(entity.absolute.path, from: from.path), ), ); + +/// Loads the file at [filename], which must contain a YAML list of strings. +/// +/// If the file is not found, returns null. +List? loadYamlList(File file) { + if (!file.existsSync()) { + return null; + } + final yaml = loadYaml(file.readAsStringSync()) as YamlList; + return yaml.toList().cast(); +} diff --git a/script/tool/lib/src/common/package_command.dart b/script/tool/lib/src/common/package_command.dart index 838256acb7e0..f85c02db8886 100644 --- a/script/tool/lib/src/common/package_command.dart +++ b/script/tool/lib/src/common/package_command.dart @@ -10,9 +10,9 @@ 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 'file_utils.dart'; import 'git_version_finder.dart'; import 'output_utils.dart'; import 'process_runner.dart'; @@ -286,11 +286,11 @@ abstract class PackageCommand extends Command { return getStringListArg(key).expand((String item) { if (item.endsWith('.yaml')) { final File file = packagesDir.fileSystem.file(item); - final Object? yaml = loadYaml(file.readAsStringSync()); - if (yaml == null) { + final List? list = loadYamlList(file); + if (list == null) { return const []; } - return (yaml as YamlList).toList().cast(); + return list; } return [item]; }).toSet(); diff --git a/script/tool/lib/src/pubspec_check_command.dart b/script/tool/lib/src/pubspec_check_command.dart index 016e89b6954c..a8101d5430ec 100644 --- a/script/tool/lib/src/pubspec_check_command.dart +++ b/script/tool/lib/src/pubspec_check_command.dart @@ -4,12 +4,24 @@ import 'package:file/file.dart'; import 'package:path/path.dart' as p; +import 'package:yaml/yaml.dart'; +import 'common/core.dart'; +import 'common/file_utils.dart'; import 'common/output_utils.dart'; import 'common/package_looping_command.dart'; import 'common/repository_package.dart'; import 'validators/pubspec_validator.dart'; +// Config file names. +const String _versionConfigFileName = 'min_version.yaml'; +const String _allowedPinnedDependenciesFileName = + 'allowed_pinned_dependencies.yaml'; +const String _allowedUnpinnedDependenciesFileName = + 'allowed_unpinned_dependencies.yaml'; + +const int _exitCodeVersionConfigIssue = 3; + /// A command to enforce pubspec conventions across the repository. /// /// This both ensures that repo best practices for which optional fields are @@ -22,36 +34,7 @@ class PubspecCheckCommand extends PackageLoopingCommand { super.processRunner, super.platform, super.gitDir, - }) { - argParser.addOption( - _minMinFlutterVersionFlag, - help: - 'The minimum Flutter version to allow as the minimum SDK constraint.', - ); - argParser.addMultiOption( - _allowDependenciesFlag, - help: - 'Packages (comma separated) that are allowed as dependencies or ' - 'dev_dependencies.\n\n' - 'Alternately, a list of one or more YAML files that contain a list ' - 'of allowed dependencies.', - defaultsTo: [], - ); - argParser.addMultiOption( - _allowPinnedDependenciesFlag, - help: - 'Packages (comma separated) that are allowed as dependencies or ' - 'dev_dependencies only if pinned to an exact version.\n\n' - 'Alternately, a list of one or more YAML files that contain a list ' - 'of allowed pinned dependencies.', - defaultsTo: [], - ); - } - - static const String _minMinFlutterVersionFlag = 'min-min-flutter-version'; - static const String _allowDependenciesFlag = 'allow-dependencies'; - static const String _allowPinnedDependenciesFlag = - 'allow-pinned-dependencies'; + }); // The names of all published packages in the repository. final AllowPackageLists _allowedPackages = ( @@ -60,6 +43,8 @@ class PubspecCheckCommand extends PackageLoopingCommand { unpinned: {}, ); + late final String _minMinFlutterVersion; + @override final String name = 'pubspec-check'; @@ -79,13 +64,8 @@ class PubspecCheckCommand extends PackageLoopingCommand { @override Future initializeRun() async { - // Find all local, published packages. - _allowedPackages.local.addAll(await _findAllPublishedPackages().toList()); - // Load explicitly allowed packages. - _allowedPackages.unpinned.addAll(getYamlListArg(_allowDependenciesFlag)); - _allowedPackages.pinned.addAll( - getYamlListArg(_allowPinnedDependenciesFlag), - ); + await _loadAllowedDependencies(); + _minMinFlutterVersion = await _loadMinMinFlutterVersion(); } @override @@ -96,7 +76,7 @@ class PubspecCheckCommand extends PackageLoopingCommand { warningLogger: printWarning, allowedPackages: _allowedPackages, repoRoot: packagesDir.parent, - minMinFlutterVersion: getStringArg(_minMinFlutterVersionFlag), + minMinFlutterVersion: _minMinFlutterVersion, ); final List errors = await validator.validatePubspec(package); return errors.isEmpty @@ -120,6 +100,53 @@ class PubspecCheckCommand extends PackageLoopingCommand { } } + Future _loadAllowedDependencies() async { + final Directory repoRoot = packagesDir.fileSystem.directory( + (await gitDir).path, + ); + final Directory toolConfigDir = toolConfigDirectory(repoRoot); + + // Find all local, published packages. + _allowedPackages.local.addAll(await _findAllPublishedPackages().toList()); + // Load explicitly allowed packages. + _allowedPackages.unpinned.addAll( + loadYamlList( + toolConfigDir.childFile(_allowedUnpinnedDependenciesFileName), + ) ?? + [], + ); + _allowedPackages.pinned.addAll( + loadYamlList( + toolConfigDir.childFile(_allowedPinnedDependenciesFileName), + ) ?? + [], + ); + } + + Future _loadMinMinFlutterVersion() async { + final Directory repoRoot = packagesDir.fileSystem.directory( + (await gitDir).path, + ); + final File versionConfig = toolConfigDirectory( + repoRoot, + ).childFile(_versionConfigFileName); + if (!versionConfig.existsSync()) { + printError( + 'Minimum version configuration file not found at $_versionConfigFileName', + ); + return ''; + } + const minFlutterKey = 'min_flutter'; + final config = loadYaml(versionConfig.readAsStringSync()) as YamlMap?; + if (config == null || config[minFlutterKey] == null) { + printError( + '$_versionConfigFileName must be a map containing a "$minFlutterKey" entry', + ); + throw ToolExit(_exitCodeVersionConfigIssue); + } + return (config[minFlutterKey] as String).trim(); + } + Pubspec? _tryParsePubspec(String pubspecContents) { try { return Pubspec.parse(pubspecContents); diff --git a/script/tool/lib/src/validate_command.dart b/script/tool/lib/src/validate_command.dart index 729aaa3a38f2..e72171a2f116 100644 --- a/script/tool/lib/src/validate_command.dart +++ b/script/tool/lib/src/validate_command.dart @@ -29,6 +29,7 @@ enum Validator { dependabot, repoInfo, gradle } /// This includes: /// - repository-level metadata about packages, such as repo README and /// auto-label entries +/// - pubspec format and contents /// - dependabot configuration coverage /// - gradle configurations class ValidateCommand extends PackageLoopingCommand { diff --git a/script/tool/test/pubspec_check_command_test.dart b/script/tool/test/pubspec_check_command_test.dart index 4a84727b9132..989bd84d6899 100644 --- a/script/tool/test/pubspec_check_command_test.dart +++ b/script/tool/test/pubspec_check_command_test.dart @@ -144,6 +144,7 @@ void main() { late CommandRunner runner; late MockPlatform mockPlatform; late Directory packagesDir; + late Directory toolConfigDir; setUp(() { mockPlatform = MockPlatform(); @@ -151,6 +152,9 @@ void main() { final GitDir gitDir; (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); + toolConfigDir = packagesDir.parent.childDirectory('tool_config'); + toolConfigDir.createSync(recursive: true); + final command = PubspecCheckCommand( packagesDir, processRunner: processRunner, @@ -165,6 +169,30 @@ void main() { runner.addCommand(command); }); + void setVersionConfig({required String minFlutterVersion}) { + toolConfigDir + .childFile('min_version.yaml') + .writeAsStringSync('min_flutter: "$minFlutterVersion"'); + } + + void setAllowedDependencies({ + Iterable? pinned, + Iterable? unpinned, + }) { + if (pinned != null) { + toolConfigDir + .childFile('allowed_pinned_dependencies.yaml') + .writeAsStringSync(pinned.map((String dep) => '- $dep').join('\n')); + } + if (unpinned != null) { + toolConfigDir + .childFile('allowed_unpinned_dependencies.yaml') + .writeAsStringSync( + unpinned.map((String dep) => '- $dep').join('\n'), + ); + } + } + test('passes for a plugin following conventions', () async { final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir); @@ -1545,11 +1573,12 @@ ${_environmentSection(flutterConstraint: '>=2.10.0')} ${_dependenciesSection()} ${_topicsSection()} '''); + setVersionConfig(minFlutterVersion: '3.0.0'); Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check', '--min-min-flutter-version', '3.0.0'], + ['pubspec-check'], errorHandler: (Error e) { commandError = e; }, @@ -1583,11 +1612,10 @@ ${_environmentSection(flutterConstraint: '>=3.3.0', dartConstraint: '>=2.18.0 <4 ${_dependenciesSection()} ${_topicsSection()} '''); + setVersionConfig(minFlutterVersion: '3.3.0'); final List output = await runCapturingPrint(runner, [ 'pubspec-check', - '--min-min-flutter-version', - '3.3.0', ]); expect( @@ -1616,11 +1644,10 @@ ${_environmentSection(flutterConstraint: '>=3.7.0', dartConstraint: '>=2.19.0 <4 ${_dependenciesSection()} ${_topicsSection()} '''); + setVersionConfig(minFlutterVersion: '3.3.0'); final List output = await runCapturingPrint(runner, [ 'pubspec-check', - '--min-min-flutter-version', - '3.3.0', ]); expect( @@ -1648,11 +1675,12 @@ ${_environmentSection(dartConstraint: '>=2.14.0 <4.0.0', flutterConstraint: null ${_dependenciesSection()} ${_topicsSection()} '''); + setVersionConfig(minFlutterVersion: '3.0.0'); Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check', '--min-min-flutter-version', '3.0.0'], + ['pubspec-check'], errorHandler: (Error e) { commandError = e; }, @@ -1684,11 +1712,10 @@ ${_environmentSection(dartConstraint: '>=2.18.0 <4.0.0', flutterConstraint: null ${_dependenciesSection()} ${_topicsSection()} '''); + setVersionConfig(minFlutterVersion: '3.3.0'); final List output = await runCapturingPrint(runner, [ 'pubspec-check', - '--min-min-flutter-version', - '3.3.0', ]); expect( @@ -1717,11 +1744,10 @@ ${_environmentSection(dartConstraint: '>=2.18.0 <4.0.0', flutterConstraint: null ${_dependenciesSection()} ${_topicsSection()} '''); + setVersionConfig(minFlutterVersion: '3.0.0'); final List output = await runCapturingPrint(runner, [ 'pubspec-check', - '--min-min-flutter-version', - '3.0.0', ]); expect( @@ -1747,11 +1773,12 @@ ${_environmentSection()} ${_dependenciesSection()} ${_topicsSection()} '''); + setVersionConfig(minFlutterVersion: '2.0.0'); Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check', '--min-min-flutter-version', '2.0.0'], + ['pubspec-check'], errorHandler: (Error e) { commandError = e; }, @@ -1929,11 +1956,10 @@ ${_environmentSection()} ${_dependenciesSection(['allowed: ^1.0.0'])} ${_topicsSection()} '''); + setAllowedDependencies(unpinned: ['allowed']); final List output = await runCapturingPrint(runner, [ 'pubspec-check', - '--allow-dependencies', - 'allowed', ]); expect( @@ -1959,11 +1985,10 @@ ${_environmentSection()} ${_dependenciesSection(['allow_pinned: 1.0.0'])} ${_topicsSection()} '''); + setAllowedDependencies(pinned: ['allow_pinned']); final List output = await runCapturingPrint(runner, [ 'pubspec-check', - '--allow-pinned-dependencies', - 'allow_pinned', ]); expect( @@ -1990,11 +2015,10 @@ ${_environmentSection()} ${_dependenciesSection(['allow_pinned: ">=1.0.0 <=1.3.1"'])} ${_topicsSection()} '''); + setAllowedDependencies(pinned: ['allow_pinned']); final List output = await runCapturingPrint(runner, [ 'pubspec-check', - '--allow-pinned-dependencies', - 'allow_pinned', ]); expect( @@ -2019,15 +2043,12 @@ ${_environmentSection()} ${_dependenciesSection(['allow_pinned: ^1.0.0'])} ${_topicsSection()} '''); + setAllowedDependencies(pinned: ['allow_pinned']); Error? commandError; final List output = await runCapturingPrint( runner, - [ - 'pubspec-check', - '--allow-pinned-dependencies', - 'allow_pinned', - ], + ['pubspec-check'], errorHandler: (Error e) { commandError = e; }, diff --git a/script/configs/allowed_pinned_deps.yaml b/tool_config/allowed_pinned_dependencies.yaml similarity index 100% rename from script/configs/allowed_pinned_deps.yaml rename to tool_config/allowed_pinned_dependencies.yaml diff --git a/script/configs/allowed_unpinned_deps.yaml b/tool_config/allowed_unpinned_dependencies.yaml similarity index 100% rename from script/configs/allowed_unpinned_deps.yaml rename to tool_config/allowed_unpinned_dependencies.yaml diff --git a/tool_config/min_version.yaml b/tool_config/min_version.yaml new file mode 100644 index 000000000000..ce92e9515f5c --- /dev/null +++ b/tool_config/min_version.yaml @@ -0,0 +1,5 @@ +# The minimum Flutter version that can be used as the minimum SDK constraints +# by packages in this repository. For example, if this is set to 3.22, then a +# package can have a minimum SDK version of 3.32, 3.35, 3.38, etc, but +# not 3.29. +min_flutter: '3.35.0' From f272dc5cfa2a48521150350b987ccc7aab742f85 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 30 Apr 2026 13:41:14 -0400 Subject: [PATCH 08/18] Fold in pubspec --- .ci/targets/repo_checks.yaml | 6 - script/tool/lib/src/main.dart | 2 - .../tool/lib/src/pubspec_check_command.dart | 158 ------------------ script/tool/lib/src/validate_command.dart | 122 +++++++++++++- ...art => validate_command_pubspec_test.dart} | 128 +++++++------- tool_config/min_version.yaml | 3 + 6 files changed, 185 insertions(+), 234 deletions(-) delete mode 100644 script/tool/lib/src/pubspec_check_command.dart rename script/tool/test/{pubspec_check_command_test.dart => validate_command_pubspec_test.dart} (96%) diff --git a/.ci/targets/repo_checks.yaml b/.ci/targets/repo_checks.yaml index 96449adb2bb9..5d5b71d0a92c 100644 --- a/.ci/targets/repo_checks.yaml +++ b/.ci/targets/repo_checks.yaml @@ -22,12 +22,6 @@ tasks: script: .ci/scripts/tool_runner.sh args: ["license-check"] always: true - # The major and minor version here should match the lowest version analyzed - # in legacy version analysis (.ci.yaml analyze_legacy). - - name: pubspec validation - script: .ci/scripts/tool_runner.sh - args: ["pubspec-check"] - always: true - name: README validation script: .ci/scripts/tool_runner.sh args: ["readme-check"] diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index e9d95916638a..4dc4187bc6ec 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -28,7 +28,6 @@ import 'native_test_command.dart'; import 'podspec_check_command.dart'; import 'publish_check_command.dart'; import 'publish_command.dart'; -import 'pubspec_check_command.dart'; import 'readme_check_command.dart'; import 'remove_dev_dependencies_command.dart'; import 'update_dependency_command.dart'; @@ -80,7 +79,6 @@ void main(List args) { ..addCommand(PodspecCheckCommand(packagesDir)) ..addCommand(PublishCheckCommand(packagesDir)) ..addCommand(PublishCommand(packagesDir)) - ..addCommand(PubspecCheckCommand(packagesDir)) ..addCommand(ReadmeCheckCommand(packagesDir)) ..addCommand(RemoveDevDependenciesCommand(packagesDir)) ..addCommand(UpdateDependencyCommand(packagesDir)) diff --git a/script/tool/lib/src/pubspec_check_command.dart b/script/tool/lib/src/pubspec_check_command.dart deleted file mode 100644 index a8101d5430ec..000000000000 --- a/script/tool/lib/src/pubspec_check_command.dart +++ /dev/null @@ -1,158 +0,0 @@ -// Copyright 2013 The Flutter Authors -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:file/file.dart'; -import 'package:path/path.dart' as p; -import 'package:yaml/yaml.dart'; - -import 'common/core.dart'; -import 'common/file_utils.dart'; -import 'common/output_utils.dart'; -import 'common/package_looping_command.dart'; -import 'common/repository_package.dart'; -import 'validators/pubspec_validator.dart'; - -// Config file names. -const String _versionConfigFileName = 'min_version.yaml'; -const String _allowedPinnedDependenciesFileName = - 'allowed_pinned_dependencies.yaml'; -const String _allowedUnpinnedDependenciesFileName = - 'allowed_unpinned_dependencies.yaml'; - -const int _exitCodeVersionConfigIssue = 3; - -/// A command to enforce pubspec conventions across the repository. -/// -/// This both ensures that repo best practices for which optional fields are -/// used are followed, and that the structure is consistent to make edits -/// across multiple pubspec files easier. -class PubspecCheckCommand extends PackageLoopingCommand { - /// Creates an instance of the version check command. - PubspecCheckCommand( - super.packagesDir, { - super.processRunner, - super.platform, - super.gitDir, - }); - - // The names of all published packages in the repository. - final AllowPackageLists _allowedPackages = ( - local: {}, - pinned: {}, - unpinned: {}, - ); - - late final String _minMinFlutterVersion; - - @override - final String name = 'pubspec-check'; - - @override - List get aliases => ['check-pubspec']; - - @override - final String description = - 'Checks that pubspecs follow repository conventions.'; - - @override - bool get hasLongOutput => false; - - @override - PackageLoopingType get packageLoopingType => - PackageLoopingType.includeAllSubpackages; - - @override - Future initializeRun() async { - await _loadAllowedDependencies(); - _minMinFlutterVersion = await _loadMinMinFlutterVersion(); - } - - @override - Future runForPackage(RepositoryPackage package) async { - final validator = PubspecValidator( - path: path, - indentation: indentation, - warningLogger: printWarning, - allowedPackages: _allowedPackages, - repoRoot: packagesDir.parent, - minMinFlutterVersion: _minMinFlutterVersion, - ); - final List errors = await validator.validatePubspec(package); - return errors.isEmpty - ? PackageResult.success() - : PackageResult.fail(errors); - } - - Stream _findAllPublishedPackages() async* { - for (final File pubspecFile - in (await packagesDir.parent - .list(recursive: true, followLinks: false) - .toList()) - .whereType() - .where( - (File entity) => p.basename(entity.path) == 'pubspec.yaml', - )) { - final Pubspec? pubspec = _tryParsePubspec(pubspecFile.readAsStringSync()); - if (pubspec != null && pubspec.publishTo != 'none') { - yield pubspec.name; - } - } - } - - Future _loadAllowedDependencies() async { - final Directory repoRoot = packagesDir.fileSystem.directory( - (await gitDir).path, - ); - final Directory toolConfigDir = toolConfigDirectory(repoRoot); - - // Find all local, published packages. - _allowedPackages.local.addAll(await _findAllPublishedPackages().toList()); - // Load explicitly allowed packages. - _allowedPackages.unpinned.addAll( - loadYamlList( - toolConfigDir.childFile(_allowedUnpinnedDependenciesFileName), - ) ?? - [], - ); - _allowedPackages.pinned.addAll( - loadYamlList( - toolConfigDir.childFile(_allowedPinnedDependenciesFileName), - ) ?? - [], - ); - } - - Future _loadMinMinFlutterVersion() async { - final Directory repoRoot = packagesDir.fileSystem.directory( - (await gitDir).path, - ); - final File versionConfig = toolConfigDirectory( - repoRoot, - ).childFile(_versionConfigFileName); - if (!versionConfig.existsSync()) { - printError( - 'Minimum version configuration file not found at $_versionConfigFileName', - ); - return ''; - } - const minFlutterKey = 'min_flutter'; - final config = loadYaml(versionConfig.readAsStringSync()) as YamlMap?; - if (config == null || config[minFlutterKey] == null) { - printError( - '$_versionConfigFileName must be a map containing a "$minFlutterKey" entry', - ); - throw ToolExit(_exitCodeVersionConfigIssue); - } - return (config[minFlutterKey] as String).trim(); - } - - Pubspec? _tryParsePubspec(String pubspecContents) { - try { - return Pubspec.parse(pubspecContents); - } on Exception catch (exception) { - print(' Cannot parse pubspec.yaml: $exception'); - } - return null; - } -} diff --git a/script/tool/lib/src/validate_command.dart b/script/tool/lib/src/validate_command.dart index e72171a2f116..b6be97c90ae5 100644 --- a/script/tool/lib/src/validate_command.dart +++ b/script/tool/lib/src/validate_command.dart @@ -4,11 +4,17 @@ import 'package:file/file.dart'; import 'package:meta/meta.dart'; +import 'package:path/path.dart' as p; +import 'package:yaml/yaml.dart'; +import 'common/core.dart'; +import 'common/file_utils.dart'; +import 'common/output_utils.dart'; import 'common/package_looping_command.dart'; import 'common/repository_package.dart'; import 'validators/dependabot_validator.dart'; import 'validators/gradle_validator.dart'; +import 'validators/pubspec_validator.dart'; import 'validators/repo_info_validator.dart'; /// The set of possible validators. @@ -21,7 +27,16 @@ import 'validators/repo_info_validator.dart'; /// includes things like command line parsing and run initialization. @visibleForTesting // ignore: public_member_api_docs -enum Validator { dependabot, repoInfo, gradle } +enum Validator { dependabot, repoInfo, gradle, pubspec } + +// Config file names. +const String _versionConfigFileName = 'min_version.yaml'; +const String _allowedPinnedDependenciesFileName = + 'allowed_pinned_dependencies.yaml'; +const String _allowedUnpinnedDependenciesFileName = + 'allowed_unpinned_dependencies.yaml'; + +const int _exitCodeVersionConfigIssue = 3; /// A command to validate that packages follow various team conventions, /// guidelines, and best practices. @@ -34,7 +49,13 @@ enum Validator { dependabot, repoInfo, gradle } /// - gradle configurations class ValidateCommand extends PackageLoopingCommand { /// Creates Dependabot check command instance. - ValidateCommand(super.packagesDir, {this.targetedValidators, super.gitDir}); + ValidateCommand( + super.packagesDir, { + this.targetedValidators, + super.processRunner, + super.platform, + super.gitDir, + }); /// The validators to run. /// @@ -50,9 +71,19 @@ class ValidateCommand extends PackageLoopingCommand { /// Packages with entries in labeler.yml. final Set _autoLabeledPackages = {}; - // The set of directories covered by the repo's Dependabot configuration. + /// The set of directories covered by the repo's Dependabot configuration. late DependabotCoverage _dependabotCoverage; + /// The set of packages that are allowed as dependencies. + final AllowPackageLists _allowedPackages = ( + local: {}, + pinned: {}, + unpinned: {}, + ); + + /// The minimum version of Flutter that is allowed for any package. + late final String _minMinFlutterVersion; + @override final String name = 'validate'; @@ -84,6 +115,10 @@ class ValidateCommand extends PackageLoopingCommand { RepoInfoValidator.loadAutoLabeledPackages(repoRoot: _repoRoot), ); } + if (_shouldRun(Validator.pubspec)) { + await _loadAllowedDependencies(); + _minMinFlutterVersion = await _loadMinMinFlutterVersion(); + } if (_shouldRun(Validator.dependabot)) { _dependabotCoverage = DependabotValidator.loadConfig(repoRoot: _repoRoot); } @@ -93,6 +128,7 @@ class ValidateCommand extends PackageLoopingCommand { Future runForPackage(RepositoryPackage package) async { final List errors = [ if (_shouldRun(Validator.repoInfo)) ...await _validateRepoInfo(package), + if (_shouldRun(Validator.pubspec)) ...await _validatePubspec(package), if (_shouldRun(Validator.dependabot)) ...await _validateDependabot(package), if (_shouldRun(Validator.gradle)) ...await _validateGradle(package), @@ -103,6 +139,9 @@ class ValidateCommand extends PackageLoopingCommand { : PackageResult.fail(errors); } + bool _shouldRun(Validator validator) => + targetedValidators?.contains(validator) ?? true; + /// Runs repo-level checks. Future> _validateRepoInfo(RepositoryPackage package) async { // Repo-level checks only apply to top-level packages. @@ -137,6 +176,79 @@ class ValidateCommand extends PackageLoopingCommand { return validator.validateGradle(package); } - bool _shouldRun(Validator validator) => - targetedValidators?.contains(validator) ?? true; + Future> _validatePubspec(RepositoryPackage package) async { + final validator = PubspecValidator( + path: path, + indentation: indentation, + warningLogger: printWarning, + allowedPackages: _allowedPackages, + repoRoot: packagesDir.parent, + minMinFlutterVersion: _minMinFlutterVersion, + ); + return validator.validatePubspec(package); + } + + Stream _findAllPublishedPackages() async* { + for (final File pubspecFile + in (await _repoRoot.list(recursive: true, followLinks: false).toList()) + .whereType() + .where( + (File entity) => p.basename(entity.path) == 'pubspec.yaml', + )) { + final Pubspec? pubspec = _tryParsePubspec(pubspecFile.readAsStringSync()); + if (pubspec != null && pubspec.publishTo != 'none') { + yield pubspec.name; + } + } + } + + Future _loadAllowedDependencies() async { + final Directory toolConfigDir = toolConfigDirectory(_repoRoot); + + // Find all local, published packages. + _allowedPackages.local.addAll(await _findAllPublishedPackages().toList()); + // Load explicitly allowed packages. + _allowedPackages.unpinned.addAll( + loadYamlList( + toolConfigDir.childFile(_allowedUnpinnedDependenciesFileName), + ) ?? + [], + ); + _allowedPackages.pinned.addAll( + loadYamlList( + toolConfigDir.childFile(_allowedPinnedDependenciesFileName), + ) ?? + [], + ); + } + + Future _loadMinMinFlutterVersion() async { + final File versionConfig = toolConfigDirectory( + _repoRoot, + ).childFile(_versionConfigFileName); + if (!versionConfig.existsSync()) { + printError( + 'Minimum version configuration file not found at $_versionConfigFileName', + ); + return ''; + } + const minFlutterKey = 'min_flutter'; + final config = loadYaml(versionConfig.readAsStringSync()) as YamlMap?; + if (config == null || config[minFlutterKey] == null) { + printError( + '$_versionConfigFileName must be a map containing a "$minFlutterKey" entry', + ); + throw ToolExit(_exitCodeVersionConfigIssue); + } + return (config[minFlutterKey] as String).trim(); + } + + Pubspec? _tryParsePubspec(String pubspecContents) { + try { + return Pubspec.parse(pubspecContents); + } on Exception catch (exception) { + print(' Cannot parse pubspec.yaml: $exception'); + } + return null; + } } diff --git a/script/tool/test/pubspec_check_command_test.dart b/script/tool/test/validate_command_pubspec_test.dart similarity index 96% rename from script/tool/test/pubspec_check_command_test.dart rename to script/tool/test/validate_command_pubspec_test.dart index 989bd84d6899..64b26fbc9859 100644 --- a/script/tool/test/pubspec_check_command_test.dart +++ b/script/tool/test/validate_command_pubspec_test.dart @@ -5,7 +5,7 @@ import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:flutter_plugin_tools/src/common/core.dart'; -import 'package:flutter_plugin_tools/src/pubspec_check_command.dart'; +import 'package:flutter_plugin_tools/src/validate_command.dart'; import 'package:git/git.dart'; import 'package:test/test.dart'; @@ -154,17 +154,18 @@ void main() { configureBaseCommandMocks(platform: mockPlatform); toolConfigDir = packagesDir.parent.childDirectory('tool_config'); toolConfigDir.createSync(recursive: true); - - final command = PubspecCheckCommand( + + final command = ValidateCommand( packagesDir, processRunner: processRunner, platform: mockPlatform, gitDir: gitDir, + targetedValidators: {Validator.pubspec}, ); runner = CommandRunner( - 'pubspec_check_command', - 'Test for pubspec_check_command', + 'validate_pubspec_test', + 'Test for pubspec validation', ); runner.addCommand(command); }); @@ -214,7 +215,7 @@ ${_flutterSection()} '''); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -251,7 +252,7 @@ ${_flutterSection()} '''); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -279,7 +280,7 @@ ${_topicsSection()} '''); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -309,7 +310,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -344,7 +345,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -375,7 +376,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -410,7 +411,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -443,7 +444,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -479,7 +480,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -515,7 +516,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -548,7 +549,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -586,7 +587,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -629,7 +630,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -665,7 +666,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -699,7 +700,7 @@ ${_topicsSection([])} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -735,7 +736,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -773,7 +774,7 @@ ${_topicsSection(['plugin a'])} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -807,7 +808,7 @@ ${_topicsSection(['plugin--a'])} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -841,7 +842,7 @@ ${_topicsSection(['1plugin-a'])} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -875,7 +876,7 @@ ${_topicsSection(['plugin-A'])} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -909,7 +910,7 @@ ${_topicsSection(['plugin-a', 'plugin-a', 'plugin-a', 'plugin-a', 'plugi Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -945,7 +946,7 @@ ${_topicsSection(['foobarfoobarfoobarfoobarfoobarfoobarfoo'])} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -981,7 +982,7 @@ ${_topicsSection(['a'])} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1015,7 +1016,7 @@ ${_topicsSection(['plugin-'])} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1049,7 +1050,7 @@ ${_topicsSection(['plugin-A', 'Plugin-b'])} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1087,7 +1088,7 @@ ${_environmentSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1122,7 +1123,7 @@ ${_devDependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1157,7 +1158,7 @@ ${_dependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1188,7 +1189,7 @@ ${_dependenciesSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1225,7 +1226,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1263,7 +1264,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1300,7 +1301,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1336,7 +1337,7 @@ ${_topicsSection(['plugin-a'])} '''); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -1369,7 +1370,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1409,7 +1410,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1445,7 +1446,7 @@ ${_topicsSection(['plugin-a'])} '''); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -1476,7 +1477,7 @@ ${_topicsSection(['plugin-a'])} '''); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -1509,7 +1510,7 @@ ${_environmentSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1545,7 +1546,7 @@ ${_devDependenciesSection()} '''); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -1578,7 +1579,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1615,7 +1616,7 @@ ${_topicsSection()} setVersionConfig(minFlutterVersion: '3.3.0'); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -1647,7 +1648,7 @@ ${_topicsSection()} setVersionConfig(minFlutterVersion: '3.3.0'); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -1680,7 +1681,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1715,7 +1716,7 @@ ${_topicsSection()} setVersionConfig(minFlutterVersion: '3.3.0'); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -1747,7 +1748,7 @@ ${_topicsSection()} setVersionConfig(minFlutterVersion: '3.0.0'); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -1778,7 +1779,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1812,7 +1813,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1857,7 +1858,7 @@ ${_topicsSection()} '''); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -1886,7 +1887,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1924,7 +1925,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -1959,7 +1960,7 @@ ${_topicsSection()} setAllowedDependencies(unpinned: ['allowed']); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -1988,7 +1989,7 @@ ${_topicsSection()} setAllowedDependencies(pinned: ['allow_pinned']); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -2018,7 +2019,7 @@ ${_topicsSection()} setAllowedDependencies(pinned: ['allow_pinned']); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -2048,7 +2049,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -2101,7 +2102,7 @@ ${_topicsSection()} Error? commandError; final List output = await runCapturingPrint( runner, - ['pubspec-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -2140,7 +2141,7 @@ ${_topicsSection()} '''); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( @@ -2166,11 +2167,12 @@ ${_topicsSection()} final GitDir gitDir; (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); - final command = PubspecCheckCommand( + final command = ValidateCommand( packagesDir, processRunner: processRunner, platform: mockPlatform, gitDir: gitDir, + targetedValidators: {Validator.pubspec}, ); runner = CommandRunner( @@ -2195,7 +2197,7 @@ ${_topicsSection()} '''); final List output = await runCapturingPrint(runner, [ - 'pubspec-check', + 'validate', ]); expect( diff --git a/tool_config/min_version.yaml b/tool_config/min_version.yaml index ce92e9515f5c..16737c58e6a6 100644 --- a/tool_config/min_version.yaml +++ b/tool_config/min_version.yaml @@ -2,4 +2,7 @@ # by packages in this repository. For example, if this is set to 3.22, then a # package can have a minimum SDK version of 3.32, 3.35, 3.38, etc, but # not 3.29. +# +# The major and minor version here should match the lowest version analyzed +# in legacy version analysis (.ci.yaml analyze_legacy). min_flutter: '3.35.0' From b357ceed3c83e64953b2c289e1e65339f2713f8a Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 30 Apr 2026 14:54:01 -0400 Subject: [PATCH 09/18] Switch excerpt exemption to use ci_config --- .ci/targets/repo_checks.yaml | 8 ----- packages/espresso/ci_config.yaml | 2 ++ .../in_app_purchase/ci_config.yaml | 2 ++ .../pointer_interceptor/ci_config.yaml | 2 ++ .../quick_actions/ci_config.yaml | 2 ++ script/configs/temp_exclude_excerpt.yaml | 12 ------- script/tool/lib/src/common/ci_config.dart | 19 +++++++++-- script/tool/lib/src/readme_check_command.dart | 10 +----- .../lib/src/validators/readme_validator.dart | 17 +++++++--- .../tool/test/readme_check_command_test.dart | 34 +++++++++++++++++-- .../packages/mustache_template/ci_config.yaml | 2 ++ 11 files changed, 71 insertions(+), 39 deletions(-) create mode 100644 packages/espresso/ci_config.yaml create mode 100644 packages/in_app_purchase/in_app_purchase/ci_config.yaml create mode 100644 packages/pointer_interceptor/pointer_interceptor/ci_config.yaml create mode 100644 packages/quick_actions/quick_actions/ci_config.yaml delete mode 100644 script/configs/temp_exclude_excerpt.yaml create mode 100644 third_party/packages/mustache_template/ci_config.yaml diff --git a/.ci/targets/repo_checks.yaml b/.ci/targets/repo_checks.yaml index 5d5b71d0a92c..7b765d60897d 100644 --- a/.ci/targets/repo_checks.yaml +++ b/.ci/targets/repo_checks.yaml @@ -26,14 +26,6 @@ tasks: script: .ci/scripts/tool_runner.sh args: ["readme-check"] always: true - # Re-run with --require-excerpts, skipping packages that still need - # to be converted. Once https://github.com/flutter/flutter/issues/102679 - # has been fixed, this can be removed --require-excerpts added to the - # run above. - - name: README snippet configuration validation - script: .ci/scripts/tool_runner.sh - args: ["readme-check", "--require-excerpts", "--exclude=script/configs/temp_exclude_excerpt.yaml"] - always: true - name: README snippet validation script: .ci/scripts/tool_runner.sh args: ["update-excerpts", "--fail-on-change"] diff --git a/packages/espresso/ci_config.yaml b/packages/espresso/ci_config.yaml new file mode 100644 index 000000000000..b352e13e0dfa --- /dev/null +++ b/packages/espresso/ci_config.yaml @@ -0,0 +1,2 @@ +# TODO(stuartmorgan): Remove this; see https://github.com/flutter/flutter/issues/102679 +exempt_from_excerpts: true diff --git a/packages/in_app_purchase/in_app_purchase/ci_config.yaml b/packages/in_app_purchase/in_app_purchase/ci_config.yaml new file mode 100644 index 000000000000..b352e13e0dfa --- /dev/null +++ b/packages/in_app_purchase/in_app_purchase/ci_config.yaml @@ -0,0 +1,2 @@ +# TODO(stuartmorgan): Remove this; see https://github.com/flutter/flutter/issues/102679 +exempt_from_excerpts: true diff --git a/packages/pointer_interceptor/pointer_interceptor/ci_config.yaml b/packages/pointer_interceptor/pointer_interceptor/ci_config.yaml new file mode 100644 index 000000000000..b352e13e0dfa --- /dev/null +++ b/packages/pointer_interceptor/pointer_interceptor/ci_config.yaml @@ -0,0 +1,2 @@ +# TODO(stuartmorgan): Remove this; see https://github.com/flutter/flutter/issues/102679 +exempt_from_excerpts: true diff --git a/packages/quick_actions/quick_actions/ci_config.yaml b/packages/quick_actions/quick_actions/ci_config.yaml new file mode 100644 index 000000000000..b352e13e0dfa --- /dev/null +++ b/packages/quick_actions/quick_actions/ci_config.yaml @@ -0,0 +1,2 @@ +# TODO(stuartmorgan): Remove this; see https://github.com/flutter/flutter/issues/102679 +exempt_from_excerpts: true diff --git a/script/configs/temp_exclude_excerpt.yaml b/script/configs/temp_exclude_excerpt.yaml deleted file mode 100644 index 2dd229b184cf..000000000000 --- a/script/configs/temp_exclude_excerpt.yaml +++ /dev/null @@ -1,12 +0,0 @@ -# Packages that have not yet adopted code-excerpt. -# -# This only exists to allow incrementally adopting the new requirement. -# Packages shoud never be added to this list. - -# TODO(stuartmorgan): Remove everything from this list. See -# https://github.com/flutter/flutter/issues/102679 -- espresso -- in_app_purchase/in_app_purchase -- mustache_template -- pointer_interceptor -- quick_actions/quick_actions diff --git a/script/tool/lib/src/common/ci_config.dart b/script/tool/lib/src/common/ci_config.dart index 0f35fe42d15e..dd6907c2e1c8 100644 --- a/script/tool/lib/src/common/ci_config.dart +++ b/script/tool/lib/src/common/ci_config.dart @@ -7,7 +7,7 @@ import 'package:yaml/yaml.dart'; /// A class representing the parsed content of a `ci_config.yaml` file. class CIConfig { /// Creates a [CIConfig] from a parsed YAML map. - CIConfig._(this.isBatchRelease); + CIConfig._({required this.isBatchRelease, required this.requiresExcerpts}); /// Parses a [CIConfig] from a YAML string. /// @@ -26,18 +26,33 @@ class CIConfig { isBatchRelease = release['batch'] == true; } - return CIConfig._(isBatchRelease); + // Any package that hasn't been explicitly exempted is assumed to require + // excerpts. + var requiresExcerpts = true; + final Object? exemptFromExcerpts = loaded['exempt_from_excerpts']; + if (exemptFromExcerpts is bool) { + requiresExcerpts = !exemptFromExcerpts; + } + + return CIConfig._( + isBatchRelease: isBatchRelease, + requiresExcerpts: requiresExcerpts, + ); } static const Map _validCIConfigSyntax = { 'release': { 'batch': {true, false}, }, + 'exempt_from_excerpts': {true, false}, }; /// Returns true if the package is configured for batch release. final bool isBatchRelease; + /// Returns true if the package is configured to require excerpts. + final bool requiresExcerpts; + static void _checkCIConfigEntries( YamlMap config, { required Map syntax, diff --git a/script/tool/lib/src/readme_check_command.dart b/script/tool/lib/src/readme_check_command.dart index fc3f18b5888f..e148d8fe8073 100644 --- a/script/tool/lib/src/readme_check_command.dart +++ b/script/tool/lib/src/readme_check_command.dart @@ -18,14 +18,7 @@ class ReadmeCheckCommand extends PackageLoopingCommand { super.processRunner, super.platform, super.gitDir, - }) { - argParser.addFlag( - _requireExcerptsArg, - help: 'Require that Dart code blocks be managed by code-excerpt.', - ); - } - - static const String _requireExcerptsArg = 'require-excerpts'; + }); @override final String name = 'readme-check'; @@ -62,7 +55,6 @@ class ReadmeCheckCommand extends PackageLoopingCommand { path: path, indentation: indentation, warningLogger: printWarning, - requireCodeExcerpts: getBoolArg(_requireExcerptsArg), ); final List errors = validator.validateReadme( diff --git a/script/tool/lib/src/validators/readme_validator.dart b/script/tool/lib/src/validators/readme_validator.dart index 23f6ef82624b..e13699fa367b 100644 --- a/script/tool/lib/src/validators/readme_validator.dart +++ b/script/tool/lib/src/validators/readme_validator.dart @@ -30,16 +30,13 @@ class ReadmeValidator { required path.Context path, required String indentation, required void Function(String) warningLogger, - required bool requireCodeExcerpts, }) : _path = path, _indentation = indentation, - _logWarning = warningLogger, - _requireCodeExcerpts = requireCodeExcerpts; + _logWarning = warningLogger; final path.Context _path; final String _indentation; final void Function(String) _logWarning; - final bool _requireCodeExcerpts; /// Validates the given README file, returning a list of resulting error /// strings. @@ -74,9 +71,18 @@ class ReadmeValidator { final List readmeLines = readme.readAsLinesSync(); final errors = []; + // Require code-excerpt managed blocks unless explicitly opted out. + final bool requireCodeExcerpts = + mainPackage.parseCIConfig()?.requiresExcerpts ?? true; + if (!requireCodeExcerpts) { + printWarning( + '${_indentation}Skipping code excerpt validation due to ${mainPackage.ciConfigFile.basename}', + ); + } final String? blockValidationError = _validateCodeBlocks( readmeLines, mainPackage: mainPackage, + requireCodeExcerpts: requireCodeExcerpts, ); if (blockValidationError != null) { errors.add(blockValidationError); @@ -110,6 +116,7 @@ class ReadmeValidator { String? _validateCodeBlocks( List readmeLines, { required RepositoryPackage mainPackage, + required bool requireCodeExcerpts, }) { final codeBlockDelimiterPattern = RegExp(r'^\s*```\s*([^ ]*)\s*'); const excerptTagStart = ' ``` dart A B C ``` @@ -747,7 +748,6 @@ A B C final List output = await runCapturingPrint(runner, [ 'readme-check', - '--require-excerpts', ]); expect( @@ -759,7 +759,7 @@ A B C ); }); - test('fails on missing excerpt tag when requested', () async { + test('fails on missing excerpt tag', () async { final RepositoryPackage package = createFakePackage( 'a_package', packagesDir, @@ -776,7 +776,7 @@ A B C Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check', '--require-excerpts'], + ['readme-check'], errorHandler: (Error e) { commandError = e; }, @@ -795,5 +795,33 @@ A B C ]), ); }); + + test('passes and warns for missing excerpt tag when opted out', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + ); + + package.readmeFile.writeAsStringSync(''' +Example: + +```dart +A B C +``` +'''); + package.ciConfigFile.writeAsStringSync('exempt_from_excerpts: true'); + + final List output = await runCapturingPrint(runner, [ + 'readme-check', + ]); + + expect( + output, + containsAll([ + contains('Running for a_package...'), + contains('Skipping code excerpt validation due to ci_config.yaml'), + ]), + ); + }); }); } diff --git a/third_party/packages/mustache_template/ci_config.yaml b/third_party/packages/mustache_template/ci_config.yaml new file mode 100644 index 000000000000..b352e13e0dfa --- /dev/null +++ b/third_party/packages/mustache_template/ci_config.yaml @@ -0,0 +1,2 @@ +# TODO(stuartmorgan): Remove this; see https://github.com/flutter/flutter/issues/102679 +exempt_from_excerpts: true From 323cb40b4f51d3f163050223e7157657cf611c86 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 30 Apr 2026 15:03:47 -0400 Subject: [PATCH 10/18] Fix regression --- script/tool/lib/src/common/file_utils.dart | 10 ++++- script/tool/lib/src/validate_command.dart | 49 +++++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/script/tool/lib/src/common/file_utils.dart b/script/tool/lib/src/common/file_utils.dart index e9939d363c30..bd56525d20f7 100644 --- a/script/tool/lib/src/common/file_utils.dart +++ b/script/tool/lib/src/common/file_utils.dart @@ -58,6 +58,12 @@ List? loadYamlList(File file) { if (!file.existsSync()) { return null; } - final yaml = loadYaml(file.readAsStringSync()) as YamlList; - return yaml.toList().cast(); + final String contents = file.readAsStringSync(); + final Object? yaml = loadYaml(contents); + // Treat an empty or comment-only file as an empty list, to avoid requiring + // adding an explicit empty list entry to the YAML when no entries are needed. + if (yaml == null) { + return []; + } + return (yaml as YamlList).toList().cast(); } diff --git a/script/tool/lib/src/validate_command.dart b/script/tool/lib/src/validate_command.dart index b6be97c90ae5..dbb60e9c8496 100644 --- a/script/tool/lib/src/validate_command.dart +++ b/script/tool/lib/src/validate_command.dart @@ -27,7 +27,7 @@ import 'validators/repo_info_validator.dart'; /// includes things like command line parsing and run initialization. @visibleForTesting // ignore: public_member_api_docs -enum Validator { dependabot, repoInfo, gradle, pubspec } +enum Validator { dependabot, gradle, pubspec, readme, repoInfo } // Config file names. const String _versionConfigFileName = 'min_version.yaml'; @@ -129,6 +129,7 @@ class ValidateCommand extends PackageLoopingCommand { final List errors = [ if (_shouldRun(Validator.repoInfo)) ...await _validateRepoInfo(package), if (_shouldRun(Validator.pubspec)) ...await _validatePubspec(package), + if (_shouldRun(Validator.readme)) ...await _validateReadme(package), if (_shouldRun(Validator.dependabot)) ...await _validateDependabot(package), if (_shouldRun(Validator.gradle)) ...await _validateGradle(package), @@ -188,6 +189,52 @@ class ValidateCommand extends PackageLoopingCommand { return validator.validatePubspec(package); } + Future> _validateReadme(RepositoryPackage package) async { + if (!package.isTopLevel) { + return []; + } + /* + final validator = ReadmeValidator( + path: path, + indentation: indentation, + warningLogger: printWarning, + requireCodeExcerpts: getBoolArg(_requireExcerptsArg), + ); + + final List errors = validator.validateReadme( + package.readmeFile, + mainPackage: package, + isExample: false, + ); + for (final RepositoryPackage packageToCheck in package.getExamples()) { + errors.addAll( + validator.validateReadme( + packageToCheck.readmeFile, + mainPackage: package, + isExample: true, + ), + ); + } + + // If there's an example/README.md for a multi-example package, validate + // that as well, as it will be shown on pub.dev. + final Directory exampleDir = package.directory.childDirectory('example'); + final File exampleDirReadme = exampleDir.childFile('README.md'); + if (exampleDir.existsSync() && !isPackage(exampleDir)) { + errors.addAll( + validator.validateReadme( + exampleDirReadme, + mainPackage: package, + isExample: true, + ), + ); + } + + return errors; +*/ + return []; + } + Stream _findAllPublishedPackages() async* { for (final File pubspecFile in (await _repoRoot.list(recursive: true, followLinks: false).toList()) From 0f7948d92796ca576de5d871272280aeac2695d2 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 30 Apr 2026 16:44:37 -0400 Subject: [PATCH 11/18] Fold in README validation --- .ci/targets/repo_checks.yaml | 4 - script/tool/lib/src/main.dart | 2 - script/tool/lib/src/readme_check_command.dart | 91 ------------------- script/tool/lib/src/validate_command.dart | 10 +- ...dart => validate_command_readme_test.dart} | 53 +++++------ 5 files changed, 33 insertions(+), 127 deletions(-) delete mode 100644 script/tool/lib/src/readme_check_command.dart rename script/tool/test/{readme_check_command_test.dart => validate_command_readme_test.dart} (96%) diff --git a/.ci/targets/repo_checks.yaml b/.ci/targets/repo_checks.yaml index 7b765d60897d..6d95f47eb1fc 100644 --- a/.ci/targets/repo_checks.yaml +++ b/.ci/targets/repo_checks.yaml @@ -22,10 +22,6 @@ tasks: script: .ci/scripts/tool_runner.sh args: ["license-check"] always: true - - name: README validation - script: .ci/scripts/tool_runner.sh - args: ["readme-check"] - always: true - name: README snippet validation script: .ci/scripts/tool_runner.sh args: ["update-excerpts", "--fail-on-change"] diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index 4dc4187bc6ec..1c61ef1f6683 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -28,7 +28,6 @@ import 'native_test_command.dart'; import 'podspec_check_command.dart'; import 'publish_check_command.dart'; import 'publish_command.dart'; -import 'readme_check_command.dart'; import 'remove_dev_dependencies_command.dart'; import 'update_dependency_command.dart'; import 'update_excerpts_command.dart'; @@ -79,7 +78,6 @@ void main(List args) { ..addCommand(PodspecCheckCommand(packagesDir)) ..addCommand(PublishCheckCommand(packagesDir)) ..addCommand(PublishCommand(packagesDir)) - ..addCommand(ReadmeCheckCommand(packagesDir)) ..addCommand(RemoveDevDependenciesCommand(packagesDir)) ..addCommand(UpdateDependencyCommand(packagesDir)) ..addCommand(UpdateExcerptsCommand(packagesDir)) diff --git a/script/tool/lib/src/readme_check_command.dart b/script/tool/lib/src/readme_check_command.dart deleted file mode 100644 index e148d8fe8073..000000000000 --- a/script/tool/lib/src/readme_check_command.dart +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright 2013 The Flutter Authors -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:file/file.dart'; - -import 'common/core.dart'; -import 'common/output_utils.dart'; -import 'common/package_looping_command.dart'; -import 'common/repository_package.dart'; -import 'validators/readme_validator.dart'; - -/// A command to enforce README conventions across the repository. -class ReadmeCheckCommand extends PackageLoopingCommand { - /// Creates an instance of the README check command. - ReadmeCheckCommand( - super.packagesDir, { - super.processRunner, - super.platform, - super.gitDir, - }); - - @override - final String name = 'readme-check'; - - @override - List get aliases => ['check-readme']; - - @override - final String description = - 'Checks that READMEs follow repository conventions.'; - - @override - final PackageLoopingType packageLoopingType = - PackageLoopingType.includeAllSubpackages; - - @override - bool get hasLongOutput => false; - - @override - Future runForPackage(RepositoryPackage package) async { - final List errors = []; - - if (package.isTopLevel) { - errors.addAll(await _validateReadme(package)); - } - - return errors.isEmpty - ? PackageResult.success() - : PackageResult.fail(errors); - } - - Future> _validateReadme(RepositoryPackage package) async { - final validator = ReadmeValidator( - path: path, - indentation: indentation, - warningLogger: printWarning, - ); - - final List errors = validator.validateReadme( - package.readmeFile, - mainPackage: package, - isExample: false, - ); - for (final RepositoryPackage packageToCheck in package.getExamples()) { - errors.addAll( - validator.validateReadme( - packageToCheck.readmeFile, - mainPackage: package, - isExample: true, - ), - ); - } - - // If there's an example/README.md for a multi-example package, validate - // that as well, as it will be shown on pub.dev. - final Directory exampleDir = package.directory.childDirectory('example'); - final File exampleDirReadme = exampleDir.childFile('README.md'); - if (exampleDir.existsSync() && !isPackage(exampleDir)) { - errors.addAll( - validator.validateReadme( - exampleDirReadme, - mainPackage: package, - isExample: true, - ), - ); - } - - return errors; - } -} diff --git a/script/tool/lib/src/validate_command.dart b/script/tool/lib/src/validate_command.dart index dbb60e9c8496..6b561e8025d0 100644 --- a/script/tool/lib/src/validate_command.dart +++ b/script/tool/lib/src/validate_command.dart @@ -15,6 +15,7 @@ import 'common/repository_package.dart'; import 'validators/dependabot_validator.dart'; import 'validators/gradle_validator.dart'; import 'validators/pubspec_validator.dart'; +import 'validators/readme_validator.dart'; import 'validators/repo_info_validator.dart'; /// The set of possible validators. @@ -190,15 +191,18 @@ class ValidateCommand extends PackageLoopingCommand { } Future> _validateReadme(RepositoryPackage package) async { + // TODO(stuartmorgan): Consider restructuring this to just check the + // current package's README for all packages, now that this is part of an + // includeAllSubpackages command. The current logic is from when it was + // its own top-level-only command. if (!package.isTopLevel) { return []; } - /* + final validator = ReadmeValidator( path: path, indentation: indentation, warningLogger: printWarning, - requireCodeExcerpts: getBoolArg(_requireExcerptsArg), ); final List errors = validator.validateReadme( @@ -231,8 +235,6 @@ class ValidateCommand extends PackageLoopingCommand { } return errors; -*/ - return []; } Stream _findAllPublishedPackages() async* { diff --git a/script/tool/test/readme_check_command_test.dart b/script/tool/test/validate_command_readme_test.dart similarity index 96% rename from script/tool/test/readme_check_command_test.dart rename to script/tool/test/validate_command_readme_test.dart index a26281ac8a96..5f1de39ca52b 100644 --- a/script/tool/test/readme_check_command_test.dart +++ b/script/tool/test/validate_command_readme_test.dart @@ -6,7 +6,7 @@ import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:flutter_plugin_tools/src/common/core.dart'; import 'package:flutter_plugin_tools/src/common/plugin_utils.dart'; -import 'package:flutter_plugin_tools/src/readme_check_command.dart'; +import 'package:flutter_plugin_tools/src/validate_command.dart'; import 'package:git/git.dart'; import 'package:test/test.dart'; @@ -24,11 +24,12 @@ void main() { final GitDir gitDir; (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); - final command = ReadmeCheckCommand( + final command = ValidateCommand( packagesDir, processRunner: processRunner, platform: mockPlatform, gitDir: gitDir, + targetedValidators: {Validator.readme}, ); runner = CommandRunner( @@ -50,7 +51,7 @@ void main() { getExampleDir(package).childFile('README.md').writeAsStringSync('A readme'); final List output = await runCapturingPrint(runner, [ - 'readme-check', + 'validate', ]); expect( @@ -74,7 +75,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -91,7 +92,7 @@ void main() { createFakePackage('a_package', packagesDir); final List output = await runCapturingPrint(runner, [ - 'readme-check', + 'validate', ]); expect( @@ -113,7 +114,7 @@ void main() { miscSubpackage.readmeFile.deleteSync(); final List output = await runCapturingPrint(runner, [ - 'readme-check', + 'validate', ]); expect(output, isNot(contains(subpackageName))); @@ -137,7 +138,7 @@ samples, guidance on mobile development, and a full API reference. Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -181,7 +182,7 @@ samples, guidance on mobile development, and a full API reference. Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -216,7 +217,7 @@ Demonstrates how to use the a_plugin_ios plugin. Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -262,7 +263,7 @@ Demonstrates how to use the a_plugin plugin. '''); final List output = await runCapturingPrint(runner, [ - 'readme-check', + 'validate', ]); expect( @@ -291,7 +292,7 @@ Some random description. Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -347,7 +348,7 @@ very unlikely to be relevant. '''); final List output = await runCapturingPrint(runner, [ - 'readme-check', + 'validate', ]); expect( @@ -388,7 +389,7 @@ samples, guidance on mobile development, and a full API reference. Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -425,7 +426,7 @@ samples, guidance on mobile development, and a full API reference. createFakePlugin('${federatedPluginName}_android', federatedDir); final List output = await runCapturingPrint(runner, [ - 'readme-check', + 'validate', ]); expect( @@ -448,7 +449,7 @@ samples, guidance on mobile development, and a full API reference. Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -470,7 +471,7 @@ samples, guidance on mobile development, and a full API reference. Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -499,7 +500,7 @@ A very useful plugin. Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -536,7 +537,7 @@ A very useful plugin. Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -577,7 +578,7 @@ A very useful plugin. Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -620,7 +621,7 @@ A very useful plugin. Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -660,7 +661,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -691,7 +692,7 @@ A B C '''); final List output = await runCapturingPrint(runner, [ - 'readme-check', + 'validate', ]); expect( @@ -719,7 +720,7 @@ A B C '''); final List output = await runCapturingPrint(runner, [ - 'readme-check', + 'validate', ]); expect( @@ -747,7 +748,7 @@ A B C '''); final List output = await runCapturingPrint(runner, [ - 'readme-check', + 'validate', ]); expect( @@ -776,7 +777,7 @@ A B C Error? commandError; final List output = await runCapturingPrint( runner, - ['readme-check'], + ['validate'], errorHandler: (Error e) { commandError = e; }, @@ -812,7 +813,7 @@ A B C package.ciConfigFile.writeAsStringSync('exempt_from_excerpts: true'); final List output = await runCapturingPrint(runner, [ - 'readme-check', + 'validate', ]); expect( From 52aa7ff226b7549b3d5feefdd9cf3587db2af09c Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 30 Apr 2026 16:47:04 -0400 Subject: [PATCH 12/18] Adjust logging for consistency --- .../lib/src/validators/readme_validator.dart | 2 +- .../tool/test/validate_command_readme_test.dart | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/script/tool/lib/src/validators/readme_validator.dart b/script/tool/lib/src/validators/readme_validator.dart index e13699fa367b..b60d1473fb7f 100644 --- a/script/tool/lib/src/validators/readme_validator.dart +++ b/script/tool/lib/src/validators/readme_validator.dart @@ -64,7 +64,7 @@ class ReadmeValidator { } print( - '${_indentation}Checking ' + '${_indentation}Validating ' '${relativePosixPath(readme, from: mainPackage.directory, platformContext: _path)}...', ); diff --git a/script/tool/test/validate_command_readme_test.dart b/script/tool/test/validate_command_readme_test.dart index 5f1de39ca52b..9c5b9f0e5ddf 100644 --- a/script/tool/test/validate_command_readme_test.dart +++ b/script/tool/test/validate_command_readme_test.dart @@ -57,10 +57,10 @@ void main() { expect( output, containsAll([ - contains(' Checking README.md...'), - contains(' Checking example/README.md...'), - contains(' Checking example/example1/README.md...'), - contains(' Checking example/example2/README.md...'), + contains(' Validating README.md...'), + contains(' Validating example/README.md...'), + contains(' Validating example/example1/README.md...'), + contains(' Validating example/example2/README.md...'), ]), ); }); @@ -269,8 +269,8 @@ Demonstrates how to use the a_plugin plugin. expect( output, containsAll([ - contains(' Checking README.md...'), - contains(' Checking example/README.md...'), + contains(' Validating README.md...'), + contains(' Validating example/README.md...'), ]), ); }); @@ -354,8 +354,8 @@ very unlikely to be relevant. expect( output, containsAll([ - contains(' Checking README.md...'), - contains(' Checking example/README.md...'), + contains(' Validating README.md...'), + contains(' Validating example/README.md...'), ]), ); }, From 72a3e9b51cb84fe3259c3928e761433da8d6d925 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 1 May 2026 10:34:37 -0400 Subject: [PATCH 13/18] Fold in version/changelog validation --- .ci/scripts/{check_version.sh => validate.sh} | 4 +- .ci/targets/repo_checks.yaml | 6 +- script/tool/lib/src/main.dart | 4 +- script/tool/lib/src/validate_command.dart | 85 ++++++++++- .../tool/lib/src/version_check_command.dart | 134 ------------------ ...art => validate_command_version_test.dart} | 97 ++++++------- 6 files changed, 136 insertions(+), 194 deletions(-) rename .ci/scripts/{check_version.sh => validate.sh} (72%) delete mode 100644 script/tool/lib/src/version_check_command.dart rename script/tool/test/{version_check_command_test.dart => validate_command_version_test.dart} (97%) diff --git a/.ci/scripts/check_version.sh b/.ci/scripts/validate.sh similarity index 72% rename from .ci/scripts/check_version.sh rename to .ci/scripts/validate.sh index e9b06692049d..d7f72cbe7336 100755 --- a/.ci/scripts/check_version.sh +++ b/.ci/scripts/validate.sh @@ -10,7 +10,7 @@ set -e # missing version/CHANGELOG detection since PR-level overrides aren't available # in post-submit. if [[ $LUCI_PR == "" ]]; then - .ci/scripts/tool_runner.sh version-check --ignore-platform-interface-breaks + .ci/scripts/tool_runner.sh validate --ignore-platform-interface-breaks else - .ci/scripts/tool_runner.sh version-check --check-for-missing-changes --pr-labels="$PR_OVERRIDE_LABELS" + .ci/scripts/tool_runner.sh validate --check-for-missing-changes --pr-labels="$PR_OVERRIDE_LABELS" fi diff --git a/.ci/targets/repo_checks.yaml b/.ci/targets/repo_checks.yaml index 6d95f47eb1fc..7701aa7303cb 100644 --- a/.ci/targets/repo_checks.yaml +++ b/.ci/targets/repo_checks.yaml @@ -27,11 +27,7 @@ tasks: args: ["update-excerpts", "--fail-on-change"] always: true - name: Repository standards validation - script: .ci/scripts/tool_runner.sh - args: ["validate"] - always: true - - name: CHANGELOG and version validation - script: .ci/scripts/check_version.sh + script: .ci/scripts/validate.sh always: true - name: federated safety check script: .ci/scripts/check_federated_safety.sh diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index 1c61ef1f6683..db25d255add4 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -34,7 +34,6 @@ import 'update_excerpts_command.dart'; import 'update_min_sdk_command.dart'; import 'update_release_info_command.dart'; import 'validate_command.dart'; -import 'version_check_command.dart'; void main(List args) { const FileSystem fileSystem = LocalFileSystem(); @@ -83,8 +82,7 @@ void main(List args) { ..addCommand(UpdateExcerptsCommand(packagesDir)) ..addCommand(UpdateMinSdkCommand(packagesDir)) ..addCommand(UpdateReleaseInfoCommand(packagesDir)) - ..addCommand(ValidateCommand(packagesDir)) - ..addCommand(VersionCheckCommand(packagesDir)); + ..addCommand(ValidateCommand(packagesDir)); commandRunner.run(args).catchError((Object e) { final toolExit = e as ToolExit; diff --git a/script/tool/lib/src/validate_command.dart b/script/tool/lib/src/validate_command.dart index 6b561e8025d0..cb38ca128288 100644 --- a/script/tool/lib/src/validate_command.dart +++ b/script/tool/lib/src/validate_command.dart @@ -9,6 +9,7 @@ import 'package:yaml/yaml.dart'; import 'common/core.dart'; import 'common/file_utils.dart'; +import 'common/git_version_finder.dart'; import 'common/output_utils.dart'; import 'common/package_looping_command.dart'; import 'common/repository_package.dart'; @@ -17,6 +18,7 @@ import 'validators/gradle_validator.dart'; import 'validators/pubspec_validator.dart'; import 'validators/readme_validator.dart'; import 'validators/repo_info_validator.dart'; +import 'validators/version_and_changelog_validator.dart'; /// The set of possible validators. /// @@ -28,7 +30,7 @@ import 'validators/repo_info_validator.dart'; /// includes things like command line parsing and run initialization. @visibleForTesting // ignore: public_member_api_docs -enum Validator { dependabot, gradle, pubspec, readme, repoInfo } +enum Validator { dependabot, gradle, pubspec, readme, repoInfo, version } // Config file names. const String _versionConfigFileName = 'min_version.yaml'; @@ -56,7 +58,41 @@ class ValidateCommand extends PackageLoopingCommand { super.processRunner, super.platform, super.gitDir, - }); + }) { + argParser.addOption( + _prLabelsArg, + help: + 'A comma-separated list of labels associated with this PR, ' + 'if applicable.\n\n' + 'If supplied, labels may override or disable some checks.', + hide: true, + ); + argParser.addFlag( + _checkForMissingChanges, + help: + 'Validates that changes to packages include CHANGELOG and ' + 'version changes unless they meet an established exemption.\n\n' + 'If used with --$_prLabelsArg, this should only be used in ' + 'pre-submit CI checks, to prevent post-submit breakage ' + 'when labels are no longer applicable.', + ); + argParser.addFlag( + _ignorePlatformInterfaceBreaks, + help: + 'Bypasses the check that platform interfaces do not contain ' + 'breaking changes.\n\n' + 'This is only intended for use in post-submit CI checks, to ' + 'prevent post-submit breakage when overriding the check with ' + 'labels. Pre-submit checks should always use ' + '--$_prLabelsArg instead.', + hide: true, + ); + } + + static const String _prLabelsArg = 'pr-labels'; + static const String _checkForMissingChanges = 'check-for-missing-changes'; + static const String _ignorePlatformInterfaceBreaks = + 'ignore-platform-interface-breaks'; /// The validators to run. /// @@ -65,6 +101,10 @@ class ValidateCommand extends PackageLoopingCommand { late Directory _repoRoot; + late final GitVersionFinder _gitVersionFinder; + + late final Set _prLabels = _getPRLabels(); + /// Data from the root README.md table of packages. final Map> _readmeTableEntries = >{}; @@ -100,6 +140,7 @@ class ValidateCommand extends PackageLoopingCommand { @override Future initializeRun() async { + _gitVersionFinder = await retrieveVersionFinder(); _repoRoot = packagesDir.fileSystem.directory((await gitDir).path); if (_shouldRun(Validator.repoInfo)) { @@ -134,6 +175,8 @@ class ValidateCommand extends PackageLoopingCommand { if (_shouldRun(Validator.dependabot)) ...await _validateDependabot(package), if (_shouldRun(Validator.gradle)) ...await _validateGradle(package), + if (_shouldRun(Validator.version)) + ...await _validateVersionAndChangelog(package), ]; return errors.isEmpty @@ -237,6 +280,34 @@ class ValidateCommand extends PackageLoopingCommand { return errors; } + Future> _validateVersionAndChangelog( + RepositoryPackage package, + ) async { + if (!package.isTopLevel) { + return []; + } + + final Directory repoRoot = packagesDir.fileSystem.directory( + (await gitDir).path, + ); + + final validator = VersionAndChangelogValidator( + path: path, + indentation: indentation, + warningLogger: logWarning, + gitVersionFinder: _gitVersionFinder, + repoRoot: repoRoot, + changedFiles: changedFiles, + prLabels: _prLabels, + ); + + return validator.validateChangelogAndVersion( + package, + checkForMissingChanges: getBoolArg(_checkForMissingChanges), + ignorePlatformInterfaceBreaks: getBoolArg(_ignorePlatformInterfaceBreaks), + ); + } + Stream _findAllPublishedPackages() async* { for (final File pubspecFile in (await _repoRoot.list(recursive: true, followLinks: false).toList()) @@ -300,4 +371,14 @@ class ValidateCommand extends PackageLoopingCommand { } return null; } + + /// Returns the labels associated with this PR, if any, or an empty set + /// if that flag is not provided. + Set _getPRLabels() { + final String labels = getStringArg(_prLabelsArg); + if (labels.isEmpty) { + return {}; + } + return labels.split(',').map((String label) => label.trim()).toSet(); + } } diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart deleted file mode 100644 index a4ec2114f805..000000000000 --- a/script/tool/lib/src/version_check_command.dart +++ /dev/null @@ -1,134 +0,0 @@ -// Copyright 2013 The Flutter Authors -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:file/file.dart'; - -import 'common/git_version_finder.dart'; -import 'common/package_looping_command.dart'; -import 'common/repository_package.dart'; -import 'validators/version_and_changelog_validator.dart'; - -/// A command to validate version changes to packages. -class VersionCheckCommand extends PackageLoopingCommand { - /// Creates an instance of the version check command. - VersionCheckCommand( - super.packagesDir, { - super.processRunner, - super.platform, - super.gitDir, - }) { - argParser.addOption( - _prLabelsArg, - help: - 'A comma-separated list of labels associated with this PR, ' - 'if applicable.\n\n' - 'If supplied, this may be to allow overrides to some version ' - 'checks.', - ); - argParser.addFlag( - _checkForMissingChanges, - help: - 'Validates that changes to packages include CHANGELOG and ' - 'version changes unless they meet an established exemption.\n\n' - 'If used with --$_prLabelsArg, this is should only be ' - 'used in pre-submit CI checks, to prevent post-submit breakage ' - 'when labels are no longer applicable.', - hide: true, - ); - argParser.addFlag( - _ignorePlatformInterfaceBreaks, - help: - 'Bypasses the check that platform interfaces do not contain ' - 'breaking changes.\n\n' - 'This is only intended for use in post-submit CI checks, to ' - 'prevent post-submit breakage when overriding the check with ' - 'labels. Pre-submit checks should always use ' - '--$_prLabelsArg instead.', - hide: true, - ); - } - - static const String _prLabelsArg = 'pr-labels'; - static const String _checkForMissingChanges = 'check-for-missing-changes'; - static const String _ignorePlatformInterfaceBreaks = - 'ignore-platform-interface-breaks'; - - late final GitVersionFinder _gitVersionFinder; - - late final Set _prLabels = _getPRLabels(); - - @override - final String name = 'version-check'; - - @override - List get aliases => ['check-version']; - - @override - final String description = - 'Checks if the versions of packages have been incremented per pub specification.\n' - 'Also checks if the latest version in CHANGELOG matches the version in pubspec.\n\n' - 'This command requires "flutter" to be in your path.'; - - @override - final PackageLoopingType packageLoopingType = - PackageLoopingType.includeAllSubpackages; - - @override - bool get hasLongOutput => false; - - @override - Future initializeRun() async { - _gitVersionFinder = await retrieveVersionFinder(); - } - - @override - Future runForPackage(RepositoryPackage package) async { - if (!package.isTopLevel) { - return PackageResult.skip('Not a top-level package.'); - } - final List errors = []; - - if (package.isTopLevel) { - errors.addAll(await _validateVersionAndChangelog(package)); - } - - return errors.isEmpty - ? PackageResult.success() - : PackageResult.fail(errors); - } - - Future> _validateVersionAndChangelog( - RepositoryPackage package, - ) async { - final Directory repoRoot = packagesDir.fileSystem.directory( - (await gitDir).path, - ); - - final validator = VersionAndChangelogValidator( - path: path, - indentation: indentation, - warningLogger: logWarning, - gitVersionFinder: _gitVersionFinder, - repoRoot: repoRoot, - changedFiles: changedFiles, - prLabels: _prLabels, - ); - - return validator.validateChangelogAndVersion( - package, - checkForMissingChanges: getBoolArg(_checkForMissingChanges), - ignorePlatformInterfaceBreaks: getBoolArg(_ignorePlatformInterfaceBreaks), - ); - } - - /// Returns the labels associated with this PR, if any, or an empty set - /// if that flag is not provided. - Set _getPRLabels() { - final String labels = getStringArg(_prLabelsArg); - if (labels.isEmpty) { - return {}; - } - return labels.split(',').map((String label) => label.trim()).toSet(); - } -} diff --git a/script/tool/test/version_check_command_test.dart b/script/tool/test/validate_command_version_test.dart similarity index 97% rename from script/tool/test/version_check_command_test.dart rename to script/tool/test/validate_command_version_test.dart index a5f8230b1f1d..15a713190273 100644 --- a/script/tool/test/version_check_command_test.dart +++ b/script/tool/test/validate_command_version_test.dart @@ -5,8 +5,8 @@ import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:flutter_plugin_tools/src/common/core.dart'; +import 'package:flutter_plugin_tools/src/validate_command.dart'; import 'package:flutter_plugin_tools/src/validators/version_and_changelog_validator.dart'; -import 'package:flutter_plugin_tools/src/version_check_command.dart'; import 'package:git/git.dart'; import 'package:pub_semver/pub_semver.dart'; import 'package:test/test.dart'; @@ -50,16 +50,17 @@ void main() { (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); - final command = VersionCheckCommand( + final command = ValidateCommand( packagesDir, processRunner: processRunner, platform: mockPlatform, gitDir: gitDir, + targetedValidators: {Validator.version}, ); runner = CommandRunner( - 'version_check_command', - 'Test for $VersionCheckCommand', + 'validate_version_test', + 'Test for version and changelog validation', ); runner.addCommand(command); }); @@ -71,7 +72,7 @@ void main() { FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); @@ -101,7 +102,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -134,7 +135,7 @@ void main() { FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', ]); expect( @@ -162,7 +163,7 @@ void main() { test('allows valid version for new package.', () async { createFakePlugin('plugin', packagesDir, version: '1.0.0'); final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', ]); expect( @@ -181,7 +182,7 @@ void main() { FakeProcessInfo(MockProcess(stdout: 'version: 0.6.2')), ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); @@ -214,7 +215,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -246,7 +247,7 @@ void main() { FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); expect( @@ -281,7 +282,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -324,7 +325,7 @@ void main() { ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', '--pr-labels=some label,override: allow breaking change,another-label', ]); @@ -336,7 +337,7 @@ void main() { 'Allowing breaking change to plugin_platform_interface ' 'due to the "override: allow breaking change" label.', ), - contains('Ran for 1 package(s) (1 with warnings)'), + contains('(1 with warnings)'), ]), ); expect( @@ -363,7 +364,7 @@ void main() { FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', '--ignore-platform-interface-breaks', ]); @@ -375,7 +376,7 @@ void main() { 'Ignoring breaking change to plugin_platform_interface due ' 'to command configuration', ), - contains('Ran for 1 package(s) (1 with warnings)'), + contains('(1 with warnings)'), ]), ); expect( @@ -406,7 +407,7 @@ void main() { '''; plugin.changelogFile.writeAsStringSync(changelog); final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); expect( @@ -430,7 +431,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -460,7 +461,7 @@ void main() { '''; plugin.changelogFile.writeAsStringSync(changelog); final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); expect( @@ -495,7 +496,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -540,7 +541,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -576,7 +577,7 @@ void main() { var hasError = false; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { expect(e, isA()); hasError = true; @@ -617,7 +618,7 @@ void main() { ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); expect( @@ -651,7 +652,7 @@ void main() { var hasError = false; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { expect(e, isA()); hasError = true; @@ -698,7 +699,7 @@ void main() { var hasError = false; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { expect(e, isA()); hasError = true; @@ -744,7 +745,7 @@ void main() { var hasError = false; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { expect(e, isA()); hasError = true; @@ -784,7 +785,7 @@ void main() { ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); expect( @@ -815,7 +816,7 @@ void main() { plugin.changelogFile.writeAsStringSync(changelog); final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); expect( @@ -849,7 +850,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -893,7 +894,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -934,7 +935,7 @@ void main() { Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -955,7 +956,7 @@ void main() { void Function(Error error)? errorHandler, }) async { return runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', '--check-for-missing-changes', ...extraArgs, @@ -1644,7 +1645,7 @@ packages/plugin/lib/plugin.dart FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')), ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); @@ -1673,7 +1674,7 @@ packages/plugin/lib/plugin.dart FakeProcessInfo(MockProcess(stdout: 'version: 1.2.0-dev')), ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); @@ -1705,7 +1706,7 @@ packages/plugin/lib/plugin.dart FakeProcessInfo(MockProcess(stdout: 'version: 1.2.0-dev')), ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); @@ -1734,7 +1735,7 @@ packages/plugin/lib/plugin.dart FakeProcessInfo(MockProcess(stdout: 'version: 1.2.0-dev.1')), ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); @@ -1766,7 +1767,7 @@ packages/plugin/lib/plugin.dart Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -1801,7 +1802,7 @@ packages/plugin/lib/plugin.dart Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -1834,7 +1835,7 @@ packages/plugin/lib/plugin.dart Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -1873,7 +1874,7 @@ release: Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -1908,7 +1909,7 @@ release: ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); @@ -1935,7 +1936,7 @@ release: Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -1984,7 +1985,7 @@ packages/package/pubspec.yaml ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', '--pr-labels=override: post-release-package', ]); @@ -2030,7 +2031,7 @@ packages/package/CHANGELOG.md Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -2078,7 +2079,7 @@ packages/package/pubspec.yaml Error? commandError; final List output = await runCapturingPrint( runner, - ['version-check', '--base-sha=main'], + ['validate', '--base-sha=main'], errorHandler: (Error e) { commandError = e; }, @@ -2126,7 +2127,7 @@ packages/package/pubspec.yaml ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', ]); @@ -2167,7 +2168,7 @@ release: final List output = await runCapturingPrint( runner, [ - 'version-check', + 'validate', '--base-sha=main', '--check-for-missing-changes', ], @@ -2228,7 +2229,7 @@ packages/package/pending_changelogs/some_change.yaml ]; final List output = await runCapturingPrint(runner, [ - 'version-check', + 'validate', '--base-sha=main', '--check-for-missing-changes', ]); From 17060a261e51a3f9a2a7cf91af04e61dfea5e107 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 1 May 2026 10:39:53 -0400 Subject: [PATCH 14/18] Typo fix --- .../lib/src/validators/version_and_changelog_validator.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/tool/lib/src/validators/version_and_changelog_validator.dart b/script/tool/lib/src/validators/version_and_changelog_validator.dart index 12a885bb3beb..5ac2a2d7f6d9 100644 --- a/script/tool/lib/src/validators/version_and_changelog_validator.dart +++ b/script/tool/lib/src/validators/version_and_changelog_validator.dart @@ -587,7 +587,7 @@ ${_indentation}The first version listed in CHANGELOG.md is $fromChangeLog. if (missingVersionChange && missingChangelogChange) { printError( 'If this PR is not exempt, you can update version and ' - 'CHANGELOG with the "update-release-info" command.\\\n' + 'CHANGELOG with the "update-release-info" command.\n' 'See here for an example: ' 'https://github.com/flutter/packages/blob/main/script/tool/README.md#update-changelog-and-version\\\n' 'For more details on versioning, check the contributing guide.', From d940516667cad4c056b4498338d561eca78ebcf5 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 1 May 2026 10:53:02 -0400 Subject: [PATCH 15/18] Update agent instructions for command changes --- AGENTS.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 52580b5da637..4824e6216be4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,11 +90,9 @@ dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart update-dependency The tool can also run native and integration tests, but these may require a more complete environment than is available. - **Validation**: Run these checks to ensure that changes follow team guidelines: ```bash + dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart validate --packages dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart publish-check --packages - dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart readme-check --packages - dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart version-check --packages dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart license-check - dart run $REPO_ROOT/script/tool/bin/flutter_plugin_tools.dart validate ``` ### Specialized Workflows From 4af276b37c822545da0c41ebb8d4f016abefd15d Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 1 May 2026 10:57:05 -0400 Subject: [PATCH 16/18] Update docs --- script/tool/README.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/script/tool/README.md b/script/tool/README.md index 754a03d74e79..a25af39f1b87 100644 --- a/script/tool/README.md +++ b/script/tool/README.md @@ -49,7 +49,7 @@ dart run script/tool/bin/flutter_plugin_tools.dart format --packages package_nam The `flutter/packages` repository uses clang version `15.0.0` . Newer versions of clang may format code differently. -### Run the Static Analysis +### Run Static Analysis To analyze only Dart code: @@ -68,6 +68,18 @@ dart run script/tool/bin/flutter_plugin_tools.dart analyze --ios --macos --packa Dart analysis can be excluded with `--no-dart`. +### Run General Validation + +To check that changes follow team standards and best practices, run: + +```sh +dart run script/tool/bin/flutter_plugin_tools.dart validate --check-for-missing-changes --packages package_name +``` + +If you are making changes that fall under a CHANGELOG and/or version change +exemption you can omit the `--check-for-missing-changes` flag to skip those +checks. + ### Run Dart Unit Tests ```sh From 9ce590eb7cb6d7bd15b21ab79d338f315f31839d Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Mon, 4 May 2026 10:34:22 -0400 Subject: [PATCH 17/18] Autoformat --- script/tool/lib/src/common/repository_package.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/tool/lib/src/common/repository_package.dart b/script/tool/lib/src/common/repository_package.dart index f495f5d41c6b..78bb8f91fd42 100644 --- a/script/tool/lib/src/common/repository_package.dart +++ b/script/tool/lib/src/common/repository_package.dart @@ -166,7 +166,7 @@ class RepositoryPackage { isFederated && !isPlatformInterface && directory.basename != directory.parent.basename; - + /// True if this appears to be a top-level package, according to repository /// conventions. bool get isTopLevel => getEnclosingPackage() == null; From 5bbe4988d68f9957ffabdfc1dff4a745e375dd0d Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Mon, 4 May 2026 10:43:23 -0400 Subject: [PATCH 18/18] Mark ci_config.yaml as a dev-only change --- script/tool/lib/src/common/package_state_utils.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/script/tool/lib/src/common/package_state_utils.dart b/script/tool/lib/src/common/package_state_utils.dart index 27043ab0f376..e6d7def3dc5d 100644 --- a/script/tool/lib/src/common/package_state_utils.dart +++ b/script/tool/lib/src/common/package_state_utils.dart @@ -221,6 +221,8 @@ Future _isDevChange( pathComponents.contains('lint-baseline.xml') || // Example build files are very unlikely to be interesting to clients. _isExampleBuildFile(pathComponents) || + // CI config files don't affect clients. + pathComponents.last == 'ci_config.yaml' || // Test-only gradle depenedencies don't affect clients. await _isGradleTestDependencyChange( pathComponents,