Implement testing support for "--compiler=none --runtime=dartium"

tools/testing/dart/browser_controller.dart

 // Copyright (c) 2013, 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.
 library browser;
 
 import "dart:async";
 import "dart:convert" show LineSplitter, UTF8;
 import "dart:core";
 import "dart:io";
 
@@ skipping 35 matching lines @@
   // of close()
   Future done;
 
   Browser();
 
   factory Browser.byName(String name) {
     if (name == 'ff' || name == 'firefox') {
       return new Firefox();
     } else if (name == 'chrome') {
       return new Chrome();
+    } else if (name == 'dartium') {
+      return new Dartium();
     } else if (name == 'safari') {
       return new Safari();
     } else if (name.startsWith('ie')) {
       return new IE();
     } else {
       throw "Non supported browser";
     }
   }
 
   static const List<String> SUPPORTED_BROWSERS =
-    const ['safari', 'ff', 'firefox', 'chrome', 'ie9', 'ie10'];
+    const ['safari', 'ff', 'firefox', 'chrome', 'ie9', 'ie10', 'dartium'];
 
   static const List<String> BROWSERS_WITH_WINDOW_SUPPORT =
       const ['safari', 'ff', 'firefox', 'chrome'];
 
   // TODO(kustermann): add standard support for chrome on android
   static bool supportedBrowser(String name) {
     return SUPPORTED_BROWSERS.contains(name);
   }
 
   void _logEvent(String event) {
@@ skipping 25 matching lines @@
     } else {
       _logEvent("The process is already dead.");
       return new Future.value(true);
     }
   }
 
   /**
    * Start the browser using the supplied argument.
    * This sets up the error handling and usage logging.
    */
