Simplify enqueueBrowserTest().

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 734 matching lines @@
     } else {
       createTestCase(
           filePath,
           filePath,
           optionsFromFile['hasCompileError'] as bool,
           optionsFromFile['hasRuntimeError'] as bool,
           hasStaticWarning: optionsFromFile['hasStaticWarning'] as bool);
     }
   }
 
-  static Path _findPubspecYamlFile(Path filePath) {
-    var root = TestUtils.dartDir;
-    assert("$filePath".startsWith("$root"));
-
-    // We start with the parent directory of [filePath] and go up until
-    // the root directory (excluding the root).
-    List<String> segments = filePath.directoryPath.relativeTo(root).segments();
-    while (segments.length > 0) {
-      var pubspecYamlPath = new Path(segments.join('/')).append('pubspec.yaml');
-      if (TestUtils.existsCache.doesFileExist(pubspecYamlPath.toNativePath())) {
-        return root.join(pubspecYamlPath);
-      }
-      segments.removeLast();
-    }
-
-    return null;
-  }
-
   void enqueueTestCaseFromTestInformation(TestInformation info) {
     String testName = buildTestCaseDisplayName(suiteDir, info.originTestPath,
         multitestName: info.optionsFromFile['isMultitest'] as bool
             ? info.multitestKey
             : "");
     Set<Expectation> expectations = testExpectations.expectations(testName);
     if (info is HtmlTestInformation) {
-      enqueueHtmlTest(info, testName, expectations);
+      _enqueueHtmlTest(info, testName, expectations);
       return;
     }
 
     var optionsFromFile = info.optionsFromFile;
 
     // If this test is inside a package, we will check if there is a
     // pubspec.yaml file and if so, create a custom package root for it.
     Path packageRoot;
     Path packages;
 
@@ skipping 24 matching lines @@
         expectationsMap = <String, Set<Expectation>>{};
         for (var name in subtestNames) {
           var fullTestName = '$testName/$name';
           expectationsMap[fullTestName] =
               testExpectations.expectations(fullTestName);
         }
       } else {
         expectationsMap = {testName: expectations};
       }
 
