From f44f625c0641e888e19e1a1419d943f949cdcc4c Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 23 Jul 2018 15:41:31 -0700 Subject: [PATCH] Fix whitespace detector to handle deleted files. (#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. --- .cirrus.yml | 8 ++++---- dev/bots/test.dart | 30 ++++++++++++++++++------------ 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 2573a2d6aa9..db9ede7fa3b 100644 --- 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 index 2ecfe788245..edd765befea 100644 --- a/dev/bots/test.dart +++ b/dev/bots/test.dart @@ -103,9 +103,8 @@ Future _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 fileTypes = [ - '*.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', ['diff', '-U0', '--no-color', '--name-only', commitRange, '--'] + fileTypes, @@ -115,8 +114,11 @@ Future _checkForTrailingSpaces() async { print('No Results for whitespace check.'); return; } - final List 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 changedFiles = changedFilesResult.stdout.split('\n').where((String filename) { + return new File(filename).existsSync(); + }).toList(); if (changedFiles.isNotEmpty) { await _runCommand('grep', [ @@ -125,8 +127,8 @@ Future _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 _runSmokeTests() async { _runCommand(flutter, ['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 _runCommand(String executable, List arguments, { String workingDirectory, Map 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 _runCommand(String executable, List 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 ints) => ints).toList())); stderr.writeln(utf8.decode((await savedStderr).expand((List 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 _runFlutterTest(String workingDirectory, { } return _runCommand(flutter, args, workingDirectory: workingDirectory, - expectFailure: expectFailure, + expectNonZeroExit: expectFailure, printOutput: printOutput, skip: skip, timeout: timeout,