Fix whitespace detector to handle deleted files. (flutter#19690)

The trailing whitespace detector wasn't handling file deletes very well (at all, really). This filters the set of files grepped to only include files that exist. Also, clarified the failure message to make it more obvious what the failure is when the grep finds results.
shenguangyao · Jul 23, 2018 · f44f625 · f44f625
1 parent d61b4fa
commit f44f625
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 16 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -64,7 +64,7 @@ task:
     - bin\flutter.bat update-packages
     - git fetch origin master
   test_all_script: |
-    export TEST_COMMIT_RANGE="$(git merge-base --fork-point FETCH_HEAD HEAD)..HEAD"
+    export TEST_COMMIT_RANGE="$(git merge-base --fork-point FETCH_HEAD HEAD || git merge-base FETCH_HEAD HEAD)..HEAD"
     bin\cache\dart-sdk\bin\dart.exe -c dev\bots\test.dart
 
 task:
@@ -83,7 +83,7 @@ task:
     - bin\flutter.bat update-packages
     - git fetch origin master
   test_all_script: |
-    export TEST_COMMIT_RANGE="$(git merge-base --fork-point FETCH_HEAD HEAD)..HEAD"
+    export TEST_COMMIT_RANGE="$(git merge-base --fork-point FETCH_HEAD HEAD || git merge-base FETCH_HEAD HEAD)..HEAD"
     bin\cache\dart-sdk\bin\dart.exe -c dev\bots\test.dart
 
 task:
@@ -98,7 +98,7 @@ task:
     - bin/flutter update-packages
   test_all_script: |
     ulimit -S -n 2048 # https://github.com/flutter/flutter/issues/2976
-    export TEST_COMMIT_RANGE="$(git merge-base --fork-point FETCH_HEAD HEAD)..HEAD"
+    export TEST_COMMIT_RANGE="$(git merge-base --fork-point FETCH_HEAD HEAD || git merge-base FETCH_HEAD HEAD)..HEAD"
     bin/cache/dart-sdk/bin/dart -c dev/bots/test.dart
 
 task:
@@ -113,5 +113,5 @@ task:
     - bin/flutter update-packages
   test_all_script: |
     ulimit -S -n 2048 # https://github.com/flutter/flutter/issues/2976
-    export TEST_COMMIT_RANGE="$(git merge-base --fork-point FETCH_HEAD HEAD)..HEAD"
+    export TEST_COMMIT_RANGE="$(git merge-base --fork-point FETCH_HEAD HEAD || git merge-base FETCH_HEAD HEAD)..HEAD"
     bin/cache/dart-sdk/bin/dart -c dev/bots/test.dart
diff --git a/dev/bots/test.dart b/dev/bots/test.dart
@@ -103,9 +103,8 @@ Future<Null> _checkForTrailingSpaces() async {
     final String commitRange = Platform.environment.containsKey('TEST_COMMIT_RANGE')
         ? Platform.environment['TEST_COMMIT_RANGE']
         : 'master..HEAD';
-    print('Checking for trailing whitespace in source files.');
     final List<String> fileTypes = <String>[
-      '*.dart', '*.cxx', '*.cpp', '*.cc', '*.c', '*.C', '*.h', '*.java', '*.mm', '*.m',
+      '*.dart', '*.cxx', '*.cpp', '*.cc', '*.c', '*.C', '*.h', '*.java', '*.mm', '*.m', '.yml',
     ];
     final EvalResult changedFilesResult = await _evalCommand(
       'git', <String>['diff', '-U0', '--no-color', '--name-only', commitRange, '--'] + fileTypes,
@@ -115,8 +114,11 @@ Future<Null> _checkForTrailingSpaces() async {
       print('No Results for whitespace check.');
       return;
     }
-    final List<String> changedFiles = changedFilesResult.stdout.trim().split('\n')
-        .where((String item) => item.trim().isNotEmpty).toList();
+    // Only include files that actually exist, so that we don't try and grep for
+    // nonexistent files (can occur when files are deleted or moved).
+    final List<String> changedFiles = changedFilesResult.stdout.split('\n').where((String filename) {
+      return new File(filename).existsSync();
+    }).toList();
     if (changedFiles.isNotEmpty) {
       await _runCommand('grep',
         <String>[
@@ -125,8 +127,8 @@ Future<Null> _checkForTrailingSpaces() async {
           r'[[:space:]]+$',
         ] + changedFiles,
         workingDirectory: flutterRoot,
-        printOutput: false,
-        expectFailure: true, // Just means a non-zero exit code is expected.
+        failureMessage: '${red}Whitespace detected at the end of source code lines.$reset\nPlease remove:',
+        expectNonZeroExit: true, // Just means a non-zero exit code is expected.
         expectedExitCode: 1, // Indicates that zero lines were found.
       );
     }
@@ -244,7 +246,7 @@ Future<Null> _runSmokeTests() async {
       _runCommand(flutter,
         <String>['drive', '--use-existing-app', '-t', path.join('test_driver', 'failure.dart')],
         workingDirectory: path.join(flutterRoot, 'packages', 'flutter_driver'),
-        expectFailure: true,
+        expectNonZeroExit: true,
         printOutput: false,
         timeout: _kShortTimeout,
       ),
@@ -387,8 +389,9 @@ String elapsedTime(DateTime start) {
 Future<Null> _runCommand(String executable, List<String> arguments, {
   String workingDirectory,
   Map<String, String> environment,
-  bool expectFailure = false,
+  bool expectNonZeroExit = false,
   int expectedExitCode,
+  String failureMessage,
   bool printOutput = true,
   bool skip = false,
   Duration timeout = _kLongTimeout,
@@ -420,17 +423,20 @@ Future<Null> _runCommand(String executable, List<String> arguments, {
 
   final int exitCode = await process.exitCode.timeout(timeout, onTimeout: () {
     stderr.writeln('Process timed out after $timeout');
-    return expectFailure ? 0 : 1;
+    return expectNonZeroExit ? 0 : 1;
   });
   print('$clock ELAPSED TIME: $bold${elapsedTime(start)}$reset for $commandDescription in $relativeWorkingDir: ');
-  if ((exitCode == 0) == expectFailure || (expectedExitCode != null && exitCode != expectedExitCode)) {
+  if ((exitCode == 0) == expectNonZeroExit || (expectedExitCode != null && exitCode != expectedExitCode)) {
+    if (failureMessage != null) {
+      print(failureMessage);
+    }
     if (!printOutput) {
       stdout.writeln(utf8.decode((await savedStdout).expand((List<int> ints) => ints).toList()));
       stderr.writeln(utf8.decode((await savedStderr).expand((List<int> ints) => ints).toList()));
     }
     print(
       '$red━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━$reset\n'
-      '${bold}ERROR:$red Last command exited with $exitCode (expected: ${expectFailure ? 'non-zero' : 'zero'}).$reset\n'
+      '${bold}ERROR:$red Last command exited with $exitCode (expected: ${expectNonZeroExit ? (expectedExitCode ?? 'non-zero') : 'zero'}).$reset\n'
       '${bold}Command:$cyan $commandDescription$reset\n'
       '${bold}Relative working directory:$red $relativeWorkingDir$reset\n'
       '$red━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━$reset'
@@ -466,7 +472,7 @@ Future<Null> _runFlutterTest(String workingDirectory, {
   }
   return _runCommand(flutter, args,
     workingDirectory: workingDirectory,
-    expectFailure: expectFailure,
+    expectNonZeroExit: expectFailure,
     printOutput: printOutput,
     skip: skip,
     timeout: timeout,