Address review comments from CL 10151014.

dart/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 373 matching lines @@
     if (shards > 1) {
       int shard = configuration['shard'];
       if (testName.hashCode() % shards != shard - 1) {
         return;
       }
     }
 
     Set<String> expectations = testExpectations.expectations(testName);
     if (configuration['report']) {
       // Tests with multiple VMOptions are counted more than once.
-      for (var dummy in optionsFromFile["vmOptions"]) {
+      for (var dummy in getVmOptions(optionsFromFile)) {
         if (TestUtils.isBrowserRuntime(configuration['runtime']) &&
             optionsFromFile['isMultitest']) {
           break;  // Browser tests skip multitests.
         }
         SummaryReport.add(expectations);
       }
     }
     if (expectations.contains(SKIP)) return;
 
     if (TestUtils.isBrowserRuntime(configuration['runtime'])) {
@@ skipping 35 matching lines @@
 
   List<Command> makeCommands(TestInformation info, var args) {
     if (configuration['compiler'] == 'dart2js') {
       args = new List.from(args);
       String testPath =
           new File(info.filename).fullPathSync().replaceAll('\\', '/');
       Directory tempDir = createOutputDirectory(testPath, '');
       args.add('--out=${tempDir.path}/out.js');
       List<Command> commands = <Command>[new Command(shellPath(), args)];
       if (configuration['runtime'] == 'd8') {
-        var d8 = '${TestUtils.buildDir(configuration)}/'
-                 'd8${TestUtils.executableSuffix("d8")}';
+        var d8 = TestUtils.d8FileName(configuration);
         commands.add(new Command(d8, ['${tempDir.path}/out.js']));
       }
       return commands;
     } else {
       return <Command>[new Command(shellPath(), args)];
     }
   }
 
   Function makeTestCaseCreator(Map optionsFromFile) {
     return (String filename,
@@ skipping 60 matching lines @@
     if (!isLibraryDefinition && optionsFromFile['containsSourceOrImport']) {
       print('Warning for $filename: Browser tests require #library ' +
             'in any file that uses #import, #source, or #resource');
     }
 
     final String compiler = configuration['compiler'];
     final String runtime = configuration['runtime'];
     final String testPath =
         new File(filename).fullPathSync().replaceAll('\\', '/');
 
-    for (var vmOptions in optionsFromFile['vmOptions']) {
+    for (var vmOptions in getVmOptions(optionsFromFile)) {
       // Create a unique temporary directory for each set of vmOptions.
       // TODO(dart:429): Replace separate replaceAlls with a RegExp when
       // replaceAll(RegExp, String) is implemented.
       String optionsName = '';
-      if (optionsFromFile['vmOptions'].length > 1) {
+      if (getVmOptions(optionsFromFile).length > 1) {
           optionsName = Strings.join(vmOptions, '-').replaceAll('-','')
                                                     .replaceAll('=','')
                                                     .replaceAll('/','');
       }
       Directory tempDir = createOutputDirectory(testPath, optionsName);
 
       String dartWrapperFilename = '${tempDir.path}/test.dart';
       String compiledDartWrapperFilename = '${tempDir.path}/test.js';
 
       String htmlPath = '${tempDir.path}/test.html';
@@ skipping 304 matching lines @@
       args.add('--error_format');
       args.add('machine');
     }
     if ((configuration['compiler'] == 'frog')
         && (configuration['runtime'] == 'none')) {
       args.add('--compile-only');
     }
 
     bool isMultitest = optionsFromFile["isMultitest"];
     List<String> dartOptions = optionsFromFile["dartOptions"];
-    List<List<String>> vmOptionsList = optionsFromFile["vmOptions"];
-    if (configuration['compiler'] == 'dart2js') {
-      vmOptionsList = [[]];
-    }
+    List<List<String>> vmOptionsList = getVmOptions(optionsFromFile);
     Expect.isTrue(!isMultitest || dartOptions == null);
     if (dartOptions == null) {
       args.add(filename);
     } else {
       var executable_name = dartOptions[0];
       // TODO(ager): Get rid of this hack when the runtime checkout goes away.
       var file = new File(executable_name);
       if (!file.existsSync()) {
         executable_name = '../$executable_name';
         Expect.isTrue(new File(executable_name).existsSync());
@@ skipping 113 matching lines @@
              "otherScripts": otherScripts,
              "isMultitest": isMultitest,
              "containsLeadingHash": containsLeadingHash,
              "isolateStubs": isolateStubs,
              "containsDomImport": containsDomImport,
              "isLibraryDefinition": isLibraryDefinition,
              "containsSourceOrImport": containsSourceOrImport,
              "numStaticTypeAnnotations": numStaticTypeAnnotations,
              "numCompileTimeAnnotations": numCompileTimeAnnotations};
   }
+
+  List<List<String>> getVmOptions(Map optionsFromFile) {
+    if (configuration['compiler'] == 'dart2js') {
+      return [[]];
+    } else {
+      return optionsFromFile['vmOptions'];
+    }
+  }
 }
 
 
 class DartcCompilationTestSuite extends StandardTestSuite {
   List<String> _testDirs;
   int activityCount = 0;
 
   DartcCompilationTestSuite(Map configuration,
                             String suiteName,
                             String directoryPath,
@@ skipping 241 matching lines @@
       default:
         throw "Unknown compiler for: ${configuration['compiler']}";
     }
   }
 
   static String dartShellFileName(Map configuration) {
     var name = configuration['dart'];
     if (name == '') {
       name = '${buildDir(configuration)}/${executableName(configuration)}';
     }
-    if (!(new File(name)).existsSync() && !configuration['list']) {
-      throw "Executable '$name' does not exist";
+    ensureExists(name, configuration);
+    return name;
+  }
+
+  static String d8FileName(Map configuration) {
+    var suffix = executableSuffix('d8');
+    var d8 = '${buildDir(configuration)}/d8$suffix';
+    ensureExists(d8, configuration);
+    return d8;
+  }
+
+  static void ensureExists(String filename, Map configuration) {
+    if (!configuration['list'] && !(new File(filename).existsSync())) {
+      throw "Executable '$filename' does not exist";
     }
-    return name;
   }
 
   static String compilerPath(Map configuration) {
     if (configuration['compiler'] == 'none') {
       return null;  // No separate compiler for dartium tests.
     }
     var name = configuration['frog'];
     if (name == '') {
       name = '${buildDir(configuration)}/${compilerName(configuration)}';
     }
@@ skipping 97 matching lines @@
  * $noCrash tests are expected to be flaky but not crash
  * $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
 """;
     print(report);
    }
 }