Skip to content

Commit b2e7c6e

Browse files
stuartmorgan-gandroidseb
authored andcommitted
[tool] Move changed file detection to base command class (flutter#8730)
Consolidates the code to find all changed file paths into the `PackageLoopingCommand` class that is the base of almost all of the repo tooling commands. This in a preparatory PR for a future change to allow each command to define a list of files or file patterns that definitively *don't* affect that test, so that CI can be smarter about what tests to run (e.g., not running expensive integration tests for README changes). A side effect of this change is that tests of almost all commands now need a mock `GitDir` instance. This would add a lot of copy/pasted boilerplate to the test setup, and there is already too much of that, so instead this refactors common test setup: - Creating a memory file system - Populating it with a packages directory - Creating a RecordingProcessRunner to mock out process calls - Creating a mock GitDir that forwards to a RecordingProcessRunner into a helper method (using records and destructuring to easily return multiple values). While some tests don't need all of these steps, those that don't can easily ignore parts of it, and it will make it much easier to update tests in the future if they need them, and it makes the setup much more consistent which makes it easier to reason about test setup in general. Prep for flutter/flutter#136394
1 parent 4122348 commit b2e7c6e

65 files changed

Lines changed: 586 additions & 632 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

script/tool/lib/src/analyze_command.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ class AnalyzeCommand extends PackageLoopingCommand {
1717
super.packagesDir, {
1818
super.processRunner,
1919
super.platform,
20+
super.gitDir,
2021
}) {
2122
argParser.addMultiOption(_customAnalysisFlag,
2223
help:

script/tool/lib/src/build_examples_command.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ class BuildExamplesCommand extends PackageLoopingCommand {
4848
super.packagesDir, {
4949
super.processRunner,
5050
super.platform,
51+
super.gitDir,
5152
}) {
5253
argParser.addFlag(platformLinux);
5354
argParser.addFlag(platformMacOS);

script/tool/lib/src/common/git_version_finder.dart

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,6 @@ class GitVersionFinder {
2828
/// The base branche used to find a merge point if baseSha is not provided.
2929
final String _baseBranch;
3030

31-
static bool _isPubspec(String file) {
32-
return file.trim().endsWith('pubspec.yaml');
33-
}
34-
35-
/// Get a list of all the pubspec.yaml file that is changed.
36-
Future<List<String>> getChangedPubSpecs() async {
37-
return (await getChangedFiles()).where(_isPubspec).toList();
38-
}
39-
4031
/// Get a list of all the changed files.
4132
Future<List<String>> getChangedFiles(
4233
{bool includeUncommitted = false}) async {

script/tool/lib/src/common/package_looping_command.dart

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import 'package:path/path.dart' as p;
99
import 'package:pub_semver/pub_semver.dart';
1010

1111
import 'core.dart';
12+
import 'git_version_finder.dart';
1213
import 'output_utils.dart';
1314
import 'package_command.dart';
1415
import 'repository_package.dart';
@@ -226,6 +227,17 @@ abstract class PackageLoopingCommand extends PackageCommand {
226227
/// The suggested indentation for printed output.
227228
String get indentation => hasLongOutput ? '' : ' ';
228229

230+
/// The base SHA used to calculate changed files.
231+
///
232+
/// This is guaranteed to be populated before [initializeRun] is called.
233+
late String baseSha;
234+
235+
/// The repo-relative paths (using Posix separators) of all files changed
236+
/// relative to [baseSha].
237+
///
238+
/// This is guaranteed to be populated before [initializeRun] is called.
239+
late List<String> changedFiles;
240+
229241
// ----------------------------------------
230242

231243
@override
@@ -264,6 +276,11 @@ abstract class PackageLoopingCommand extends PackageCommand {
264276

265277
final DateTime runStart = DateTime.now();
266278

279+
// Populate the list of changed files for subclasses to use.
280+
final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
281+
baseSha = await gitVersionFinder.getBaseSha();
282+
changedFiles = await gitVersionFinder.getChangedFiles();
283+
267284
await initializeRun();
268285

269286
final List<PackageEnumerationEntry> targetPackages =

script/tool/lib/src/custom_test_command.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class CustomTestCommand extends PackageLoopingCommand {
2121
super.packagesDir, {
2222
super.processRunner,
2323
super.platform,
24+
super.gitDir,
2425
});
2526

2627
@override

script/tool/lib/src/dart_test_command.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class DartTestCommand extends PackageLoopingCommand {
2727
super.packagesDir, {
2828
super.processRunner,
2929
super.platform,
30+
super.gitDir,
3031
}) {
3132
argParser.addOption(
3233
kEnableExperiment,

script/tool/lib/src/drive_examples_command.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class DriveExamplesCommand extends PackageLoopingCommand {
2727
super.packagesDir, {
2828
super.processRunner,
2929
super.platform,
30+
super.gitDir,
3031
}) {
3132
argParser.addFlag(platformAndroid,
3233
help: 'Runs the Android implementation of the examples',

script/tool/lib/src/federation_safety_check_command.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,8 @@ class FederationSafetyCheckCommand extends PackageLoopingCommand {
5959
@override
6060
Future<void> initializeRun() async {
6161
final GitVersionFinder gitVersionFinder = await retrieveVersionFinder();
62-
final String baseSha = await gitVersionFinder.getBaseSha();
6362
print('Validating changes relative to "$baseSha"\n');
64-
for (final String path in await gitVersionFinder.getChangedFiles()) {
63+
for (final String path in changedFiles) {
6564
// Git output always uses Posix paths.
6665
final List<String> allComponents = p.posix.split(path);
6766
final int packageIndex = allComponents.indexOf('packages');

script/tool/lib/src/fetch_deps_command.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class FetchDepsCommand extends PackageLoopingCommand {
2727
super.packagesDir, {
2828
super.processRunner,
2929
super.platform,
30+
super.gitDir,
3031
}) {
3132
argParser.addFlag(_dartFlag, defaultsTo: true, help: 'Run "pub get"');
3233
argParser.addFlag(_supportingTargetPlatformsOnlyFlag,

script/tool/lib/src/firebase_test_lab_command.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class FirebaseTestLabCommand extends PackageLoopingCommand {
2323
super.packagesDir, {
2424
super.processRunner,
2525
super.platform,
26+
super.gitDir,
2627
}) {
2728
argParser.addOption(
2829
_gCloudProjectArg,

0 commit comments

Comments
 (0)