-  Future<bool> startBrowser(String command, List<String> arguments) {
-    return Process.start(command, arguments).then((startedProcess) {
+  Future<bool> startBrowser(String command,
+                            List<String> arguments,
+                            {Map<String,String> environment}) {
+    return Process.start(command, arguments, environment: environment)
+        .then((startedProcess) {
       process = startedProcess;
       // Used to notify when exiting, and as a return value on calls to
       // close().
       var doneCompleter = new Completer();
       done = doneCompleter.future;
 
       Completer stdoutDone = new Completer();
       Completer stderrDone = new Completer();
 
       process.stdout.transform(UTF8.decoder).listen((data) {
@@ skipping 185 matching lines @@
   String toString() => "Safari";
 
   // Delete the user specific browser cache and profile data.
   // Safari only have one per user, and you can't specify one by command line.
   static bool deleteCache = false;
 
 }
 
 
 class Chrome extends Browser {
-  static String _binary = _getBinary();
-
+  String _binary;

[kasperl, 2013/10/01 13:15:55]

final? You'll need to initialize it in the initializer list.

[kustermann, 2013/10/01 15:41:59]

I've tried:

  final String _binary;
  ...
  Chrome() : _binary = _getBinary();
  String _getBinary() { ... }

Results in

'file:///...../dart/tools/testing/dart/browser_controller.dart': Error: line 327
pos 24: illegal implicit access to receiver 'this'
  Chrome() : _binary = _getBinary();

I guess this only works if the method is static.

   String _version = "Version not found yet";
 
-  // This is extracted to a function since we may need to support several
-  // locations.
-  static String _getWindowsBinary() {
-    return "C:\\Program Files (x86)\\Google\\Chrome\\Application\\chrome.exe";
+  Chrome() {
+    _binary = _getBinary();
   }
 
-  static String _getBinary() {
+  String _getBinary() {
+    // This is extracted to a function since we may need to support several

[kasperl, 2013/10/01 13:15:55]

So this is just planning for future changes?

[kustermann, 2013/10/01 15:41:59]

Yeah, rico put this in.

It's not really necessary at the moment. I'll remove the function.

+    // locations.
+    String _getWindowsBinary() {
+      return "C:\\Program Files (x86)\\Google\\Chrome\\Application\\chrome.exe";
+    }
+
     if (Platform.isWindows) return _getWindowsBinary();
     if (Platform.isMacOS) {
       return "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome";
     }
     if (Platform.isLinux) return 'google-chrome';
   }
 
+  Map<String, String> _getEnvironment() => null;
+
   Future<bool> _getVersion() {
     if (Platform.isWindows) {
       // The version flag does not work on windows.
       // See issue:
       // https://code.google.com/p/chromium/issues/detail?id=158372
       // The registry hack does not seem to work.
       _version = "Can't get version on windows";
       // We still validate that the binary exists so that we can give good
       // feedback.
       return new File(_binary).exists().then((exists) {
@@ skipping 21 matching lines @@
     // Get the version and log that.
     return _getVersion().then((success) {
       if (!success) return false;
       _logEvent("Got version: $_version");
 
       return new Directory('').createTemp().then((userDir) {
         _cleanup = () { userDir.deleteSync(recursive: true); };
         var args = ["--user-data-dir=${userDir.path}", url,
                     "--disable-extensions", "--disable-popup-blocking",
                     "--bwsi", "--no-first-run"];
-        return startBrowser(_binary, args);
+        return startBrowser(_binary, args, environment: _getEnvironment());
       });
     }).catchError((e) {
       _logEvent("Running $_binary --version failed with $e");
       return false;
     });
   }
 
   String toString() => "Chrome";
 }
 
+class Dartium extends Chrome {
+  String _getBinary() {
+    if (Platform.operatingSystem == 'macos') {
+      return new Path('client/tests/dartium/Chromium.app/Contents/'
+                      'MacOS/Chromium').toNativePath();
+    }
+    return new Path('client/tests/dartium/chrome').toNativePath();
+  }
+
+  Map<String, String> _getEnvironment() {
+    var environment = new Map<String,String>.from(Platform.environment);
+    // By setting this environment variable, dartium will forward "print()"
+    // calls in dart to the top-level javascript function "dartPrint()" if
+    // available.
+    environment['DART_FORWARDING_PRINT'] = '1';
+    return environment;
+  }
+
+  String toString() => "Dartium";
+}
+
 class IE extends Browser {
 
   static const String binary =
       "c:\\Program Files\\Internet Explorer\\iexplore.exe";
 
   Future<String> getVersion() {
     var args = ["query",
                 "HKEY_LOCAL_MACHINE\\Software\\Microsoft\\Internet Explorer",
                 "/v",
                 "version"];
@@ skipping 214 matching lines @@
   List<BrowserTest> testQueue = new List<BrowserTest>();
   Map<String, BrowserTestingStatus> browserStatus =
       new Map<String, BrowserTestingStatus>();
 
   var adbDeviceMapping = new Map<String, AdbDevice>();
   // This cache is used to guarantee that we never see double reporting.
   // If we do we need to provide developers with this information.
   // We don't add urls to the cache until we have run it.
   Map<int, String> testCache = new Map<int, String>();
   List<int> doubleReportingTests = new List<int>();
+  Map<int, String> doubleReportingOutputs = new Map<int, String>();
 
   BrowserTestingServer testingServer;
 
   BrowserTestRunner(this.local_ip, this.browserName, this.maxNumBrowsers);
 
   Future<bool> start() {
     // If [browserName] doesn't support opening new windows, we use new iframes
     // instead.
     bool useIframe =
         !Browser.BROWSERS_WITH_WINDOW_SUPPORT.contains(browserName);
@@ skipping 56 matching lines @@
       browsersCompleter.complete(browsers);
     }
     return browsersCompleter.future;
   }
 
   var timedOut = [];
 
   void handleResults(String browserId, String output, int testId) {
     var status = browserStatus[browserId];
     if (testCache.containsKey(testId)) {
       doubleReportingTests.add(testId);

[kasperl, 2013/10/01 13:15:55]

Isn't the (linked) key set of the doubleReportingOutputs enough to keep track of
the doubleReportingTests? Why have two data structures for this?

[kustermann, 2013/10/01 15:41:59]

The keys in doubleReportingOutputs are enough, that's correct.

+      doubleReportingOutputs[testId] = output;
       return;
     }
 
     if (status == null || status.timeout) {
       // We don't do anything, this browser is currently being killed and
       // replaced. The browser here can be null if we decided to kill the
       // browser.
     } else if (status.currentTest != null) {
       status.currentTest.timeoutTimer.cancel();
       status.currentTest.stopwatch.stop();
@@ skipping 119 matching lines @@
   void queueTest(BrowserTest test) {
     testQueue.add(test);
   }
 
   void printDoubleReportingTests() {
     if (doubleReportingTests.length == 0) return;
     // TODO(ricow): die on double reporting.
     // Currently we just report this here, we could have a callback to the
     // encapsulating environment.
     print("");
     print("Double reporting tests");

[ricow1, 2013/10/02 18:17:47]

Do we want to remove the double reporting here when we print it to the debug log
below?

[kustermann, 2013/10/03 11:06:01]

For now I'd leave it, if you pass '--write-debug-log' you'll never see any
warning. (and if you don't pass it, you'll get tons of output -- nobody wants
that)

     for (var id in doubleReportingTests) {
       print("  ${testCache[id]}");
     }
+
+    DebugLogger.warning("Double reporting tests:");
+    for (var id in doubleReportingTests) {
+      DebugLogger.warning("  ${testCache[id]}, output: ");
+      DebugLogger.warning("${doubleReportingOutputs[id]}");
+      DebugLogger.warning("");
+      DebugLogger.warning("");
+    }

[kustermann, 2013/10/01 12:55:06]

I don't know if I should submit this, but we'd get valuable information on the
buildbot, why double reporting tests are broken.

[kasperl, 2013/10/01 13:15:55]

I'd submit it.

   }
 
   Future<bool> terminate() {
     var futures = [];
     underTermination = true;
     testingServer.underTermination = true;
     for (BrowserTestingStatus status in browserStatus.values) {
       futures.add(status.browser.close());
     }
     return Future.wait(futures).then((values) {
@@ skipping 324 matching lines @@
     Dart test driver, number of tests: <div id="number"></div><br>
     Currently executing: <div id="currently_executing"></div><br>
     Unhandled error: <div id="unhandled_error"></div>
     <iframe id="embedded_iframe"></iframe>
   </body>
 </html>
 """;
     return driverContent;
   }
 }