Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Commit

Permalink
[tool] Fix false positives in update-exceprts (#6950)
Browse files Browse the repository at this point in the history
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 flutter/flutter#111592
Fixes flutter/flutter#111590
  • Loading branch information
stuartmorgan authored Jan 13, 2023
1 parent be2e3de commit 92a5367
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 7 deletions.
18 changes: 13 additions & 5 deletions script/tool/lib/src/update_excerpts_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -206,20 +206,28 @@ class UpdateExcerptsCommand extends PackageLoopingCommand {
.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.
Future<String?> _validateRepositoryState() async {
final io.ProcessResult modifiedFiles = await processRunner.run(
final io.ProcessResult checkFiles = await processRunner.run(
'git',
<String>['ls-files', '--modified'],
workingDir: packagesDir,
logOnError: true,
);
if (modifiedFiles.exitCode != 0) {
if (checkFiles.exitCode != 0) {
return 'Unable to determine local file state';
}

final String stdout = modifiedFiles.stdout as String;
return stdout.trim().isEmpty ? null : 'Snippets are out of sync';
final String stdout = checkFiles.stdout as String;
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;
}
}
25 changes: 23 additions & 2 deletions script/tool/test/update_excerpts_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
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>[
MockProcess(stdout: changedFilePath),
];
Expand All @@ -253,6 +253,27 @@ void main() {
output,
containsAllInOrder(<Matcher>[
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)'),
]));
});

Expand Down

0 comments on commit 92a5367

Please sign in to comment.