Add enhanced testing support for dart fix

Change-Id: I8cf49b85142c242597542eae9306eb09be5e4149
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175920
Reviewed-by: Phil Quitslund <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>

Author: bwilkerson

pkg/dartdev/lib/src/commands/fix.dart

@@ -88,10 +88,12 @@ To use the tool, run either ['dart fix --dry-run'] for a preview of the proposed
     if (!dir.existsSync()) {
       usageException("Directory doesn't exist: ${dir.path}");
     }
+    dir = io.Directory(path.canonicalize(path.normalize(dir.absolute.path)));
+    var dirPath = dir.path;
 
     var modeText = dryRun ? ' (dry run)' : '';
 
-    final projectName = path.basename(path.canonicalize(dir.path));
+    final projectName = path.basename(dirPath);
     var progress = log.progress(
         'Computing fixes in ${log.ansi.emphasized(projectName)}$modeText');
 
@@ -111,24 +113,24 @@ To use the tool, run either ['dart fix --dry-run'] for a preview of the proposed
       }
     });
 
-    fixes = await server.requestBulkFixes(dir.absolute.path);
+    fixes = await server.requestBulkFixes(dirPath);
     final List<SourceFileEdit> edits = fixes.edits;
 
     await server.shutdown();
 
     progress.finish(showTiming: true);
 
     if (testMode) {
-      if (_compareFixes(edits)) {
-        return 1;
-      }
+      var result = _compareFixesInDirectory(dir, edits);
+      log.stdout('Passed: ${result.passCount}, Failed: ${result.failCount}');
+      return result.failCount > 0 ? 1 : 0;
     } else if (edits.isEmpty) {
       log.stdout('Nothing to fix!');
     } else {
       var details = fixes.details;
       details.sort((f1, f2) => path
-          .relative(f1.path, from: dir.path)
-          .compareTo(path.relative(f2.path, from: dir.path)));
+          .relative(f1.path, from: dirPath)
+          .compareTo(path.relative(f2.path, from: dirPath)));
 
       var fileCount = 0;
       var fixCount = 0;
@@ -170,32 +172,83 @@ To use the tool, run either ['dart fix --dry-run'] for a preview of the proposed
 
   /// Return `true` if any of the fixes fail to create the same content as is
   /// found in the golden file.
