diff --git a/script/tool/lib/src/analyze_command.dart b/script/tool/lib/src/analyze_command.dart index 4651dbb42c7..51a2403ab2e 100644 --- a/script/tool/lib/src/analyze_command.dart +++ b/script/tool/lib/src/analyze_command.dart @@ -6,6 +6,7 @@ import 'dart:io' as io; import 'package:file/file.dart'; +import 'common/file_filters.dart'; import 'common/output_utils.dart'; import 'common/package_looping_command.dart'; import 'common/repository_package.dart'; @@ -92,6 +93,13 @@ class AnalyzeCommand extends PackageLoopingCommand { return false; } + @override + bool shouldIgnoreFile(String path) { + return isRepoLevelNonCodeImpactingFile(path) || + isNativeCodeFile(path) || + isPackageSupportFile(path); + } + @override Future initializeRun() async { _allowedCustomAnalysisDirectories = getYamlListArg(_customAnalysisFlag); diff --git a/script/tool/lib/src/build_examples_command.dart b/script/tool/lib/src/build_examples_command.dart index 4a5c0272853..932e3090a39 100644 --- a/script/tool/lib/src/build_examples_command.dart +++ b/script/tool/lib/src/build_examples_command.dart @@ -6,6 +6,7 @@ import 'package:file/file.dart'; import 'package:yaml/yaml.dart'; import 'common/core.dart'; +import 'common/file_filters.dart'; import 'common/output_utils.dart'; import 'common/package_looping_command.dart'; import 'common/plugin_utils.dart'; @@ -134,6 +135,11 @@ class BuildExamplesCommand extends PackageLoopingCommand { return getNullableBoolArg(_swiftPackageManagerFlag); } + @override + bool shouldIgnoreFile(String path) { + return isRepoLevelNonCodeImpactingFile(path) || isPackageSupportFile(path); + } + @override Future initializeRun() async { final List platformFlags = _platforms.keys.toList(); diff --git a/script/tool/lib/src/common/file_filters.dart b/script/tool/lib/src/common/file_filters.dart new file mode 100644 index 00000000000..b1eb0899f29 --- /dev/null +++ b/script/tool/lib/src/common/file_filters.dart @@ -0,0 +1,53 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/// Returns true for repository-level paths of files that do not affect *any* +/// code-related commands (example builds, Dart analysis, native code analysis, +/// native tests, Dart tests, etc.) for use in command-ignored-files lists for +/// commands that are only affected by package code. +bool isRepoLevelNonCodeImpactingFile(String path) { + return [ + 'AUTHORS', + 'CODEOWNERS', + 'CONTRIBUTING.md', + 'LICENSE', + 'README.md', + // This deliberate lists specific files rather than excluding the whole + // .github directory since it's better to have false negatives than to + // accidentally skip tests if something is later added to the directory + // that could affect packages. + '.github/PULL_REQUEST_TEMPLATE.md', + '.github/dependabot.yml', + '.github/labeler.yml', + '.github/post_merge_labeler.yml', + '.github/workflows/pull_request_label.yml', + ].contains(path); +} + +/// Returns true for native (non-Dart) code files, for use in command-ignored- +/// files lists for commands that aren't affected by native code (e.g., Dart +/// analysis and unit tests). +bool isNativeCodeFile(String path) { + return path.endsWith('.c') || + path.endsWith('.cc') || + path.endsWith('.cpp') || + path.endsWith('.h') || + path.endsWith('.m') || + path.endsWith('.swift') || + path.endsWith('.java') || + path.endsWith('.kt'); +} + +/// Returns true for package-level human-focused support files, for use in +/// command-ignored-files lists for commands that aren't affected by files that +/// aren't used in any builds. +/// +/// This must *not* include metadata files that do affect builds, such as +/// pubspec.yaml. +bool isPackageSupportFile(String path) { + return path.endsWith('/AUTHORS') || + path.endsWith('/CHANGELOG.md') || + path.endsWith('/CONTRIBUTING.md') || + path.endsWith('/README.md'); +} diff --git a/script/tool/lib/src/common/package_looping_command.dart b/script/tool/lib/src/common/package_looping_command.dart index 4ac90ae80b6..6ac17df37c0 100644 --- a/script/tool/lib/src/common/package_looping_command.dart +++ b/script/tool/lib/src/common/package_looping_command.dart @@ -109,6 +109,20 @@ abstract class PackageLoopingCommand extends PackageCommand { /// The package currently being run by [runForPackage]. PackageEnumerationEntry? _currentPackageEntry; + /// When running against a merge base, this is called before [initializeRun] + /// for every changed file, to see if that file is a file that is guaranteed + /// *not* to require running this command. + /// + /// If every changed file returns true, then the command will be skipped. + /// Because this causes tests not to run, subclasses should be very + /// consevative about what returns true; for anything borderline it is much + /// better to err on the side of running tests unnecessarily than to risk + /// losing test coverage. + /// + /// [path] is a POSIX-style path regardless of the host platforrm, and is + /// relative to the git repo root. + bool shouldIgnoreFile(String path) => false; + /// Called during [run] before any calls to [runForPackage]. This provides an /// opportunity to fail early if the command can't be run (e.g., because the /// arguments are invalid), and to set up any run-level state. @@ -281,6 +295,14 @@ abstract class PackageLoopingCommand extends PackageCommand { baseSha = await gitVersionFinder.getBaseSha(); changedFiles = await gitVersionFinder.getChangedFiles(); + // Check whether the command needs to run. + if (changedFiles.isNotEmpty && changedFiles.every(shouldIgnoreFile)) { + _printColorized( + 'SKIPPING ALL PACKAGES: No changed files affect this command', + Styles.DARK_GRAY); + return true; + } + await initializeRun(); final List targetPackages = diff --git a/script/tool/lib/src/dart_test_command.dart b/script/tool/lib/src/dart_test_command.dart index 5e99211169e..f5189e557cb 100644 --- a/script/tool/lib/src/dart_test_command.dart +++ b/script/tool/lib/src/dart_test_command.dart @@ -5,6 +5,7 @@ import 'package:file/file.dart'; import 'common/core.dart'; +import 'common/file_filters.dart'; import 'common/output_utils.dart'; import 'common/package_looping_command.dart'; import 'common/plugin_utils.dart'; @@ -65,6 +66,13 @@ class DartTestCommand extends PackageLoopingCommand { PackageLoopingType get packageLoopingType => PackageLoopingType.includeAllSubpackages; + @override + bool shouldIgnoreFile(String path) { + return isRepoLevelNonCodeImpactingFile(path) || + isNativeCodeFile(path) || + isPackageSupportFile(path); + } + @override Future runForPackage(RepositoryPackage package) async { if (!package.testDirectory.existsSync()) { diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index c26c4850072..84689bcb5aa 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -9,6 +9,7 @@ import 'dart:io'; import 'package:file/file.dart'; import 'common/core.dart'; +import 'common/file_filters.dart'; import 'common/output_utils.dart'; import 'common/package_looping_command.dart'; import 'common/plugin_utils.dart'; @@ -68,6 +69,11 @@ class DriveExamplesCommand extends PackageLoopingCommand { Map> _targetDeviceFlags = const >{}; + @override + bool shouldIgnoreFile(String path) { + return isRepoLevelNonCodeImpactingFile(path) || isPackageSupportFile(path); + } + @override Future initializeRun() async { final List platformSwitches = [ diff --git a/script/tool/lib/src/firebase_test_lab_command.dart b/script/tool/lib/src/firebase_test_lab_command.dart index 445a5aec511..9a95507f9a3 100644 --- a/script/tool/lib/src/firebase_test_lab_command.dart +++ b/script/tool/lib/src/firebase_test_lab_command.dart @@ -8,6 +8,7 @@ import 'package:file/file.dart'; import 'package:uuid/uuid.dart'; import 'common/core.dart'; +import 'common/file_filters.dart'; import 'common/flutter_command_utils.dart'; import 'common/gradle.dart'; import 'common/output_utils.dart'; @@ -122,6 +123,11 @@ class FirebaseTestLabCommand extends PackageLoopingCommand { _firebaseProjectConfigured = true; } + @override + bool shouldIgnoreFile(String path) { + return isRepoLevelNonCodeImpactingFile(path) || isPackageSupportFile(path); + } + @override Future runForPackage(RepositoryPackage package) async { final List results = []; diff --git a/script/tool/lib/src/lint_android_command.dart b/script/tool/lib/src/lint_android_command.dart index e9d49b9b459..b7dff59a68d 100644 --- a/script/tool/lib/src/lint_android_command.dart +++ b/script/tool/lib/src/lint_android_command.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'common/core.dart'; +import 'common/file_filters.dart'; import 'common/flutter_command_utils.dart'; import 'common/gradle.dart'; import 'common/output_utils.dart'; @@ -29,6 +30,15 @@ class LintAndroidCommand extends PackageLoopingCommand { final String description = 'Runs "gradlew lint" on Android plugins.\n\n' 'Requires the examples to have been build at least once before running.'; + @override + bool shouldIgnoreFile(String path) { + return isRepoLevelNonCodeImpactingFile(path) || + isPackageSupportFile(path) || + // These are part of the build, but don't affect native code analysis. + path.endsWith('/pubspec.yaml') || + path.endsWith('.dart'); + } + @override Future runForPackage(RepositoryPackage package) async { if (!pluginSupportsPlatform(platformAndroid, package, diff --git a/script/tool/lib/src/native_test_command.dart b/script/tool/lib/src/native_test_command.dart index 39630189ae7..35498b4c29f 100644 --- a/script/tool/lib/src/native_test_command.dart +++ b/script/tool/lib/src/native_test_command.dart @@ -9,6 +9,7 @@ import 'package:meta/meta.dart'; import 'common/cmake.dart'; import 'common/core.dart'; +import 'common/file_filters.dart'; import 'common/flutter_command_utils.dart'; import 'common/gradle.dart'; import 'common/output_utils.dart'; @@ -114,6 +115,13 @@ this command. Set _xcodeWarningsExceptions = {}; + @override + bool shouldIgnoreFile(String path) { + return isRepoLevelNonCodeImpactingFile(path) || isPackageSupportFile(path); + // It may seem tempting to filter out *.dart, but that would skip critical + // testing since native integration tests run the full compiled application. + } + @override Future initializeRun() async { _platforms = { diff --git a/script/tool/lib/src/xcode_analyze_command.dart b/script/tool/lib/src/xcode_analyze_command.dart index 7629a4e7346..8494550bc77 100644 --- a/script/tool/lib/src/xcode_analyze_command.dart +++ b/script/tool/lib/src/xcode_analyze_command.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'common/core.dart'; +import 'common/file_filters.dart'; import 'common/flutter_command_utils.dart'; import 'common/output_utils.dart'; import 'common/package_looping_command.dart'; @@ -47,6 +48,15 @@ class XcodeAnalyzeCommand extends PackageLoopingCommand { final String description = 'Runs Xcode analysis on the iOS and/or macOS example apps.'; + @override + bool shouldIgnoreFile(String path) { + return isRepoLevelNonCodeImpactingFile(path) || + isPackageSupportFile(path) || + // These are part of the build, but don't affect native code analysis. + path.endsWith('/pubspec.yaml') || + path.endsWith('.dart'); + } + @override Future initializeRun() async { if (!(getBoolArg(platformIOS) || getBoolArg(platformMacOS))) { diff --git a/script/tool/test/analyze_command_test.dart b/script/tool/test/analyze_command_test.dart index f27503cabff..b60cd63b9c5 100644 --- a/script/tool/test/analyze_command_test.dart +++ b/script/tool/test/analyze_command_test.dart @@ -16,12 +16,13 @@ void main() { late MockPlatform mockPlatform; late Directory packagesDir; late RecordingProcessRunner processRunner; + late RecordingProcessRunner gitProcessRunner; late CommandRunner runner; setUp(() { mockPlatform = MockPlatform(); final GitDir gitDir; - (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = + (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); final AnalyzeCommand analyzeCommand = AnalyzeCommand( packagesDir, @@ -470,4 +471,90 @@ void main() { ]), ); }); + + group('file filtering', () { + test('runs command for changes to Dart source', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package_a/foo.dart +''')), + ]; + + final List output = + await runCapturingPrint(runner, ['analyze']); + + expect( + output, + containsAllInOrder([ + contains('Running for package_a'), + ])); + }); + + const List files = [ + 'foo.java', + 'foo.kt', + 'foo.m', + 'foo.swift', + 'foo.c', + 'foo.cc', + 'foo.cpp', + 'foo.h', + ]; + for (final String file in files) { + test('skips command for changes to non-Dart source $file', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package_a/$file +''')), + ]; + + final List output = + await runCapturingPrint(runner, ['analyze']); + + expect( + output, + isNot(containsAllInOrder([ + contains('Running for package_a'), + ]))); + expect( + output, + containsAllInOrder([ + contains('SKIPPING ALL PACKAGES'), + ])); + }); + } + + test('skips commands if all files should be ignored', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +README.md +CODEOWNERS +packages/package_a/CHANGELOG.md +''')), + ]; + + final List output = + await runCapturingPrint(runner, ['analyze']); + + expect( + output, + isNot(containsAllInOrder([ + contains('Running for package_a'), + ]))); + expect( + output, + containsAllInOrder([ + contains('SKIPPING ALL PACKAGES'), + ])); + }); + }); } diff --git a/script/tool/test/build_examples_command_test.dart b/script/tool/test/build_examples_command_test.dart index 2e2a2112baa..98ebe28ffa5 100644 --- a/script/tool/test/build_examples_command_test.dart +++ b/script/tool/test/build_examples_command_test.dart @@ -19,11 +19,12 @@ void main() { late Directory packagesDir; late CommandRunner runner; late RecordingProcessRunner processRunner; + late RecordingProcessRunner gitProcessRunner; setUp(() { mockPlatform = MockPlatform(); final GitDir gitDir; - (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = + (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); final BuildExamplesCommand command = BuildExamplesCommand( packagesDir, @@ -998,5 +999,72 @@ void main() { pluginExampleDirectory.path), ])); }); + + group('file filtering', () { + const List files = [ + 'pubspec.yaml', + 'foo.dart', + 'foo.java', + 'foo.kt', + 'foo.m', + 'foo.swift', + 'foo.cc', + 'foo.cpp', + 'foo.h', + ]; + for (final String file in files) { + test('runs command for changes to $file', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package_a/$file +''')), + ]; + + // The target platform is irrelevant here; because this repo's + // packages are fully federated, there's no need to distinguish + // the ignore list by target (e.g., skipping iOS tests if only Java or + // Kotlin files change), because package-level filering will already + // accomplish the same goal. + final List output = await runCapturingPrint( + runner, ['build-examples', '--web']); + + expect( + output, + containsAllInOrder([ + contains('Running for package_a'), + ])); + }); + } + + test('skips commands if all files should be ignored', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +README.md +CODEOWNERS +packages/package_a/CHANGELOG.md +''')), + ]; + + final List output = + await runCapturingPrint(runner, ['build-examples']); + + expect( + output, + isNot(containsAllInOrder([ + contains('Running for package_a'), + ]))); + expect( + output, + containsAllInOrder([ + contains('SKIPPING ALL PACKAGES'), + ])); + }); + }); }); } diff --git a/script/tool/test/common/package_looping_command_test.dart b/script/tool/test/common/package_looping_command_test.dart index 6474dff8025..d2247b9d488 100644 --- a/script/tool/test/common/package_looping_command_test.dart +++ b/script/tool/test/common/package_looping_command_test.dart @@ -10,12 +10,11 @@ import 'package:file/file.dart'; import 'package:flutter_plugin_tools/src/common/core.dart'; import 'package:flutter_plugin_tools/src/common/output_utils.dart'; import 'package:flutter_plugin_tools/src/common/package_looping_command.dart'; -import 'package:mockito/mockito.dart'; +import 'package:git/git.dart'; import 'package:test/test.dart'; import '../mocks.dart'; import '../util.dart'; -import 'package_command_test.mocks.dart'; // Constants for colorized output start and end. const String _startElapsedTimeColor = '\x1B[90m'; @@ -79,10 +78,12 @@ void main() { late MockPlatform mockPlatform; late Directory packagesDir; late Directory thirdPartyPackagesDir; + late GitDir gitDir; + late RecordingProcessRunner gitProcessRunner; setUp(() { mockPlatform = MockPlatform(); - (:packagesDir, processRunner: _, gitProcessRunner: _, gitDir: _) = + (:packagesDir, processRunner: _, :gitProcessRunner, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); // Correct color handling is part of the behavior being tested here. useColorForOutput = true; @@ -96,10 +97,8 @@ void main() { useColorForOutput = io.stdout.supportsAnsiEscapes; }); - /// Creates a TestPackageLoopingCommand instance that uses [gitDiffResponse] - /// for git diffs, and logs output to [printOutput]. + /// Creates a TestPackageLoopingCommand with the given configuration. TestPackageLoopingCommand createTestCommand({ - String gitDiffResponse = '', bool hasLongOutput = true, PackageLoopingType packageLoopingType = PackageLoopingType.topLevelOnly, bool failsDuringInit = false, @@ -108,20 +107,6 @@ void main() { String? customFailureListHeader, String? customFailureListFooter, }) { - // Set up the git diff response. - final MockGitDir gitDir = MockGitDir(); - when(gitDir.runCommand(any, throwOnError: anyNamed('throwOnError'))) - .thenAnswer((Invocation invocation) { - final List arguments = - invocation.positionalArguments[0]! as List; - String? gitStdOut; - if (arguments[0] == 'diff') { - gitStdOut = gitDiffResponse; - } - return Future.value( - io.ProcessResult(0, 0, gitStdOut ?? '', '')); - }); - return TestPackageLoopingCommand( packagesDir, platform: mockPlatform, @@ -212,6 +197,77 @@ void main() { }); }); + group('file filtering', () { + test('runs command if the changed files list is empty', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: '')), + ]; + + final TestPackageLoopingCommand command = + createTestCommand(hasLongOutput: false); + final List output = await runCommand(command); + + expect( + output, + containsAllInOrder([ + '${_startHeadingColor}Running for package_a...$_endColor', + ])); + }); + + test('runs command if any files are not ignored', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +skip/a +other +skip/b +''')), + ]; + + final TestPackageLoopingCommand command = + createTestCommand(hasLongOutput: false); + final List output = await runCommand(command); + + expect( + output, + containsAllInOrder([ + '${_startHeadingColor}Running for package_a...$_endColor', + ])); + }); + + test('skips commands if all files should be ignored', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +skip/a +skip/b +''')), + ]; + + final TestPackageLoopingCommand command = + createTestCommand(hasLongOutput: false); + final List output = await runCommand(command); + + expect( + output, + isNot(containsAllInOrder([ + contains('Running for package_a'), + ]))); + expect( + output, + containsAllInOrder([ + '${_startSkipColor}SKIPPING ALL PACKAGES: No changed files affect this command$_endColor', + ])); + }); + }); + group('package iteration', () { test('includes plugins and packages', () async { final RepositoryPackage plugin = @@ -898,6 +954,11 @@ class TestPackageLoopingCommand extends PackageLoopingCommand { @override final String description = 'sample package looping command'; + @override + bool shouldIgnoreFile(String path) { + return path.startsWith('skip/'); + } + @override Future initializeRun() async { if (warnsDuringInit) { diff --git a/script/tool/test/dart_test_command_test.dart b/script/tool/test/dart_test_command_test.dart index 0c12634a766..6b4d6cb4512 100644 --- a/script/tool/test/dart_test_command_test.dart +++ b/script/tool/test/dart_test_command_test.dart @@ -20,11 +20,12 @@ void main() { late Directory packagesDir; late CommandRunner runner; late RecordingProcessRunner processRunner; + late RecordingProcessRunner gitProcessRunner; setUp(() { mockPlatform = MockPlatform(); final GitDir gitDir; - (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = + (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); final DartTestCommand command = DartTestCommand( packagesDir, @@ -713,5 +714,91 @@ test_on: !vm && firefox ]), ); }); + + group('file filtering', () { + test('runs command for changes to Dart source', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package_a/foo.dart +''')), + ]; + + final List output = + await runCapturingPrint(runner, ['test']); + + expect( + output, + containsAllInOrder([ + contains('Running for package_a'), + ])); + }); + + const List files = [ + 'foo.java', + 'foo.kt', + 'foo.m', + 'foo.swift', + 'foo.c', + 'foo.cc', + 'foo.cpp', + 'foo.h', + ]; + for (final String file in files) { + test('skips command for changes to non-Dart source $file', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package_a/$file +''')), + ]; + + final List output = + await runCapturingPrint(runner, ['test']); + + expect( + output, + isNot(containsAllInOrder([ + contains('Running for package_a'), + ]))); + expect( + output, + containsAllInOrder([ + contains('SKIPPING ALL PACKAGES'), + ])); + }); + } + + test('skips commands if all files should be ignored', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +README.md +CODEOWNERS +packages/package_a/CHANGELOG.md +''')), + ]; + + final List output = + await runCapturingPrint(runner, ['test']); + + expect( + output, + isNot(containsAllInOrder([ + contains('Running for package_a'), + ]))); + expect( + output, + containsAllInOrder([ + contains('SKIPPING ALL PACKAGES'), + ])); + }); + }); }); } diff --git a/script/tool/test/drive_examples_command_test.dart b/script/tool/test/drive_examples_command_test.dart index 8ee59212135..b556124f585 100644 --- a/script/tool/test/drive_examples_command_test.dart +++ b/script/tool/test/drive_examples_command_test.dart @@ -28,11 +28,12 @@ void main() { late Directory packagesDir; late CommandRunner runner; late RecordingProcessRunner processRunner; + late RecordingProcessRunner gitProcessRunner; setUp(() { mockPlatform = MockPlatform(); final GitDir gitDir; - (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = + (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); final DriveExamplesCommand command = DriveExamplesCommand( packagesDir, @@ -1714,6 +1715,73 @@ void main() { expect(processRunner.recordedCalls.isEmpty, true); }); }); + + group('file filtering', () { + const List files = [ + 'pubspec.yaml', + 'foo.dart', + 'foo.java', + 'foo.kt', + 'foo.m', + 'foo.swift', + 'foo.cc', + 'foo.cpp', + 'foo.h', + ]; + for (final String file in files) { + test('runs command for changes to $file', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package_a/$file +''')), + ]; + + // The target platform is irrelevant here; because this repo's + // packages are fully federated, there's no need to distinguish + // the ignore list by target (e.g., skipping iOS tests if only Java or + // Kotlin files change), because package-level filering will already + // accomplish the same goal. + final List output = await runCapturingPrint( + runner, ['drive-examples', '--web']); + + expect( + output, + containsAllInOrder([ + contains('Running for package_a'), + ])); + }); + } + + test('skips commands if all files should be ignored', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +README.md +CODEOWNERS +packages/package_a/CHANGELOG.md +''')), + ]; + + final List output = + await runCapturingPrint(runner, ['drive-examples']); + + expect( + output, + isNot(containsAllInOrder([ + contains('Running for package_a'), + ]))); + expect( + output, + containsAllInOrder([ + contains('SKIPPING ALL PACKAGES'), + ])); + }); + }); }); } diff --git a/script/tool/test/firebase_test_lab_command_test.dart b/script/tool/test/firebase_test_lab_command_test.dart index f3d2b21e171..fa139f31a2e 100644 --- a/script/tool/test/firebase_test_lab_command_test.dart +++ b/script/tool/test/firebase_test_lab_command_test.dart @@ -20,11 +20,12 @@ void main() { late Directory packagesDir; late CommandRunner runner; late RecordingProcessRunner processRunner; + late RecordingProcessRunner gitProcessRunner; setUp(() { mockPlatform = MockPlatform(); final GitDir gitDir; - (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = + (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); final FirebaseTestLabCommand command = FirebaseTestLabCommand( packagesDir, @@ -881,5 +882,75 @@ class MainActivityTest { ]), ); }); + + group('file filtering', () { + const List files = [ + 'pubspec.yaml', + 'foo.dart', + 'foo.java', + 'foo.kt', + 'foo.m', + 'foo.swift', + 'foo.cc', + 'foo.cpp', + 'foo.h', + ]; + for (final String file in files) { + test('runs command for changes to $file', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package_a/$file +''')), + ]; + + final List output = await runCapturingPrint(runner, [ + 'firebase-test-lab', + '--results-bucket=a_bucket', + '--device', + 'model=redfin,version=30', + ]); + + expect( + output, + containsAllInOrder([ + contains('Running for package_a'), + ])); + }); + } + + test('skips commands if all files should be ignored', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +README.md +CODEOWNERS +packages/package_a/CHANGELOG.md +''')), + ]; + + final List output = await runCapturingPrint(runner, [ + 'firebase-test-lab', + '--results-bucket=a_bucket', + '--device', + 'model=redfin,version=30', + ]); + + expect( + output, + isNot(containsAllInOrder([ + contains('Running for package_a'), + ]))); + expect( + output, + containsAllInOrder([ + contains('SKIPPING ALL PACKAGES'), + ])); + }); + }); }); } diff --git a/script/tool/test/lint_android_command_test.dart b/script/tool/test/lint_android_command_test.dart index 8ac1bccfa82..543c6ceff30 100644 --- a/script/tool/test/lint_android_command_test.dart +++ b/script/tool/test/lint_android_command_test.dart @@ -19,11 +19,12 @@ void main() { late CommandRunner runner; late MockPlatform mockPlatform; late RecordingProcessRunner processRunner; + late RecordingProcessRunner gitProcessRunner; setUp(() { mockPlatform = MockPlatform(); final GitDir gitDir; - (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = + (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); final LintAndroidCommand command = LintAndroidCommand( packagesDir, @@ -241,5 +242,61 @@ void main() { ], )); }); + + group('file filtering', () { + const List files = [ + 'foo.java', + 'foo.kt', + ]; + for (final String file in files) { + test('runs command for changes to $file', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package_a/$file +''')), + ]; + + final List output = + await runCapturingPrint(runner, ['lint-android']); + + expect( + output, + containsAllInOrder([ + contains('Running for package_a'), + ])); + }); + } + + test('skips commands if all files should be ignored', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +README.md +CODEOWNERS +packages/package_a/CHANGELOG.md +packages/package_a/lib/foo.dart +''')), + ]; + + final List output = + await runCapturingPrint(runner, ['lint-android']); + + expect( + output, + isNot(containsAllInOrder([ + contains('Running for package_a'), + ]))); + expect( + output, + containsAllInOrder([ + contains('SKIPPING ALL PACKAGES'), + ])); + }); + }); }); } diff --git a/script/tool/test/native_test_command_test.dart b/script/tool/test/native_test_command_test.dart index b5b298d2fea..10ff16de2e6 100644 --- a/script/tool/test/native_test_command_test.dart +++ b/script/tool/test/native_test_command_test.dart @@ -84,6 +84,7 @@ void main() { late Directory packagesDir; late CommandRunner runner; late RecordingProcessRunner processRunner; + late RecordingProcessRunner gitProcessRunner; setUp(() { // iOS and macOS tests expect macOS, Linux tests expect Linux; nothing @@ -91,7 +92,7 @@ void main() { // allow them to share a setup group. mockPlatform = MockPlatform(isMacOS: true, isLinux: true); final GitDir gitDir; - (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = + (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); final NativeTestCommand command = NativeTestCommand( packagesDir, @@ -379,6 +380,73 @@ void main() { destination: 'id=$_simulatorDeviceId'), ])); }); + + group('file filtering', () { + const List files = [ + 'pubspec.yaml', + 'foo.dart', + 'foo.java', + 'foo.kt', + 'foo.m', + 'foo.swift', + 'foo.cc', + 'foo.cpp', + 'foo.h', + ]; + for (final String file in files) { + test('runs command for changes to $file', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package_a/$file +''')), + ]; + + // The target platform is irrelevant here; because this repo's + // packages are fully federated, there's no need to distinguish + // the ignore list by target (e.g., skipping iOS tests if only Java or + // Kotlin files change), because package-level filering will already + // accomplish the same goal. + final List output = await runCapturingPrint( + runner, ['native-test', '--android']); + + expect( + output, + containsAllInOrder([ + contains('Running for package_a'), + ])); + }); + } + + test('skips commands if all files should be ignored', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +README.md +CODEOWNERS +packages/package_a/CHANGELOG.md +''')), + ]; + + final List output = await runCapturingPrint( + runner, ['native-test', 'android']); + + expect( + output, + isNot(containsAllInOrder([ + contains('Running for package_a'), + ]))); + expect( + output, + containsAllInOrder([ + contains('SKIPPING ALL PACKAGES'), + ])); + }); + }); }); group('macOS', () { diff --git a/script/tool/test/xcode_analyze_command_test.dart b/script/tool/test/xcode_analyze_command_test.dart index 7ffac396a59..6342fab97c5 100644 --- a/script/tool/test/xcode_analyze_command_test.dart +++ b/script/tool/test/xcode_analyze_command_test.dart @@ -21,11 +21,12 @@ void main() { late Directory packagesDir; late CommandRunner runner; late RecordingProcessRunner processRunner; + late RecordingProcessRunner gitProcessRunner; setUp(() { mockPlatform = MockPlatform(isMacOS: true); final GitDir gitDir; - (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = + (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) = configureBaseCommandMocks(platform: mockPlatform); final XcodeAnalyzeCommand command = XcodeAnalyzeCommand( packagesDir, @@ -561,5 +562,64 @@ void main() { expect(processRunner.recordedCalls, orderedEquals([])); }); }); + + group('file filtering', () { + const List files = [ + 'foo.m', + 'foo.swift', + 'foo.cc', + 'foo.cpp', + 'foo.h', + ]; + for (final String file in files) { + test('runs command for changes to $file', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +packages/package_a/$file +''')), + ]; + + final List output = await runCapturingPrint( + runner, ['xcode-analyze', '--ios']); + + expect( + output, + containsAllInOrder([ + contains('Running for package_a'), + ])); + }); + } + + test('skips commands if all files should be ignored', () async { + createFakePackage('package_a', packagesDir); + + gitProcessRunner.mockProcessesForExecutable['git-diff'] = + [ + FakeProcessInfo(MockProcess(stdout: ''' +README.md +CODEOWNERS +packages/package_a/CHANGELOG.md +packages/package_a/lib/foo.dart +''')), + ]; + + final List output = + await runCapturingPrint(runner, ['xcode-analyze', '--ios']); + + expect( + output, + isNot(containsAllInOrder([ + contains('Running for package_a'), + ]))); + expect( + output, + containsAllInOrder([ + contains('SKIPPING ALL PACKAGES'), + ])); + }); + }); }); }