[tool] Fix filter-packages-to when everything is changed (#5182)

`filter-packages-to` didn't correctly the handle the case where the set of target packages is empty, meaning that all packages should be tested. This broke it for cases such as changing a CI configuration file, making the filter not take effect.
This commit is contained in:
stuartmorgan
2023-10-19 08:28:16 -07:00
committed by GitHub
parent 24654d3d8c
commit c4c3e4eba9
2 changed files with 94 additions and 12 deletions

View File

@ -436,16 +436,26 @@ abstract class PackageCommand extends Command<void> {
packages = <String>{currentPackageName};
}
Set<String> excludedPackageNames = getExcludedPackageNames();
final Set<String> filter =
_expandYamlInPackageList(getStringListArg(_filterPackagesArg));
if (filter.isNotEmpty) {
final List<String> sortedList = filter.toList()..sort();
final Set<String> excludedPackageNames = getExcludedPackageNames();
final bool hasFilter = argResults?.wasParsed(_filterPackagesArg) ?? false;
final Set<String>? excludeAllButPackageNames = hasFilter
? _expandYamlInPackageList(getStringListArg(_filterPackagesArg))
: null;
if (excludeAllButPackageNames != null &&
excludeAllButPackageNames.isNotEmpty) {
final List<String> sortedList = excludeAllButPackageNames.toList()
..sort();
print('--$_filterPackagesArg is excluding packages that are not '
'included in: ${sortedList.join(',')}');
excludedPackageNames =
excludedPackageNames.union(packages.difference(filter));
}
// Returns true if a package that could be identified by any of
// `possibleNames` should be excluded.
bool isExcluded(Set<String> possibleNames) {
if (excludedPackageNames.intersection(possibleNames).isNotEmpty) {
return true;
}
return excludeAllButPackageNames != null &&
excludeAllButPackageNames.intersection(possibleNames).isEmpty;
}
for (final Directory dir in <Directory>[
@ -459,7 +469,7 @@ abstract class PackageCommand extends Command<void> {
if (packages.isEmpty || packages.contains(p.basename(entity.path))) {
yield PackageEnumerationEntry(
RepositoryPackage(entity as Directory),
excluded: excludedPackageNames.contains(entity.basename));
excluded: isExcluded(<String>{entity.basename}));
}
} else if (entity is Directory) {
// Look for Dart packages under this top-level directory; this is the
@ -481,9 +491,7 @@ abstract class PackageCommand extends Command<void> {
packages.intersection(possibleMatches).isNotEmpty) {
yield PackageEnumerationEntry(
RepositoryPackage(subdir as Directory),
excluded: excludedPackageNames
.intersection(possibleMatches)
.isNotEmpty);
excluded: isExcluded(possibleMatches));
}
}
}

View File

@ -825,6 +825,80 @@ packages/plugin3/plugin3.dart
expect(command.plugins, unorderedEquals(<String>[plugin1.path]));
});
test(
'honors --filter-packages-to flag when a file is changed that makes '
'all packages potentially changed', () async {
processRunner.mockProcessesForExecutable['git-diff'] =
<FakeProcessInfo>[
FakeProcessInfo(MockProcess(stdout: '''
.ci.yaml
''')),
];
final RepositoryPackage plugin1 =
createFakePlugin('plugin1', packagesDir.childDirectory('plugin1'));
createFakePlugin('plugin2', packagesDir);
createFakePlugin('plugin3', packagesDir);
await runCapturingPrint(runner, <String>[
'sample',
'--filter-packages-to=plugin1',
'--base-sha=main',
'--run-on-changed-packages'
]);
expect(command.plugins, unorderedEquals(<String>[plugin1.path]));
});
test('--filter-packages-to handles federated plugin groups', () async {
processRunner.mockProcessesForExecutable['git-diff'] =
<FakeProcessInfo>[
FakeProcessInfo(MockProcess(stdout: '''
packages/a_plugin/a_plugin/lib/foo.dart
packages/a_plugin/a_plugin_impl/lib/foo.dart
packages/a_plugin/a_plugin_platform_interface/lib/foo.dart
''')),
];
final Directory groupDir = packagesDir.childDirectory('a_plugin');
final RepositoryPackage plugin1 =
createFakePlugin('a_plugin', groupDir);
final RepositoryPackage plugin2 =
createFakePlugin('a_plugin_impl', groupDir);
final RepositoryPackage plugin3 =
createFakePlugin('a_plugin_platform_interface', groupDir);
await runCapturingPrint(runner, <String>[
'sample',
'--filter-packages-to=a_plugin',
'--base-sha=main',
'--run-on-changed-packages'
]);
expect(
command.plugins,
unorderedEquals(
<String>[plugin1.path, plugin2.path, plugin3.path]));
});
test('--filter-packages-to and --exclude work together', () async {
processRunner.mockProcessesForExecutable['git-diff'] =
<FakeProcessInfo>[
FakeProcessInfo(MockProcess(stdout: '''
.ci.yaml
''')),
];
final RepositoryPackage plugin1 =
createFakePlugin('plugin1', packagesDir.childDirectory('plugin1'));
createFakePlugin('plugin2', packagesDir);
createFakePlugin('plugin3', packagesDir);
await runCapturingPrint(runner, <String>[
'sample',
'--filter-packages-to=plugin1,plugin2',
'--exclude=plugin2',
'--base-sha=main',
'--run-on-changed-packages'
]);
expect(command.plugins, unorderedEquals(<String>[plugin1.path]));
});
});
group('test run-on-dirty-packages', () {