-      enqueueBrowserTest(
+      _enqueueBrowserTest(
           packageRoot, packages, info, testName, expectationsMap);
     } else {
       enqueueStandardTest(info, testName, expectations);
     }
   }
 
   void enqueueStandardTest(
       TestInformation info, String testName, Set<Expectation> expectations) {
     var commonArguments =
         commonArgumentsFromFile(info.filePath, info.optionsFromFile);
@@ skipping 179 matching lines @@
    * JavaScript.  This function creates a working directory to hold the
    * JavaScript version of the test, and copies the appropriate framework
    * files to that directory.  It creates a [BrowserTestCase], which has
    * two sequential steps to be run by the [ProcessQueue] when the test is
    * executed: a compilation step and an execution step, both with the
    * appropriate executable and arguments. The [expectations] object can be
    * either a Set<String> if the test is a regular test, or a Map<String
    * subTestName, Set<String>> if we are running a browser multi-test (one
    * compilation and many browser runs).
    */
-  void enqueueBrowserTest(Path packageRoot, Path packages, TestInformation info,
-      String testName, Map<String, Set<Expectation>> expectations) {
+  void _enqueueBrowserTest(
+      Path packageRoot,
+      Path packages,
+      TestInformation info,
+      String testName,
+      Map<String, Set<Expectation>> expectations) {
     var badChars = new RegExp('[-=/]');
     var vmOptionsList = getVmOptions(info.optionsFromFile);
     var multipleOptions = vmOptionsList.length > 1;
     for (var vmOptions in vmOptionsList) {
       var optionsName =
           multipleOptions ? vmOptions.join('-').replaceAll(badChars, '') : '';
       var tempDir = createOutputDirectory(info.filePath, optionsName);
-      enqueueBrowserTestWithOptions(packageRoot, packages, info, testName,
+      _enqueueBrowserTestWithOptions(packageRoot, packages, info, testName,
           expectations, vmOptions, tempDir);
     }
   }
 
-  void enqueueBrowserTestWithOptions(
+  void _enqueueBrowserTestWithOptions(
       Path packageRoot,
       Path packages,
       TestInformation info,
       String testName,
       Map<String, Set<Expectation>> expectations,
       List<String> vmOptions,
       String tempDir) {
     // TODO(Issue 14651): If we're on dartium, we need to pass [packageRoot]
     // on to the browser (it may be test specific).
     var filePath = info.filePath;
     var fileName = filePath.toString();
 
     var optionsFromFile = info.optionsFromFile;
     var compilationTempDir = createCompilationOutputDirectory(info.filePath);
     var dartWrapperFilename = '$tempDir/test.dart';
     var compiledDartWrapperFilename = '$compilationTempDir/test.js';
     var dir = filePath.directoryPath;
     var nameNoExt = filePath.filenameWithoutExtension;
     var customHtmlPath = dir.append('$nameNoExt.html').toNativePath();
     var customHtml = new File(customHtmlPath);
 
-    // Construct the command(s) that compile all the inputs needed by the
-    // browser test. For running Dart in DRT, this will be noop commands.
-    var commands = <Command>[];
-
     // Use existing HTML document if available.
     String htmlPath;
     String content;
     if (customHtml.existsSync()) {
-      // If necessary, run the Polymer deploy steps.
-      // TODO(jmesserly): this should be generalized for any tests that
-      // require Pub deploy, not just polymer.
-      // TODO(rnystrom): This does not appear to be used any more. Remove.
-      if (customHtml.readAsStringSync().contains('<!--polymer-test')) {
-        if (configuration.compiler != Compiler.none) {
-          commands.add(
-              _polymerDeployCommand(customHtmlPath, tempDir, optionsFromFile));
+      htmlPath = '$tempDir/test.html';
+      dartWrapperFilename = filePath.toNativePath();
 
-          Path pubspecYamlFile = _findPubspecYamlFile(filePath);
-          Path homeDir =
-              (pubspecYamlFile == null) ? dir : pubspecYamlFile.directoryPath;
-          htmlPath = '$tempDir/${dir.relativeTo(homeDir)}/$nameNoExt.html';
-          dartWrapperFilename = '${htmlPath}_bootstrap.dart';
-          compiledDartWrapperFilename = '$dartWrapperFilename.js';
-        } else {
-          htmlPath = customHtmlPath;
-        }
+      var htmlContents = customHtml.readAsStringSync();
+      if (configuration.compiler == Compiler.none) {
+        var dartUrl = _createUrlPathFromFile(filePath);
+        var dartScript =
+            '<script type="application/dart" src="$dartUrl"></script>';
+        var jsUrl = '/packages/browser/dart.js';
+        var jsScript = '<script type="text/javascript" src="$jsUrl"></script>';
+        htmlContents =
+            htmlContents.replaceAll('%TEST_SCRIPTS%', '$dartScript\n$jsScript');
       } else {
-        htmlPath = '$tempDir/test.html';
-        dartWrapperFilename = filePath.toNativePath();
-
-        var htmlContents = customHtml.readAsStringSync();
-        if (configuration.compiler == Compiler.none) {
-          var dartUrl = _createUrlPathFromFile(filePath);
-          var dartScript =
-              '<script type="application/dart" src="$dartUrl"></script>';
-          var jsUrl = '/packages/browser/dart.js';
-          var jsScript =
-              '<script type="text/javascript" src="$jsUrl"></script>';
-          htmlContents = htmlContents.replaceAll(
-              '%TEST_SCRIPTS%', '$dartScript\n$jsScript');
-        } else {
-          compiledDartWrapperFilename = '$tempDir/$nameNoExt.js';
-          var jsUrl = '$nameNoExt.js';
-          htmlContents = htmlContents.replaceAll(
-              '%TEST_SCRIPTS%', '<script src="$jsUrl"></script>');
-        }
-        new File(htmlPath).writeAsStringSync(htmlContents);
+        compiledDartWrapperFilename = '$tempDir/$nameNoExt.js';
+        htmlContents = htmlContents.replaceAll(
+            '%TEST_SCRIPTS%', '<script src="$nameNoExt.js"></script>');
       }
+      new File(htmlPath).writeAsStringSync(htmlContents);
     } else {
       htmlPath = '$tempDir/test.html';
       if (configuration.compiler != Compiler.dart2js &&
           configuration.compiler != Compiler.dartdevc) {
         // test.dart will import the dart test.
         _createWrapperFile(dartWrapperFilename, filePath);
       } else {
         dartWrapperFilename = fileName;
       }
 
       // Create the HTML file for the test.
-      var htmlTest = new File(htmlPath).openSync(mode: FileMode.WRITE);
-
       var scriptPath = dartWrapperFilename;
       if (configuration.compiler != Compiler.none) {
         scriptPath = compiledDartWrapperFilename;
       }
       scriptPath = _createUrlPathFromFile(new Path(scriptPath));
 
       if (configuration.compiler != Compiler.dartdevc) {
         content = getHtmlContents(fileName, scriptType, scriptPath);
       } else {
         var jsDir = new Path(compilationTempDir)
             .relativeTo(TestUtils.dartDir)
             .toString();
         content = dartdevcHtml(nameNoExt, jsDir);
       }
 
-      htmlTest.writeStringSync(content);
-      htmlTest.closeSync();
+      new File(htmlPath).writeAsStringSync(content);
     }
 
+    // Construct the command(s) that compile all the inputs needed by the
+    // browser test. For running Dart in DRT, this will be noop commands.
+    var commands = <Command>[];
+
     switch (configuration.compiler) {
       case Compiler.dart2js:
         commands.add(_dart2jsCompileCommand(dartWrapperFilename,
             compiledDartWrapperFilename, tempDir, optionsFromFile));
         break;
 
       case Compiler.dartdevc:
         commands.add(_dartdevcCompileCommand(dartWrapperFilename,
             '$compilationTempDir/$nameNoExt.js', optionsFromFile));
         break;
 
       case Compiler.none:
         break;
 
       default:
         assert(false);
     }
 
     // Some tests require compiling multiple input scripts.
-    var otherScripts = optionsFromFile['otherScripts'] as List<String>;
-    for (var name in otherScripts) {
+    for (var name in optionsFromFile['otherScripts'] as List<String>) {
       var namePath = new Path(name);
       var fromPath = filePath.directoryPath.join(namePath);
 
       switch (configuration.compiler) {
         case Compiler.dart2js:
           commands.add(_dart2jsCompileCommand(fromPath.toNativePath(),
               '$tempDir/${namePath.filename}.js', tempDir, optionsFromFile));
           break;
 
         case Compiler.dartdevc:
           commands.add(_dartdevcCompileCommand(fromPath.toNativePath(),
-              '$tempDir/$nameNoExt.js', optionsFromFile));
+              '$tempDir/${namePath.filename}.js', optionsFromFile));
           break;
 
         default:
           assert(configuration.compiler == Compiler.none);
       }
 
       if (configuration.compiler == Compiler.none) {
         // For the tests that require multiple input scripts but are not
         // compiled, move the input scripts over with the script so they can
         // be accessed.
-        var result = new File(fromPath.toNativePath()).readAsStringSync();
-        new File('$tempDir/${namePath.filename}').writeAsStringSync(result);
+        new File(fromPath.toNativePath())
+            .copySync('$tempDir/${namePath.filename}');
       }
     }
 
-    // Variables for browser multi-tests.
-    var isMultitest = info.optionsFromFile['isMultiHtmlTest'] as bool;
-    var subtestNames = isMultitest
-        ? (info.optionsFromFile['subtestNames'] as List<String>)
-        : <String>[null];
-    for (var subtestName in subtestNames) {
-      // Construct the command that executes the browser test
-      var commandSet = commands.toList();
-
-      var htmlPath_subtest = _createUrlPathFromFile(new Path(htmlPath));
-      var fullHtmlPath =
-          _getUriForBrowserTest(htmlPath_subtest, subtestName).toString();
-
-      if (configuration.runtime == Runtime.drt) {
-        var dartFlags = <String>[];
-        var contentShellOptions = ['--no-timeout', '--run-layout-test'];
-
-        // Disable the GPU under Linux and Dartium. If the GPU is enabled,
-        // Chrome may send a termination signal to a test.  The test will be
-        // terminated if a machine (bot) doesn't have a GPU or if a test is
-        // still running after a certain period of time.
-        if (configuration.system == System.linux &&
-            configuration.runtime == Runtime.drt) {
-          contentShellOptions.add('--disable-gpu');
-          // TODO(terry): Roll 50 need this in conjection with disable-gpu.
-          contentShellOptions.add('--disable-gpu-early-init');
-        }
-
-        if (configuration.compiler == Compiler.none) {
-          dartFlags.add('--ignore-unrecognized-flags');
-          if (configuration.isChecked) {
-            dartFlags.add('--enable_asserts');
-            dartFlags.add("--enable_type_checks");
-          }
-          dartFlags.addAll(vmOptions);
-        }
-
-        commandSet.add(Command.contentShell(contentShellFilename, fullHtmlPath,
-            contentShellOptions, dartFlags, environmentOverrides));
-      } else {
-        commandSet.add(Command.browserTest(fullHtmlPath, configuration,
-            retry: !isNegative(info)));
+    if (info.optionsFromFile['isMultiHtmlTest'] as bool) {
+      // Variables for browser multi-tests.
+      var subtestNames = info.optionsFromFile['subtestNames'] as List<String>;
+      for (var subtestName in subtestNames) {
+        _enqueueSingleBrowserTest(commands, info, '$testName/$subtestName',
+            subtestName, expectations, vmOptions, htmlPath);
       }
-
-      // Create BrowserTestCase and queue it.
-      var fullTestName = isMultitest ? '$testName/$subtestName' : testName;
-      var expectation = expectations[fullTestName];
-      var testCase = new BrowserTestCase('$suiteName/$fullTestName', commandSet,
-          configuration, expectation, info, isNegative(info), fullHtmlPath);
-
-      enqueueNewTestCase(testCase);
+    } else {
+      _enqueueSingleBrowserTest(
+          commands, info, testName, null, expectations, vmOptions, htmlPath);
     }
   }
 
-  void enqueueHtmlTest(HtmlTestInformation info, String testName,
+  /// Enqueues a single browser test, or a single subtest of an HTML multitest.
+  void _enqueueSingleBrowserTest(
+      List<Command> commands,
+      TestInformation info,
+      String testName,
+      String subtestName,
+      Map<String, Set<Expectation>> expectations,
+      List<String> vmOptions,
+      String htmlPath) {
+    // Construct the command that executes the browser test.
+    commands = commands.toList();
+
+    var htmlPathSubtest = _createUrlPathFromFile(new Path(htmlPath));
+    var fullHtmlPath =
+        _getUriForBrowserTest(htmlPathSubtest, subtestName).toString();
+
+    if (configuration.runtime == Runtime.drt) {
+      var dartFlags = <String>[];
+      var contentShellOptions = ['--no-timeout', '--run-layout-test'];
+
+      // Disable the GPU under Linux and Dartium. If the GPU is enabled,
+      // Chrome may send a termination signal to a test.  The test will be
+      // terminated if a machine (bot) doesn't have a GPU or if a test is
+      // still running after a certain period of time.
+      if (configuration.system == System.linux &&
+          configuration.runtime == Runtime.drt) {
+        contentShellOptions.add('--disable-gpu');
+        // TODO(terry): Roll 50 need this in conjection with disable-gpu.
+        contentShellOptions.add('--disable-gpu-early-init');
+      }
+
+      if (configuration.compiler == Compiler.none) {
+        dartFlags.add('--ignore-unrecognized-flags');
+        if (configuration.isChecked) {
+          dartFlags.add('--enable_asserts');
+          dartFlags.add("--enable_type_checks");
+        }
+        dartFlags.addAll(vmOptions);
+      }
+
+      commands.add(Command.contentShell(contentShellFilename, fullHtmlPath,
+          contentShellOptions, dartFlags, environmentOverrides));
+    } else {
+      commands.add(Command.browserTest(fullHtmlPath, configuration,
+          retry: !isNegative(info)));
+    }
+
+    // Create BrowserTestCase and queue it.
+    var expectation = expectations[testName];
+    var testCase = new BrowserTestCase('$suiteName/$testName', commands,
+        configuration, expectation, info, isNegative(info), fullHtmlPath);
+
+    enqueueNewTestCase(testCase);
+  }
+
+  void _enqueueHtmlTest(HtmlTestInformation info, String testName,
       Set<Expectation> expectations) {
     var compiler = configuration.compiler;
     var runtime = configuration.runtime;
 
     if (compiler == Compiler.dartdevc) {
       // TODO(rnystrom): Support this for dartdevc (#29919).
       print("Ignoring $testName on dartdevc since HTML tests are not "
           "implemented for that compiler yet.");
       return;
     }
@@ skipping 114 matching lines @@
 
     return Command.compilation(
         Compiler.dartdevc.name,
         outputFile,
         configuration.compilerConfiguration.bootstrapDependencies(buildDir),
         compilerPath,
         args,
         environmentOverrides);
   }
 
-  /** Helper to create a Polymer deploy command for a single HTML file. */
-  Command _polymerDeployCommand(String inputFile, String outputDir,
-      Map<String, dynamic> optionsFromFile) {
-    var args = <String>[];
-    var packages = packagesArgument(optionsFromFile['packageRoot'] as String,
-        optionsFromFile['packages'] as String);
-    if (packages != null) args.add(packages);
-    args
-      ..add('package:polymer/deploy.dart')
-      ..add('--test')
-      ..add(inputFile)
-      ..add('--out')
-      ..add(outputDir)
-      ..add('--file-filter')
-      ..add('.svn');
-    if (configuration.isCsp) args.add('--csp');
-
-    return Command.process(
-        'polymer_deploy', dartVmBinaryFileName, args, environmentOverrides);
-  }
-
   String get scriptType {
     switch (configuration.compiler) {
       case Compiler.none:
         return 'application/dart';
       case Compiler.dart2js:
       case Compiler.dart2analyzer:
       case Compiler.dartdevc:
         return 'text/javascript';
       default:
         print('Non-web runtime, so no scriptType for: '
@@ skipping 341 matching lines @@
 
 /// Used for testing packages in on off settings, i.e., we pass in the actual
 /// directory that we want to test.
 class PKGTestSuite extends StandardTestSuite {
   PKGTestSuite(Configuration configuration, Path directoryPath)
       : super(configuration, directoryPath.filename, directoryPath,
             ["$directoryPath/.status"],
             isTestFilePredicate: (f) => f.endsWith('_test.dart'),
             recursive: true);
 
-  void enqueueBrowserTest(Path packageRoot, packages, TestInformation info,
+  void _enqueueBrowserTest(Path packageRoot, packages, TestInformation info,
       String testName, Map<String, Set<Expectation>> expectations) {
     var filePath = info.filePath;
     var dir = filePath.directoryPath;
     var nameNoExt = filePath.filenameWithoutExtension;
     var customHtmlPath = dir.append('$nameNoExt.html');
     var customHtml = new File(customHtmlPath.toNativePath());
     if (!customHtml.existsSync()) {
-      super.enqueueBrowserTest(
+      super._enqueueBrowserTest(
           packageRoot, packages, info, testName, expectations);
     } else {
       var relativeHtml = customHtmlPath.relativeTo(TestUtils.dartDir);
       var fullPath = _createUrlPathFromFile(customHtmlPath);
 
       var commands = [
         Command.browserTest(fullPath, configuration, retry: !isNegative(info))
       ];
       var testDisplayName = '$suiteName/$testName';
       enqueueNewTestCase(new BrowserTestCase(
@@ skipping 57 matching lines @@
 
   bool isTestFile(String filename) {
     // NOTE: We exclude tests and patch files for now.
     return filename.endsWith(".dart") &&
         !filename.endsWith("_test.dart") &&
         !filename.contains("_internal/js_runtime/lib");
   }
 
   bool get listRecursively => true;
 }