From 92a5367d58df5f9abbb800176e8b41a0d17bc353 Mon Sep 17 00:00:00 2001
From: stuartmorgan <stuartmorgan@google.com>
Date: Fri, 13 Jan 2023 03:12:28 -0800
Subject: [PATCH] [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
---
 .../tool/lib/src/update_excerpts_command.dart | 18 +++++++++----
 .../test/update_excerpts_command_test.dart    | 25 +++++++++++++++++--
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/script/tool/lib/src/update_excerpts_command.dart b/script/tool/lib/src/update_excerpts_command.dart
index 5a59104d4e7f..e65bed846cbc 100644
--- a/script/tool/lib/src/update_excerpts_command.dart
+++ b/script/tool/lib/src/update_excerpts_command.dart
@@ -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;
   }
 }
diff --git a/script/tool/test/update_excerpts_command_test.dart b/script/tool/test/update_excerpts_command_test.dart
index 79f53d8779bb..5a2f0f340414 100644
--- a/script/tool/test/update_excerpts_command_test.dart
+++ b/script/tool/test/update_excerpts_command_test.dart
@@ -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),
     ];
@@ -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)'),
         ]));
   });