Code review changes for r36718.

tools/testing/dart/test_suite.dart

 // Copyright (c) 2012, 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.
 
 /**
  * Classes and methods for enumerating and preparing tests.
  *
  * This library includes:
  *
  * - Creating tests by listing all the Dart files in certain directories,
@@ skipping 384 matching lines @@
 
   Future<Map> discoverPackagesInRepository() {
     /*
      * Layout of packages inside the dart repository:
      *  dart/
      *      pkg/PACKAGE_NAME
      *      pkg/third_party/PACKAGE_NAME
      *      third_party/pkg/PACKAGE_NAME
      */
 
-    isValid(packageName) => packageName != 'third_party';
+    // Packages that contain "-" in their names are pinned to specific versions.
+    // We can ignore those for the purposes of testing.

[kustermann, 2014/05/28 07:13:57]

This comment is misleading (as I already mentioned in the first CL). I think a
better comment would be:

// Directories containing "-" are not valid dart packages and we
// therefore do not include them in the list of packages.

[nweiz, 2014/05/28 20:33:27]

Done.

+    isValid(packageName) =>
+        packageName != 'third_party' && !packageName.contains('-');
 
     var dartDir = TestUtils.dartDir;
     var futures = [
       listDir(dartDir.append('pkg'), isValid),
       listDir(dartDir.append('pkg').append('third_party'), isValid),
       listDir(dartDir.append('third_party').append('pkg'), isValid),
     ];
     return Future.wait(futures).then((results) {
       var packageDirectories = {};
       for (var result in results) {
         for (var packageTuple in result) {
           String packageName = packageTuple[0];
-
-          // Some packages are pinned to specific versions. We can ignore those
-          // for the purposes of testing.
-          if (packageName.contains('-')) continue;
-
           String fullPath = packageTuple[1];
           String yamlFile =
               new Path(fullPath).append('pubspec.yaml').toNativePath();
           if (new File(yamlFile).existsSync()) {
             packageDirectories[packageName] = fullPath;
           }
         }
       }
       return packageDirectories;
     });
@@ skipping 1791 matching lines @@
  * $pass tests are expected to pass
  * $failOk tests are expected to fail that we won't fix
  * $fail tests are expected to fail that we should fix
  * $crash tests are expected to crash that we should fix
  * $timeout tests are allowed to timeout
  * $compileErrorSkip tests are skipped on browsers due to compile-time error
 """;
     print(report);
   }
 }