Added functionality to dump relevant status file entries to a single file to make it easier to see …

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/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;
 
 void main(List<String> arguments) {
   if (arguments.contains("--dry-run")) {
     dryRun = true;
     arguments = arguments.where((argument) => argument != "--dry-run").toList();
   }
 
   if (arguments.length != 2) {
     stderr.writeln(
         "Usage: dart migrate_batch.dart [--dry-run] <first file> <last file>");
     stderr.writeln();
     stderr.writeln("Example:");
     stderr.writeln();
     stderr.writeln(
         "    \$ dart migrate_batch.dart corelib/map_to_string corelib/queue");
     exit(1);
   }
 
-  var first = toTwoPath(arguments[0]);
-  var last = toTwoPath(arguments[1]);
+  var first = arguments[0];
+  var last = arguments[1];
+
+  print("Preparing to migrate tests from ${bold(first)} to ${bold(last)}...");

[Bob Nystrom, 2017/07/24 21:47:47]

This is because the scan process is a lot longer with your change, right? Let's
see if we can fix that directly...

[bkonyi, 2017/07/25 22:13:39]

Oh yeah, this is here because I'm really not doing things as efficiently as I
could be when scanning through the status files. It's not terribly slow, but
it's noticeable for sure. I've fixed this now.

+
+  first = toTwoPath(first);
+  last = toTwoPath(last);
 
   var tests = scanTests();
 
   // Find the range of files in the chunk. We use comparisons here instead of
   // equality to try to compensate for files that may only appear in one fork
   // and should be part of the chunk but aren't officially listed as the begin
   // or end point.
   var startIndex = -1;
   var endIndex = 0;
   for (var i = 0; i < tests.length; i++) {
     if (startIndex == -1 && tests[i].twoPath.compareTo(first) >= 0) {
       startIndex = i;
     }
 
     if (tests[i].twoPath.compareTo(last) > 0) {
       endIndex = i;
       break;
     }
   }
 
+  if ((endIndex - startIndex) == 0) {
+    print(bold("No tests to migrate."));
+    return;
+  }
+
   print("Migrating ${bold(endIndex - startIndex)} tests from ${bold(first)} "
       "to ${bold(last)}...");
   print("");
 
   var todos = <String>[];
   var migratedTests = 0;
   var unmigratedTests = 0;
   for (var i = startIndex; i < endIndex; i++) {
     if (tests[i].migrate(todos)) {
       migratedTests++;
@@ skipping 11 matching lines @@
     summary += "Successfully migrated ${green(migratedTests)} test$s. ";
   }
 
   if (unmigratedTests > 0) {
     var s = migratedTests == 1 ? "" : "s";
     summary += "Need manual work on ${red(unmigratedTests)} test$s:";
   }
 
   print(summary);
   todos.forEach(todo);
+
+  // Print status file entries.
+  var statusFileEntries = "";
+  for (var i = startIndex; i < endIndex; i++) {
+    statusFileEntries += printStatusFileEntries(tests[i]);

[Bob Nystrom, 2017/07/24 21:47:47]

Instead of repeatedly concatenating strings, it's a little faster and more
idiomatic to use a StringBuffer. Something like:

var buffer = new StringBuffer();
for (var i = startIndex; i < endIndex; i++) {
   writeStatusFileEntries(buffer, tests[i]);
}

new File("statuses.migration").writeAsStringSync(buffer.toString());

void writeStatusFileEntries(StringBuffer buffer, Fork test) {
  if (test.onePath != null && !test.oneStatusFile.isEmpty) {
    buffer.writeln("Status file entries for: ${test.onePath}");
    ...
  }
}

[bkonyi, 2017/07/25 22:13:39]

Right, that makes sense. Done.

+  }
+
+  new File("statuses.migration").writeAsStringSync(statusFileEntries);
+  print(
+      bold("Wrote relevant test status file entries to 'statuses.migration'"));
+}
+
+String printStatusFileEntries(Fork test) {
+  var statusFileEntries = "";
+  if (test.onePath != null && !test.oneStatusFile.isEmpty) {
+    statusFileEntries +=
+        bold("Status file entries for: ${test.onePath}") + "\n";
+    for (var section in test.oneStatusFile.sections) {
+      statusFileEntries += section.toString();
+    }
+  }
+  if (test.strongPath != null && !test.strongStatusFile.isEmpty) {
+    if (statusFileEntries != "") {
+      statusFileEntries += "\n";
+    }
+    statusFileEntries +=
+        bold("Status file entries for: ${test.strongPath}") + "\n";
+    for (var section in test.strongStatusFile.sections) {
+      statusFileEntries += section.toString();
+    }
+  }
+  return statusFileEntries;
 }
 
 String toTwoPath(String path) {
   // Allow eliding "_test" and/or ".dart" to make things more command-line
   // friendly.
   if (!path.endsWith("_test.dart")) path += "_test.dart";
   if (!path.endsWith(".dart")) path += ".dart";
 
   for (var dir in simpleDirs) {
     if (p.isWithin(dir, path)) {
@@ skipping 23 matching lines @@
 /// tests that only appear in one fork or the other, or both.
 List<Fork> scanTests() {
   var tests = <String, Fork>{};
 
   addTestDirectory(String fromDir, String twoDir) {
     for (var entry
         in new Directory(p.join(testRoot, fromDir)).listSync(recursive: true)) {
       if (!entry.path.endsWith("_test.dart")) continue;
 
       var fromPath = p.relative(entry.path, from: testRoot);
+      var fromStatus = testRoot + "/$fromDir/${fromDir}.status";
+      var testName = entry.path.split("/").last.split(".")[0];
+      var statusFile = getStatusFileEntries(fromStatus, testName);

[Bob Nystrom, 2017/07/24 21:47:47]

This call does two things:

1. It reads the status file associated with this test.
2. It figures out which sections in the status file apply to the test.

Step 1 hits the file system (i.e. is slow) and does the same work for every test
in the directory. We shouldn't do that for each test.

Instead, how about:

1. After all the tests are scanned, do a separate scan to read all of the status
files in each test directory. Stuff the results in something like a Map<String,
StatusFile> where the keys are the file names. Or maybe Map<String,
List<StatusFile>> where the keys are the test directories and the values are the
lists of status files in each directory (since a directory may have more than
one).

2. Then, for each test being migrated, go through that map and find the relevant
status files and sections.

That way, any given status file is only read from disc once (similar to what we
do for the tests themselves).

[bkonyi, 2017/07/25 22:13:39]

Done.

       var twoPath = p.join(twoDir, p.relative(fromPath, from: fromDir));
-
       var fork = tests.putIfAbsent(twoPath, () => new Fork(twoPath));
       if (fromDir.contains("_strong")) {
         fork.strongPath = fromPath;
+        fork.strongStatusFile = statusFile;
       } else {
         fork.onePath = fromPath;
+        fork.oneStatusFile = statusFile;
       }
     }
   }
 
   addTestDirectory("corelib", "corelib_2");
   addTestDirectory("corelib_strong", "corelib_2");
   addTestDirectory("html", "lib_2/html");
   addTestDirectory("isolate", "lib_2/isolate");
   addTestDirectory("language", "language_2");
   addTestDirectory("language_strong", "language_2");
   addTestDirectory("lib", "lib_2");
   addTestDirectory("lib_strong", "lib_2");
 
   var sorted = tests.values.toList();
   sorted.sort((a, b) => a.twoPath.compareTo(b.twoPath));
   return sorted;
 }
 
+StatusFile getStatusFileEntries(String path, String test) {
+  final statusFile = new StatusFile.read(path);
+  final filteredStatusFile = new StatusFile(path);

[Bob Nystrom, 2017/07/24 21:47:47]

Style nit: Elsewhere, I tend to use "var" even for locals that aren't modified.
I don't find "final" adds too much value since most methods are pretty short
anyway.

If you do really like using final, I'll note that you can use it for the loop
variables below as well. (A for-in loop in Dart doesn't mutate the loop
variable, it creates a new one each iteration. The difference is subtle, but
user-visible if you close over a loop variable in a closure.)

[bkonyi, 2017/07/25 22:13:40]

Hm, that's interesting. I've heard different opinions in the var vs final
debate. Do we have something about this in the official style guide?

+  for (var section in statusFile.sections) {
+    StatusSection currentSection = null;
+    for (var entry in section.entries) {
+      if (entry.contains(test)) {
+        if (currentSection == null) {
+          currentSection = new StatusSection(section.condition);
+        }
+        currentSection.entries.add(entry);
+      }
+    }
+    if (currentSection != null) {
+      filteredStatusFile.sections.add(currentSection);
+    }
+  }
+  return filteredStatusFile;
+}
+
 /// 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;
   }
 
   new File(p.join(testRoot, from)).renameSync(p.join(testRoot, to));
 }
@@ skipping 17 matching lines @@
 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;
+  StatusFile oneStatusFile;

[Bob Nystrom, 2017/07/24 21:47:47]

Unfortunately, to make things more annoying, there may be more than one status
file for a given directory. For example, language/ has:

language.status
language_dart2js.status
language_kernel.status

Ideally, it would find all of those. You could probably just look in the
directory for every "*.status" file.

[bkonyi, 2017/07/25 22:13:39]

Ah, I didn't realize that. Fixed.

   String strongPath;
+  StatusFile strongStatusFile;
 
   String get twoSource {
     if (twoPath == null) return null;
     if (_twoSource == null) _twoSource = readFile(twoPath);
     return _twoSource;
   }
 
   String _twoSource;
 
   String get oneSource {
@@ skipping 69 matching lines @@
       }
     }
 
     if (checkForUnitTest(twoPath, twoSource)) {
       todos.add("Migrate ${bold(twoPath)} off unittest.");
     }
 
     return todos.length == todosBefore;
   }
 }