Report more heuristics for tests that require manual work.

tools/migration/bin/migrate_batch.dart

 // Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /// Given the beginning and ending file names in a batch, does as much automated
 /// migration and possible and prints out the remaining manual steps required.
 ///
 /// This should be safe to run, and safe to re-run on an in-progress chunk.
 /// However, it has not been thoroughly tested, so run at your own risk.
 
 import 'dart:io';
 
 import 'package:path/path.dart' as p;
 
 import 'package:migration/src/log.dart';
+import 'package:migration/src/validate.dart';
 import 'package:status_file/status_file.dart';
 
 const simpleDirs = const ["corelib", "language", "lib"];
 
 final String sdkRoot =
     p.normalize(p.join(p.dirname(p.fromUri(Platform.script)), '../../../'));
 
 final String testRoot = p.join(sdkRoot, "tests");
 
 bool dryRun = false;
@@ skipping 39 matching lines @@
 
   if ((endIndex - startIndex) == 0) {
     print(bold("No tests to migrate."));
     return;
   }
 
   print("Migrating ${bold(endIndex - startIndex)} tests from ${bold(first)} "
       "to ${bold(last)}...");
   print("");
 
-  tests = tests.getRange(startIndex, endIndex);
-  var todos = <String>[];
+  var allTodos = <String, List<String>>{};
+  tests = tests.sublist(startIndex, endIndex);
   var migratedTests = 0;
   var unmigratedTests = 0;
   for (var test in tests) {
-    if (test.migrate(todos)) {
+    var todos = test.migrate();
+    if (todos.isEmpty) {
       migratedTests++;
     } else {
       unmigratedTests++;
+      allTodos[test.twoPath] = todos;
     }
   }
 
+  // Print status file entries.
+  var statusFileEntries = new StringBuffer();
+  var statusFiles = loadStatusFiles();
+  for (var statusFile in statusFiles) {
+    printStatusFileEntries(statusFileEntries, tests, statusFile);
+  }
+
+  new File("statuses.migration")
+      .writeAsStringSync(statusFileEntries.toString());
+  print("Wrote relevant test status file entries to 'statuses.migration'.");
+
+  // Tell the user what's left TODO.
   print("");
-
   var summary = "";
 
   if (migratedTests > 0) {
     var s = migratedTests == 1 ? "" : "s";
     summary += "Successfully migrated ${green(migratedTests)} test$s. ";
   }
 
   if (unmigratedTests > 0) {
-    var s = migratedTests == 1 ? "" : "s";
+    var s = unmigratedTests == 1 ? "" : "s";
     summary += "Need manual work on ${red(unmigratedTests)} test$s:";
   }
 
   print(summary);
-  todos.forEach(todo);
-
-  // Print status file entries.
-  var statusFileEntries = new StringBuffer();
-  var statusFiles = loadStatusFiles();
-  for (var statusFile in statusFiles) {
-    printStatusFileEntries(statusFileEntries, tests, statusFile);
+  var todoTests = allTodos.keys.toList();
+  todoTests.sort();
+  for (var todoTest in todoTests) {
+    print("- ${bold(todoTest)}:");
+    allTodos[todoTest].forEach(todo);
   }
-
-  new File("statuses.migration")
-      .writeAsStringSync(statusFileEntries.toString());
-  print(
-      bold("Wrote relevant test status file entries to 'statuses.migration'"));
 }
 
 /// Returns a [String] of the relevant status file entries associated with the
 /// tests in [tests] found in [statusFile].
 void printStatusFileEntries(
     StringBuffer statusFileEntries, List<Fork> tests, StatusFile statusFile) {
   var filteredStatusFile = new StatusFile(statusFile.path);
   var testNames = <String>[];
   for (var test in tests) {
     testNames.add(test.twoPath.split("/").last.split(".")[0]);
@@ skipping 109 matching lines @@
 }
 
 /// Moves the file from [from] to [to], which are both assumed to be relative
 /// paths inside "tests".
 void moveFile(String from, String to) {
   if (dryRun) {
     print("  Dry run: move $from to $to");
     return;
   }
 
+  // Create the directory if needed.
+  new Directory(p.dirname(p.join(testRoot, to))).createSync(recursive: true);

[bkonyi, 2017/07/26 17:59:26]

Nice! I just ran into this case.

+
   new File(p.join(testRoot, from)).renameSync(p.join(testRoot, to));
 }
 
 /// Reads the contents of the file at [path], which is assumed to be relative
 /// within "tests".
 String readFile(String path) {
   return new File(p.join(testRoot, path)).readAsStringSync();
 }
 
 /// Deletes the file at [path], which is assumed to be relative within "tests".
 void deleteFile(String path) {
   if (dryRun) {
     print("  Dry run: delete $path");
     return;
   }
 
   new File(p.join(testRoot, path)).deleteSync();
 }
 
-bool checkForUnitTest(String path, String source) {
-  if (!source.contains("package:unittest")) return false;
-
-  note("${bold(path)} uses unittest package.");
-  return true;
-}
-
 class Fork {
   final String twoPath;
   String onePath;
   String strongPath;
 
   String get twoSource {
     if (twoPath == null) return null;
     if (_twoSource == null) _twoSource = readFile(twoPath);
     return _twoSource;
   }
@@ skipping 11 matching lines @@
   String get strongSource {
     if (strongPath == null) return null;
     if (_strongSource == null) _strongSource = readFile(strongPath);
     return _strongSource;
   }
 
   String _strongSource;
 
   Fork(this.twoPath);
 
-  bool migrate(List<String> todos) {
+  List<String> migrate() {
     print("- ${bold(twoPath)}:");
 
-    var todosBefore = todos.length;
+    var todos = <String>[];
     var isMigrated = new File(p.join(testRoot, twoPath)).existsSync();
 
     // If there is a migrated version and it's the same as an unmigrated one,
     // delete the unmigrated one.
     if (isMigrated) {
       if (onePath != null) {
         if (oneSource == twoSource) {
           deleteFile(onePath);
           done("Deleted already-migrated $onePath.");
         } else {
           note("${bold(onePath)} does not match already-migrated "
               "${bold(twoPath)}.");
-          todos.add("Merge ${bold(onePath)} into ${bold(twoPath)}.");
-          checkForUnitTest(onePath, oneSource);
+          todos.add("Merge from ${bold(onePath)} into this file.");
+          validateFile(onePath, oneSource);
         }
       }
 
       if (strongPath != null) {
         if (strongSource == twoSource) {
           deleteFile(strongPath);
           done("Deleted already-migrated ${bold(strongPath)}.");
         } else {
           note("${bold(strongPath)} does not match already-migrated "
               "${bold(twoPath)}.");
-          todos.add("Merge ${bold(strongPath)} into ${bold(twoPath)}.");
-          checkForUnitTest(strongPath, strongSource);
+          todos.add("Merge from ${bold(strongPath)} into this file.");
+          validateFile(strongPath, strongSource);
         }
       }
     } else {
       // If it only exists in one place, just move it.
       if (strongPath == null) {
         moveFile(onePath, twoPath);
         done("Moved from ${bold(onePath)} (no strong mode fork).");
       } else if (onePath == null) {
         moveFile(strongPath, twoPath);
         done("Moved from ${bold(strongPath)} (no 1.0 mode fork).");
       } else if (oneSource == strongSource) {
         // The forks are identical, pick one.
         moveFile(onePath, twoPath);
         deleteFile(strongPath);
         done("Merged identical forks.");
-        checkForUnitTest(twoPath, oneSource);
+        validateFile(twoPath, oneSource);
       } else {
         // Otherwise, a manual merge is required. Start with the strong one.
+        print(new File(strongPath).existsSync());
         moveFile(strongPath, twoPath);
         done("Moved strong fork, kept 1.0 fork, manual merge required.");
-        todos.add("Merge ${bold(onePath)} into ${bold(twoPath)}.");
-        checkForUnitTest(onePath, oneSource);
+        todos.add("Merge from ${bold(onePath)} into this file.");
+        validateFile(onePath, oneSource);
       }
     }
 
-    if (checkForUnitTest(twoPath, twoSource)) {
-      todos.add("Migrate ${bold(twoPath)} off unittest.");
-    }
+    validateFile(twoPath, twoSource, todos);
 
-    return todos.length == todosBefore;
+    return todos;
   }
 }