From e04ba886a85edfd4fcea794d0e06be062953d480 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Tue, 12 Sep 2023 08:21:00 -0700 Subject: [PATCH] [tool] Add a package inclusion filter (#4904) Creates a package filter flag that is an opt-in mirror to the `exclude` flag, for use with the automatic package selection options used in CI. Like `exclude`, it allows for YAML files as input. This allows for creating split test runs in CI (e.g., during incremental migrations), where the same file is an inclusion filter for one run and an exclusion filter for the other, guaranteeing that tests are in one or the other without the possibility of some tests falling through the cracks. --- .../tool/lib/src/common/package_command.dart | 47 +++++++++++++++---- .../test/common/package_command_test.dart | 40 +++++++++++++++- 2 files changed, 76 insertions(+), 11 deletions(-) diff --git a/script/tool/lib/src/common/package_command.dart b/script/tool/lib/src/common/package_command.dart index fdfb54ee7d461..c16783db3a7e3 100644 --- a/script/tool/lib/src/common/package_command.dart +++ b/script/tool/lib/src/common/package_command.dart @@ -98,6 +98,14 @@ abstract class PackageCommand extends Command { 'Cannot be combined with $_packagesArg.\n\n' 'This is intended for use in CI.\n', hide: true); + argParser.addMultiOption(_filterPackagesArg, + help: 'Filters any selected packages to only those included in this ' + 'list. This is intended for use in CI with flags such as ' + '--$_packagesForBranchArg.\n\n' + 'Entries can be package names or YAML files that contain a list ' + 'of package names.', + defaultsTo: [], + hide: true); argParser.addFlag(_currentPackageArg, negatable: false, help: @@ -129,6 +137,7 @@ abstract class PackageCommand extends Command { static const String _runOnChangedPackagesArg = 'run-on-changed-packages'; static const String _runOnDirtyPackagesArg = 'run-on-dirty-packages'; static const String _excludeArg = 'exclude'; + static const String _filterPackagesArg = 'filter-packages-to'; // Diff base selection. static const String _baseBranchArg = 'base-branch'; static const String _baseShaArg = 'base-sha'; @@ -255,18 +264,25 @@ abstract class PackageCommand extends Command { _shardCount = shardCount; } + /// Converts a list of items which are either package names or yaml files + /// containing a list of package names to a flat list of package names by + /// reading all the file contents. + Set _expandYamlInPackageList(List items) { + return items.expand((String item) { + if (item.endsWith('.yaml')) { + final File file = packagesDir.fileSystem.file(item); + return (loadYaml(file.readAsStringSync()) as YamlList) + .toList() + .cast(); + } + return [item]; + }).toSet(); + } + /// Returns the set of packages to exclude based on the `--exclude` argument. Set getExcludedPackageNames() { final Set excludedPackages = _excludedPackages ?? - getStringListArg(_excludeArg).expand((String item) { - if (item.endsWith('.yaml')) { - final File file = packagesDir.fileSystem.file(item); - return (loadYaml(file.readAsStringSync()) as YamlList) - .toList() - .cast(); - } - return [item]; - }).toSet(); + _expandYamlInPackageList(getStringListArg(_excludeArg)); // Cache for future calls. _excludedPackages = excludedPackages; return excludedPackages; @@ -420,7 +436,18 @@ abstract class PackageCommand extends Command { packages = {currentPackageName}; } - final Set excludedPackageNames = getExcludedPackageNames(); + Set excludedPackageNames = getExcludedPackageNames(); + + final Set filter = + _expandYamlInPackageList(getStringListArg(_filterPackagesArg)); + if (filter.isNotEmpty) { + final List sortedList = filter.toList()..sort(); + print('--$_filterPackagesArg is excluding packages that are not ' + 'included in: ${sortedList.join(',')}'); + excludedPackageNames = + excludedPackageNames.union(packages.difference(filter)); + } + for (final Directory dir in [ packagesDir, if (thirdPartyPackagesDir.existsSync()) thirdPartyPackagesDir, diff --git a/script/tool/test/common/package_command_test.dart b/script/tool/test/common/package_command_test.dart index ed0e905ee5e70..e269b063886af 100644 --- a/script/tool/test/common/package_command_test.dart +++ b/script/tool/test/common/package_command_test.dart @@ -204,6 +204,21 @@ void main() { expect(command.plugins, unorderedEquals([])); }); + test('filter-packages-to accepts config files', () async { + final RepositoryPackage plugin1 = + createFakePlugin('plugin1', packagesDir); + createFakePlugin('plugin2', packagesDir); + final File configFile = packagesDir.childFile('exclude.yaml'); + configFile.writeAsStringSync('- plugin1'); + + await runCapturingPrint(runner, [ + 'sample', + '--packages=plugin1,plugin2', + '--filter-packages-to=${configFile.path}' + ]); + expect(command.plugins, unorderedEquals([plugin1.path])); + }); + test( 'explicitly specifying the plugin (group) name of a federated plugin ' 'should include all plugins in the group', () async { @@ -765,7 +780,7 @@ packages/plugin1/plugin1/plugin1.dart expect(command.plugins, unorderedEquals([plugin1.path])); }); - test('--exclude flag works with --run-on-changed-packages', () async { + test('honors --exclude flag', () async { processRunner.mockProcessesForExecutable['git-diff'] = [ FakeProcessInfo(MockProcess(stdout: ''' @@ -787,6 +802,29 @@ packages/plugin3/plugin3.dart expect(command.plugins, unorderedEquals([plugin1.path])); }); + + test('honors --filter-packages-to flag', () async { + processRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/plugin1/plugin1.dart +packages/plugin2/ios/plugin2.m +packages/plugin3/plugin3.dart +''')), + ]; + final RepositoryPackage plugin1 = + createFakePlugin('plugin1', packagesDir.childDirectory('plugin1')); + createFakePlugin('plugin2', packagesDir); + createFakePlugin('plugin3', packagesDir); + await runCapturingPrint(runner, [ + 'sample', + '--filter-packages-to=plugin1', + '--base-sha=main', + '--run-on-changed-packages' + ]); + + expect(command.plugins, unorderedEquals([plugin1.path])); + }); }); group('test run-on-dirty-packages', () {