From 2d7013315b4cd8e6afb097c65555d5f68da62381 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 27 Feb 2025 14:37:15 -0500 Subject: [PATCH 1/4] Add skip support --- .../src/common/package_looping_command.dart | 22 +++++ .../common/package_looping_command_test.dart | 99 +++++++++++++++---- 2 files changed, 103 insertions(+), 18 deletions(-) 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/test/common/package_looping_command_test.dart b/script/tool/test/common/package_looping_command_test.dart index 6474dff8025..930a65b7402 100644 --- a/script/tool/test/common/package_looping_command_test.dart +++ b/script/tool/test/common/package_looping_command_test.dart @@ -10,6 +10,7 @@ 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:git/git.dart'; import 'package:mockito/mockito.dart'; import 'package:test/test.dart'; @@ -79,10 +80,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 +99,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 +109,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 +199,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 +956,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) { From 09ab2da11857d0153a8c98b6672e9bb96a5602db Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 27 Feb 2025 22:53:25 -0500 Subject: [PATCH 2/4] Add initial filtering to many of the expensive commands --- script/tool/lib/src/analyze_command.dart | 19 ++++ .../tool/lib/src/build_examples_command.dart | 8 ++ script/tool/lib/src/common/core.dart | 21 +++++ script/tool/lib/src/dart_test_command.dart | 18 ++++ .../tool/lib/src/drive_examples_command.dart | 8 ++ .../lib/src/firebase_test_lab_command.dart | 8 ++ script/tool/lib/src/native_test_command.dart | 10 +++ .../tool/lib/src/xcode_analyze_command.dart | 8 ++ script/tool/test/analyze_command_test.dart | 89 ++++++++++++++++++- .../test/build_examples_command_test.dart | 70 ++++++++++++++- .../common/package_looping_command_test.dart | 2 - script/tool/test/dart_test_command_test.dart | 89 ++++++++++++++++++- .../test/drive_examples_command_test.dart | 70 ++++++++++++++- .../test/firebase_test_lab_command_test.dart | 73 ++++++++++++++- .../tool/test/native_test_command_test.dart | 70 ++++++++++++++- .../tool/test/xcode_analyze_command_test.dart | 65 +++++++++++++- 16 files changed, 619 insertions(+), 9 deletions(-) diff --git a/script/tool/lib/src/analyze_command.dart b/script/tool/lib/src/analyze_command.dart index 4651dbb42c7..55989d98d39 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/core.dart'; import 'common/output_utils.dart'; import 'common/package_looping_command.dart'; import 'common/repository_package.dart'; @@ -92,6 +93,24 @@ class AnalyzeCommand extends PackageLoopingCommand { return false; } + @override + bool shouldIgnoreFile(String path) { + return repoLevelNonCodeImpactingFiles.contains(path) || + // Native code, which is not part of Dart analysis. + path.endsWith('.c') || + path.endsWith('.cc') || + path.endsWith('.cpp') || + path.endsWith('.h') || + path.endsWith('.m') || + path.endsWith('.swift') || + path.endsWith('.java') || + path.endsWith('.kt') || + // Package metadata that doesn't impact analysis. + path.endsWith('/AUTHORS') || + path.endsWith('/CHANGELOG.md') || + path.endsWith('/README.md'); + } + @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..45588bf72d1 100644 --- a/script/tool/lib/src/build_examples_command.dart +++ b/script/tool/lib/src/build_examples_command.dart @@ -134,6 +134,14 @@ class BuildExamplesCommand extends PackageLoopingCommand { return getNullableBoolArg(_swiftPackageManagerFlag); } + @override + bool shouldIgnoreFile(String path) { + return repoLevelNonCodeImpactingFiles.contains(path) || + path.endsWith('/AUTHORS') || + path.endsWith('/CHANGELOG.md') || + path.endsWith('/README.md'); + } + @override Future initializeRun() async { final List platformFlags = _platforms.keys.toList(); diff --git a/script/tool/lib/src/common/core.dart b/script/tool/lib/src/common/core.dart index 57b48252241..c81fae3c2a0 100644 --- a/script/tool/lib/src/common/core.dart +++ b/script/tool/lib/src/common/core.dart @@ -140,3 +140,24 @@ Directory? ciLogsDirectory(Platform platform, FileSystem fileSystem) { } return logsDirectory; } + +/// Full repo-relative paths to 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. +const List repoLevelNonCodeImpactingFiles = [ + '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 added here 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', +]; diff --git a/script/tool/lib/src/dart_test_command.dart b/script/tool/lib/src/dart_test_command.dart index 5e99211169e..f4666055709 100644 --- a/script/tool/lib/src/dart_test_command.dart +++ b/script/tool/lib/src/dart_test_command.dart @@ -65,6 +65,24 @@ class DartTestCommand extends PackageLoopingCommand { PackageLoopingType get packageLoopingType => PackageLoopingType.includeAllSubpackages; + @override + bool shouldIgnoreFile(String path) { + return repoLevelNonCodeImpactingFiles.contains(path) || + // Native code, which is not part of Dart unit testing. + path.endsWith('.c') || + path.endsWith('.cc') || + path.endsWith('.cpp') || + path.endsWith('.h') || + path.endsWith('.m') || + path.endsWith('.swift') || + path.endsWith('.java') || + path.endsWith('.kt') || + // Package metadata that doesn't impact Dart unit tests. + path.endsWith('/AUTHORS') || + path.endsWith('/CHANGELOG.md') || + path.endsWith('/README.md'); + } + @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..cf7c2a9d488 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -68,6 +68,14 @@ class DriveExamplesCommand extends PackageLoopingCommand { Map> _targetDeviceFlags = const >{}; + @override + bool shouldIgnoreFile(String path) { + return repoLevelNonCodeImpactingFiles.contains(path) || + path.endsWith('/AUTHORS') || + path.endsWith('/CHANGELOG.md') || + path.endsWith('/README.md'); + } + @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 3fb8e14cf3d..9926555cb4a 100644 --- a/script/tool/lib/src/firebase_test_lab_command.dart +++ b/script/tool/lib/src/firebase_test_lab_command.dart @@ -121,6 +121,14 @@ class FirebaseTestLabCommand extends PackageLoopingCommand { _firebaseProjectConfigured = true; } + @override + bool shouldIgnoreFile(String path) { + return repoLevelNonCodeImpactingFiles.contains(path) || + path.endsWith('/AUTHORS') || + path.endsWith('/CHANGELOG.md') || + path.endsWith('/README.md'); + } + @override Future runForPackage(RepositoryPackage package) async { final List results = []; diff --git a/script/tool/lib/src/native_test_command.dart b/script/tool/lib/src/native_test_command.dart index d5995a35cd2..2ac2ac6964b 100644 --- a/script/tool/lib/src/native_test_command.dart +++ b/script/tool/lib/src/native_test_command.dart @@ -113,6 +113,16 @@ this command. Set _xcodeWarningsExceptions = {}; + @override + bool shouldIgnoreFile(String path) { + return repoLevelNonCodeImpactingFiles.contains(path) || + path.endsWith('/AUTHORS') || + path.endsWith('/CHANGELOG.md') || + path.endsWith('/README.md'); + // 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 6d6bd489649..45494a68778 100644 --- a/script/tool/lib/src/xcode_analyze_command.dart +++ b/script/tool/lib/src/xcode_analyze_command.dart @@ -46,6 +46,14 @@ class XcodeAnalyzeCommand extends PackageLoopingCommand { final String description = 'Runs Xcode analysis on the iOS and/or macOS example apps.'; + @override + bool shouldIgnoreFile(String path) { + return repoLevelNonCodeImpactingFiles.contains(path) || + path.endsWith('/AUTHORS') || + path.endsWith('/CHANGELOG.md') || + path.endsWith('/README.md'); + } + @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 930a65b7402..d2247b9d488 100644 --- a/script/tool/test/common/package_looping_command_test.dart +++ b/script/tool/test/common/package_looping_command_test.dart @@ -11,12 +11,10 @@ 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:git/git.dart'; -import 'package:mockito/mockito.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'; 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 13eeaa494ed..787ad4fb952 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, @@ -901,5 +902,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/native_test_command_test.dart b/script/tool/test/native_test_command_test.dart index 0a9924727ef..97951c9c74e 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, @@ -358,6 +359,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 a69ee23120d..0c6cd03d5fd 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, @@ -489,5 +490,67 @@ void main() { expect(processRunner.recordedCalls, orderedEquals([])); }); }); + + 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, ['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 +''')), + ]; + + 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'), + ])); + }); + }); }); } From 72248b0d53daf6d8acadd05fea76620e88b2eb3f Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 27 Mar 2025 10:30:00 -0400 Subject: [PATCH 3/4] Clarify comment --- script/tool/lib/src/common/core.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/script/tool/lib/src/common/core.dart b/script/tool/lib/src/common/core.dart index c81fae3c2a0..cccd88aa24e 100644 --- a/script/tool/lib/src/common/core.dart +++ b/script/tool/lib/src/common/core.dart @@ -153,8 +153,8 @@ const List repoLevelNonCodeImpactingFiles = [ '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 added here that could affect - // packages. + // 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', From f62c27839f971224722f8b349ffb63c39c4a6c71 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 17 Apr 2025 16:03:29 -0400 Subject: [PATCH 4/4] Collect common helpers --- script/tool/lib/src/analyze_command.dart | 19 ++---- .../tool/lib/src/build_examples_command.dart | 6 +- script/tool/lib/src/common/core.dart | 21 ------- script/tool/lib/src/common/file_filters.dart | 53 +++++++++++++++++ script/tool/lib/src/dart_test_command.dart | 18 ++---- .../tool/lib/src/drive_examples_command.dart | 6 +- .../lib/src/firebase_test_lab_command.dart | 7 +-- script/tool/lib/src/lint_android_command.dart | 11 ++++ script/tool/lib/src/native_test_command.dart | 7 +-- .../tool/lib/src/xcode_analyze_command.dart | 11 ++-- .../tool/test/lint_android_command_test.dart | 59 ++++++++++++++++++- .../tool/test/xcode_analyze_command_test.dart | 5 +- 12 files changed, 148 insertions(+), 75 deletions(-) create mode 100644 script/tool/lib/src/common/file_filters.dart diff --git a/script/tool/lib/src/analyze_command.dart b/script/tool/lib/src/analyze_command.dart index 55989d98d39..51a2403ab2e 100644 --- a/script/tool/lib/src/analyze_command.dart +++ b/script/tool/lib/src/analyze_command.dart @@ -6,7 +6,7 @@ import 'dart:io' as 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/repository_package.dart'; @@ -95,20 +95,9 @@ class AnalyzeCommand extends PackageLoopingCommand { @override bool shouldIgnoreFile(String path) { - return repoLevelNonCodeImpactingFiles.contains(path) || - // Native code, which is not part of Dart analysis. - path.endsWith('.c') || - path.endsWith('.cc') || - path.endsWith('.cpp') || - path.endsWith('.h') || - path.endsWith('.m') || - path.endsWith('.swift') || - path.endsWith('.java') || - path.endsWith('.kt') || - // Package metadata that doesn't impact analysis. - path.endsWith('/AUTHORS') || - path.endsWith('/CHANGELOG.md') || - path.endsWith('/README.md'); + return isRepoLevelNonCodeImpactingFile(path) || + isNativeCodeFile(path) || + isPackageSupportFile(path); } @override diff --git a/script/tool/lib/src/build_examples_command.dart b/script/tool/lib/src/build_examples_command.dart index 45588bf72d1..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'; @@ -136,10 +137,7 @@ class BuildExamplesCommand extends PackageLoopingCommand { @override bool shouldIgnoreFile(String path) { - return repoLevelNonCodeImpactingFiles.contains(path) || - path.endsWith('/AUTHORS') || - path.endsWith('/CHANGELOG.md') || - path.endsWith('/README.md'); + return isRepoLevelNonCodeImpactingFile(path) || isPackageSupportFile(path); } @override diff --git a/script/tool/lib/src/common/core.dart b/script/tool/lib/src/common/core.dart index cccd88aa24e..57b48252241 100644 --- a/script/tool/lib/src/common/core.dart +++ b/script/tool/lib/src/common/core.dart @@ -140,24 +140,3 @@ Directory? ciLogsDirectory(Platform platform, FileSystem fileSystem) { } return logsDirectory; } - -/// Full repo-relative paths to 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. -const List repoLevelNonCodeImpactingFiles = [ - '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', -]; 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/dart_test_command.dart b/script/tool/lib/src/dart_test_command.dart index f4666055709..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'; @@ -67,20 +68,9 @@ class DartTestCommand extends PackageLoopingCommand { @override bool shouldIgnoreFile(String path) { - return repoLevelNonCodeImpactingFiles.contains(path) || - // Native code, which is not part of Dart unit testing. - path.endsWith('.c') || - path.endsWith('.cc') || - path.endsWith('.cpp') || - path.endsWith('.h') || - path.endsWith('.m') || - path.endsWith('.swift') || - path.endsWith('.java') || - path.endsWith('.kt') || - // Package metadata that doesn't impact Dart unit tests. - path.endsWith('/AUTHORS') || - path.endsWith('/CHANGELOG.md') || - path.endsWith('/README.md'); + return isRepoLevelNonCodeImpactingFile(path) || + isNativeCodeFile(path) || + isPackageSupportFile(path); } @override diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index cf7c2a9d488..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'; @@ -70,10 +71,7 @@ class DriveExamplesCommand extends PackageLoopingCommand { @override bool shouldIgnoreFile(String path) { - return repoLevelNonCodeImpactingFiles.contains(path) || - path.endsWith('/AUTHORS') || - path.endsWith('/CHANGELOG.md') || - path.endsWith('/README.md'); + return isRepoLevelNonCodeImpactingFile(path) || isPackageSupportFile(path); } @override diff --git a/script/tool/lib/src/firebase_test_lab_command.dart b/script/tool/lib/src/firebase_test_lab_command.dart index 9926555cb4a..4ce1f3b3ce3 100644 --- a/script/tool/lib/src/firebase_test_lab_command.dart +++ b/script/tool/lib/src/firebase_test_lab_command.dart @@ -8,6 +8,8 @@ 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'; import 'common/package_looping_command.dart'; @@ -123,10 +125,7 @@ class FirebaseTestLabCommand extends PackageLoopingCommand { @override bool shouldIgnoreFile(String path) { - return repoLevelNonCodeImpactingFiles.contains(path) || - path.endsWith('/AUTHORS') || - path.endsWith('/CHANGELOG.md') || - path.endsWith('/README.md'); + return isRepoLevelNonCodeImpactingFile(path) || isPackageSupportFile(path); } @override diff --git a/script/tool/lib/src/lint_android_command.dart b/script/tool/lib/src/lint_android_command.dart index b893d9b0315..146d60b7f9d 100644 --- a/script/tool/lib/src/lint_android_command.dart +++ b/script/tool/lib/src/lint_android_command.dart @@ -3,6 +3,8 @@ // 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'; import 'common/package_looping_command.dart'; @@ -28,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 2ac2ac6964b..a5370485b0f 100644 --- a/script/tool/lib/src/native_test_command.dart +++ b/script/tool/lib/src/native_test_command.dart @@ -9,6 +9,8 @@ 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'; import 'common/package_looping_command.dart'; @@ -115,10 +117,7 @@ this command. @override bool shouldIgnoreFile(String path) { - return repoLevelNonCodeImpactingFiles.contains(path) || - path.endsWith('/AUTHORS') || - path.endsWith('/CHANGELOG.md') || - path.endsWith('/README.md'); + 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. } diff --git a/script/tool/lib/src/xcode_analyze_command.dart b/script/tool/lib/src/xcode_analyze_command.dart index 45494a68778..c423c848aec 100644 --- a/script/tool/lib/src/xcode_analyze_command.dart +++ b/script/tool/lib/src/xcode_analyze_command.dart @@ -3,6 +3,8 @@ // 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'; import 'common/plugin_utils.dart'; @@ -48,10 +50,11 @@ class XcodeAnalyzeCommand extends PackageLoopingCommand { @override bool shouldIgnoreFile(String path) { - return repoLevelNonCodeImpactingFiles.contains(path) || - path.endsWith('/AUTHORS') || - path.endsWith('/CHANGELOG.md') || - path.endsWith('/README.md'); + 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 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/xcode_analyze_command_test.dart b/script/tool/test/xcode_analyze_command_test.dart index 0c6cd03d5fd..59e92a57184 100644 --- a/script/tool/test/xcode_analyze_command_test.dart +++ b/script/tool/test/xcode_analyze_command_test.dart @@ -493,10 +493,6 @@ void main() { group('file filtering', () { const List files = [ - 'pubspec.yaml', - 'foo.dart', - 'foo.java', - 'foo.kt', 'foo.m', 'foo.swift', 'foo.cc', @@ -534,6 +530,7 @@ packages/package_a/$file README.md CODEOWNERS packages/package_a/CHANGELOG.md +packages/package_a/lib/foo.dart ''')), ];