-  bool _compareFixes(List<SourceFileEdit> edits) {
-    var passCount = 0;
-    var failCount = 0;
+  _TestResult _compareFixesInDirectory(
+      io.Directory directory, List<SourceFileEdit> edits) {
+    var result = _TestResult();
+    //
+    // Gather the files of interest in this directory and process
+    // subdirectories.
+    //
+    var dartFiles = <io.File>[];
+    var expectFileMap = <String, io.File>{};
+    for (var child in directory.listSync()) {
+      if (child is io.Directory) {
+        var childResult = _compareFixesInDirectory(child, edits);
+        result.passCount += childResult.passCount;
+        result.failCount += childResult.failCount;
+      } else if (child is io.File) {
+        var name = child.name;
+        if (name.endsWith('.dart')) {
+          dartFiles.add(child);
+        } else if (name.endsWith('.expect')) {
+          expectFileMap[child.path] = child;
+        }
+      }
+    }
+    var editMap = <String, SourceFileEdit>{};
     for (var edit in edits) {
-      var filePath = edit.file;
+      editMap[edit.file] = edit;
+    }
+    for (var originalFile in dartFiles) {
+      var filePath = originalFile.path;
       var baseName = path.basename(filePath);
       var expectFileName = baseName + '.expect';
       var expectFilePath = path.join(path.dirname(filePath), expectFileName);
+      var expectFile = expectFileMap.remove(expectFilePath);
+      if (expectFile == null) {
+        result.failCount++;
+        log.stdout(
+            'No corresponding expect file for the Dart file at "$filePath".');
+        continue;
+      }
+      var edit = editMap[filePath];
       try {
-        var originalCode = io.File(filePath).readAsStringSync();
-        var expectedCode = io.File(expectFilePath).readAsStringSync();
-        var actualCode = SourceEdit.applySequence(originalCode, edit.edits);
-        if (actualCode != expectedCode) {
-          failCount++;
+        var originalCode = originalFile.readAsStringSync();
+        var expectedCode = expectFile.readAsStringSync();
+        var actualCode = edit == null
+            ? originalCode
+            : SourceEdit.applySequence(originalCode, edit.edits);
+        // Use a whitespace insensitive comparison.
+        if (_compressWhitespace(actualCode) !=
+            _compressWhitespace(expectedCode)) {
+          result.failCount++;
           _reportFailure(filePath, actualCode, expectedCode);
+          _printEdits(edits);
         } else {
-          passCount++;
+          result.passCount++;
         }
       } on io.FileSystemException {
-        // Ignored for now.
+        result.failCount++;
+        log.stdout('Failed to process "$filePath".');
+        log.stdout(
+            '  Ensure that the file and its expect file are both readable.');
       }
     }
-    log.stdout('Passed: $passCount, Failed: $failCount');
-    return failCount > 0;
+    //
+    // Report any `.expect` files that have no corresponding `.dart` file.
+    //
+    for (var unmatchedExpectPath in expectFileMap.keys) {
+      result.failCount++;
+      log.stdout(
+          'No corresponding Dart file for the expect file at "$unmatchedExpectPath".');
+    }
+    return result;
   }
 
+  /// Compress sequences of whitespace characters into a single space.
+  String _compressWhitespace(String code) =>
+      code.replaceAll(RegExp(r'\s*'), ' ');
+
   String _pluralFix(int count) => count == 1 ? 'fix' : 'fixes';
 
   void _printDetails(List<BulkFix> details, io.Directory workingDir) {
@@ -215,6 +268,16 @@ To use the tool, run either ['dart fix --dry-run'] for a preview of the proposed
     }
   }
 
+  void _printEdits(List<SourceFileEdit> edits) {
+    log.stdout('Edits returned from server:');
+    for (var fileEdit in edits) {
+      log.stdout('  ${fileEdit.file}');
+      for (var edit in fileEdit.edits) {
+        log.stdout("    ${edit.offset} - ${edit.end}, '${edit.replacement}'");
+      }
+    }
+  }
+
   /// Report that the [actualCode] produced by applying fixes to the content of
   /// [filePath] did not match the [expectedCode].
   void _reportFailure(String filePath, String actualCode, String expectedCode) {
@@ -228,3 +291,15 @@ To use the tool, run either ['dart fix --dry-run'] for a preview of the proposed
 
   static String _format(int value) => _numberFormat.format(value);
 }
+
+/// The result of running tests in a given directory.
+class _TestResult {
+  /// The number of tests that passed.
+  int passCount = 0;
+
+  /// The number of tests that failed.
+  int failCount = 0;
+
+  /// Initialize a newly created result object.
+  _TestResult();
+}

pkg/dartdev/test/commands/fix_test.dart

@@ -21,12 +21,38 @@ const runFromSource = false;
 void defineFix() {
   TestProject p;
 
+  ProcessResult result;
+
   final bullet = Logger.standard().ansi.bullet;
 
   setUp(() => p = null);
 
   tearDown(() => p?.dispose());
 
+  void assertResult({int exitCode = 0}) {
+    String message;
+    if (result.exitCode != exitCode) {
+      if (result.stderr.isNotEmpty) {
+        message = 'Error code was ${result.exitCode} and stderr was not empty';
+      } else {
+        message = 'Error code was ${result.exitCode}';
+      }
+    } else if (result.stderr.isNotEmpty) {
+      message = 'stderr was not empty';
+    } else {
+      return;
+    }
+    fail('''
+$message
+
+stdout:
+${result.stdout}
+
+stderr:
+${result.stderr}
+''');
+  }
+
   ProcessResult runFix(List<String> args, {String workingDir}) {
     if (runFromSource) {
       var binary = path.join(Directory.current.path, 'bin', 'dartdev.dart');
@@ -188,7 +214,7 @@ linter:
   });
 
   group('compare-to-golden', () {
-    test('different', () {
+    test('applied fixes do not match expected', () {
       p = project(
         mainSrc: '''
 class A {
@@ -215,12 +241,11 @@ class B extends A {
   String a() => '';
 }
 ''');
-      var result = runFix(['--compare-to-golden', '.'], workingDir: p.dirPath);
-      expect(result.exitCode, 1);
-      expect(result.stderr, isEmpty);
+      result = runFix(['--compare-to-golden', '.'], workingDir: p.dirPath);
+      assertResult(exitCode: 1);
     });
 
-    test('same', () {
+    test('applied fixes match expected', () {
       p = project(
         mainSrc: '''
 class A {
@@ -248,9 +273,66 @@ class B extends A {
   String a() => '';
 }
 ''');
-      var result = runFix(['--compare-to-golden', '.'], workingDir: p.dirPath);
-      expect(result.exitCode, 0);
-      expect(result.stderr, isEmpty);
+      result = runFix(['--compare-to-golden', '.'], workingDir: p.dirPath);
+      assertResult();
+    });
+
+    test('missing expect', () {
+      p = project(
+        mainSrc: '''
+class A {
+  String a() => "";
+}
+
+class B extends A {
+  String a() => "";
+}
+''',
+        analysisOptions: '''
+linter:
+  rules:
+    - annotate_overrides
+    - prefer_single_quotes
+''',
+      );
+      result = runFix(['--compare-to-golden', '.'], workingDir: p.dirPath);
+      assertResult(exitCode: 1);
+    });
+
+    test('missing original', () {
+      p = project(mainSrc: '''
+class C {}
+''');
+      p.file('lib/main.dart.expect', '''
+class C {}
+''');
+      p.file('lib/secondary.dart.expect', '''
+class A {}
+''');
+      result = runFix(['--compare-to-golden', '.'], workingDir: p.dirPath);
+      assertResult(exitCode: 1);
+    });
+
+    test('no fixes to apply does not match expected', () {
+      p = project(
+        mainSrc: '''
+class A {
+  String a() => "";
+}
+''',
+        analysisOptions: '''
+linter:
+  rules:
+    - annotate_overrides
+''',
+      );
+      p.file('lib/main.dart.expect', '''
+class A {
+  String a() => '';
+}
+''');
+      result = runFix(['--compare-to-golden', '.'], workingDir: p.dirPath);
+      assertResult(exitCode: 1);
     });
   });
 }