[tool] Fix false positives in update-exceprts (#6950)

When determining whether or not to fail with `--fail-on-change`, only
look at .md files. In some cases, running the necessary commands (e.g.,
`flutter pub get`) may change unrelated files, causing fales positive
failures. Only changed documentation files should be flagged.

Also log the specific files that were detected as changed, to aid in
debugging any future false positives.

Fixes https://github.com/flutter/flutter/issues/111592
Fixes https://github.com/flutter/flutter/issues/111590
This commit is contained in:
stuartmorgan
2023-01-13 03:12:28 -08:00
committed by GitHub
parent bbab349927
commit 459e80606b
2 changed files with 36 additions and 7 deletions

View File

@ -206,20 +206,28 @@ class UpdateExcerptsCommand extends PackageLoopingCommand {
.renameSync(package.pubspecFile.path); .renameSync(package.pubspecFile.path);
} }
/// Checks the git state, returning an error string unless nothing has /// Checks the git state, returning an error string if any .md files have
/// changed. /// changed.
Future<String?> _validateRepositoryState() async { Future<String?> _validateRepositoryState() async {
final io.ProcessResult modifiedFiles = await processRunner.run( final io.ProcessResult checkFiles = await processRunner.run(
'git', 'git',
<String>['ls-files', '--modified'], <String>['ls-files', '--modified'],
workingDir: packagesDir, workingDir: packagesDir,
logOnError: true, logOnError: true,
); );
if (modifiedFiles.exitCode != 0) { if (checkFiles.exitCode != 0) {
return 'Unable to determine local file state'; return 'Unable to determine local file state';
} }
final String stdout = modifiedFiles.stdout as String; final String stdout = checkFiles.stdout as String;
return stdout.trim().isEmpty ? null : 'Snippets are out of sync'; final List<String> changedFiles = stdout.trim().split('\n');
final Iterable<String> changedMDFiles =
changedFiles.where((String filePath) => filePath.endsWith('.md'));
if (changedMDFiles.isNotEmpty) {
return 'Snippets are out of sync in the following files: '
'${changedMDFiles.join(', ')}';
}
return null;
} }
} }

View File

@ -232,11 +232,11 @@ void main() {
])); ]));
}); });
test('fails if files are changed with --fail-on-change', () async { test('fails if READMEs are changed with --fail-on-change', () async {
createFakePlugin('a_plugin', packagesDir, createFakePlugin('a_plugin', packagesDir,
extraFiles: <String>[kReadmeExcerptConfigPath]); extraFiles: <String>[kReadmeExcerptConfigPath]);
const String changedFilePath = 'packages/a_plugin/linux/foo_plugin.cc'; const String changedFilePath = 'packages/a_plugin/README.md';
processRunner.mockProcessesForExecutable['git'] = <io.Process>[ processRunner.mockProcessesForExecutable['git'] = <io.Process>[
MockProcess(stdout: changedFilePath), MockProcess(stdout: changedFilePath),
]; ];
@ -253,6 +253,27 @@ void main() {
output, output,
containsAllInOrder(<Matcher>[ containsAllInOrder(<Matcher>[
contains('README.md is out of sync with its source excerpts'), contains('README.md is out of sync with its source excerpts'),
contains('Snippets are out of sync in the following files: '
'packages/a_plugin/README.md'),
]));
});
test('passes if unrelated files are changed with --fail-on-change', () async {
createFakePlugin('a_plugin', packagesDir,
extraFiles: <String>[kReadmeExcerptConfigPath]);
const String changedFilePath = 'packages/a_plugin/linux/CMakeLists.txt';
processRunner.mockProcessesForExecutable['git'] = <io.Process>[
MockProcess(stdout: changedFilePath),
];
final List<String> output = await runCapturingPrint(
runner, <String>['update-excerpts', '--fail-on-change']);
expect(
output,
containsAllInOrder(<Matcher>[
contains('Ran for 1 package(s)'),
])); ]));
}); });