Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
auto-submit merged 2 commits into from
Oct 19, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad45edb
[tool] Fix `filter-packages-to` when everything is changed
stuartmorgan-g Oct 19, 2023
771f377
Improve comment
stuartmorgan-g Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions script/tool/lib/src/common/package_command.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>[
Expand All @@ -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
Expand All @@ -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));
}
}
}
Expand Down
74 changes: 74 additions & 0 deletions script/tool/test/common/package_command_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () {
Expand Down