Add browser controller and allow it to be used under a flag.

tools/testing/dart/test_runner.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 executing tests.
  *
  * This module includes:
  * - Managing parallel execution of tests, including timeout checks.
  * - Evaluating the output of each test as pass/fail/crash/timeout.
  */
 library test_runner;
 
 import "dart:async";
 import "dart:collection" show Queue;
 // We need to use the 'io' prefix here, otherwise io.exitCode will shadow
 // CommandOutput.exitCode in subclasses of CommandOutput.
 import "dart:io" as io;
 import "dart:isolate";
 import "dart:uri";
+import "browser_controler.dart";

[kustermann, 2013/05/13 16:00:00]

browser_controler -> browser_controller + file rename.

[ricow1, 2013/05/14 07:20:58]

Done.

 import "http_server.dart" as http_server;
 import "status_file_parser.dart";
 import "test_progress.dart";
 import "test_suite.dart";
 import "utils.dart";
 
 const int NO_TIMEOUT = 0;
 const int SLOW_TIMEOUT_MULTIPLIER = 4;
 
 const int CRASHING_BROWSER_EXITCODE = -10;
@@ skipping 310 matching lines @@
     final arch = configuration['arch'];
     final checked = configuration['checked'] ? '-checked' : '';
     return "$compiler-$runtime$checked ${mode}_$arch";
   }
 
   List<String> get batchRunnerArguments => ['-batch'];
   List<String> get batchTestArguments => commands.last.arguments;
 
   bool get usesWebDriver => TestUtils.usesWebDriver(configuration['runtime']);
 
+  bool get usesBrowserControler => configuration['use_browser_controller'];

[kustermann, 2013/05/13 16:00:00]

Controler -> Controller

[ricow1, 2013/05/14 07:20:58]

Done.

+
   void completed() { completedHandler(this); }
 
   bool get isFlaky {
       if (expectedOutcomes.contains(SKIP)) {
         return false;
       }
 
       var flags = new Set.from(expectedOutcomes);
       flags..remove(TIMEOUT)
            ..remove(SLOW);
@@ skipping 158 matching lines @@
   }
   factory CommandOutputImpl.fromCase(TestCase testCase,
                                      Command command,
                                      int exitCode,
                                      bool incomplete,
                                      bool timedOut,
                                      List<int> stdout,
                                      List<int> stderr,
                                      Duration time,
                                      bool compilationSkipped) {
-    if (testCase is BrowserTestCase) {
+    if (testCase.usesBrowserControler) {
+      return new HTMLBrowserCommandOutputImpl(testCase,
+                                              command,
+                                              exitCode,
+                                              incomplete,
+                                              timedOut,
+                                              stdout,
+                                              stderr,
+                                              time,
+                                              compilationSkipped);
+    } else if (testCase is BrowserTestCase) {
       return new BrowserCommandOutputImpl(testCase,
                                           command,
                                           exitCode,
                                           incomplete,
                                           timedOut,
                                           stdout,
                                           stderr,
                                           time,
                                           compilationSkipped);
     } else if (testCase.configuration['analyzer']) {
@@ skipping 199 matching lines @@
               return (exitCode != 0 && !hasCrashed);
             }
           }
           break;
       }
     }
     return true;
   }
 }
 
+class HTMLBrowserCommandOutputImpl extends BrowserCommandOutputImpl {
+ HTMLBrowserCommandOutputImpl(
+      testCase,
+      command,
+      exitCode,
+      incomplete,
+      timedOut,
+      stdout,
+      stderr,
+      time,
+      compilationSkipped) :
+    super(testCase,
+          command,
+          exitCode,
+          incomplete,
+          timedOut,
+          stdout,
+          stderr,
+          time,
+          compilationSkipped);
+
+  bool get _browserTestFailure {
+    // We should not need to convert back and forward.
+    var output = decodeUtf8(super.stdout);
+    if (output.contains("FAIL")) return true;
+    return !output.contains("PASS");
+  }
+}
+
+
 // The static analyzer does not actually execute code, so
 // the criteria for success now depend on the text sent
 // to stderr.
 class AnalysisCommandOutputImpl extends CommandOutputImpl {
   // An error line has 8 fields that look like:
   // ERROR|COMPILER|MISSING_SOURCE|file:/tmp/t.dart|15|1|24|Missing source.
   final int ERROR_LEVEL = 0;
   final int ERROR_TYPE = 1;
   final int FORMATTED_ERROR = 7;
 
@@ skipping 529 matching lines @@
   List<EventListener> _eventListener;
 
   // For dartc/selenium batch processing we keep a list of batch processes.
   Map<String, List<BatchRunnerProcess>> _batchProcesses;
 
   // Cache information about test cases per test suite. For multiple
   // configurations there is no need to repeatedly search the file
   // system, generate tests, and search test files for options.
   Map<String, List<TestInformation>> _testCache;
 
+  Map<String, BrowserTestRunner> _browserTestRunners;
+
   /**
    * String indicating the browser used to run the tests. Empty if no browser
    * used.
    */
   String browserUsed = '';
 
   /**
    * Process running the selenium server .jar (only used for Safari and Opera
    * tests.)
    */
@@ skipping 10 matching lines @@
                DateTime startTime,
                testSuites,
                this._eventListener,
                this._allDone,
                [bool verbose = false,
                 bool listTests = false])
       : _verbose = verbose,
         _listTests = listTests,
         _tests = new Queue<TestCase>(),
         _batchProcesses = new Map<String, List<BatchRunnerProcess>>(),
-        _testCache = new Map<String, List<TestInformation>>() {
+        _testCache = new Map<String, List<TestInformation>>(),
+        _browserTestRunners = new Map<String, BrowserTestRunner>(){

[kustermann, 2013/05/13 16:00:00]

space before {

[ricow1, 2013/05/14 07:20:58]

Done.

     _runTests(testSuites);
   }
 
   /**
    * Perform any cleanup needed once all tests in a TestSuite have completed
    * and notify our progress indicator that we are done.
    */
   void _cleanupAndMarkDone() {
     _allDone();
     if (browserUsed != '' && _seleniumServer != null) {
       _seleniumServer.kill();
     }
     eventAllTestsDone();
   }
 
   void _checkDone() {
     if (_allTestsWereEnqueued && _tests.isEmpty && _numProcesses == 0) {
-      _terminateBatchRunners().then((_) => _cleanupAndMarkDone());
+      _terminateBatchRunners().then((_) {
+        _terminateBrowserRunners().then((_) => _cleanupAndMarkDone());
+      });
     }
   }
 
   void _runTests(List<TestSuite> testSuites) {
     // FIXME: For some reason we cannot call this method on all test suites
     // in parallel.
     // If we do, not all tests get enqueued (if --arch=all was specified,
     // we don't get twice the number of tests [tested on -rvm -cnone])
     // Issue: 7927
     Iterator<TestSuite> iterator = testSuites.iterator;
@@ skipping 142 matching lines @@
     var futures = new List();
     for (var runners in _batchProcesses.values) {
       for (var runner in runners) {
         futures.add(runner.terminate());
       }
     }
     // Change to Future.wait when updating binaries.
     return Future.wait(futures);
   }
 
+  Future _terminateBrowserRunners() {
+    var futures = [];
+    for (BrowserTestRunner runner in _browserTestRunners.values) {
+      futures.add(runner.terminate());
+    }
+    return Future.wait(futures);
+  }
+
   BatchRunnerProcess _getBatchRunner(TestCase test) {
     // Start batch processes if needed
     var compiler = test.configuration['compiler'];
     var runners = _batchProcesses[compiler];
     if (runners == null) {
       runners = new List<BatchRunnerProcess>(_maxProcesses);
       for (int i = 0; i < _maxProcesses; i++) {
         runners[i] = new BatchRunnerProcess(test);
       }
       _batchProcesses[compiler] = runners;
     }
 
     for (var runner in runners) {
       if (!runner.active) return runner;
     }
     throw new Exception('Unable to find inactive batch runner.');
   }
 
+  Future<BrowserTestRunner> getBrowserTestRunner(TestCase test) {

[kustermann, 2013/05/13 16:00:00]

You could make it private as well.

[ricow1, 2013/05/14 07:20:58]

Done.

+    var runtime = test.configuration['runtime'];
+    if (_browserTestRunners[runtime] == null) {
+      var testRunner = new BrowserTestRunner(runtime, 4);

[kustermann, 2013/05/13 16:00:00]

Make a constant.

[ricow1, 2013/05/14 07:20:58]

Done.

+      _browserTestRunners[runtime] = testRunner;
+      return testRunner.start().then((started) {
+        if (started) {
+          return testRunner;
+        }
+        return null;

[kustermann, 2013/05/13 16:00:00]

If we're unable to start the browser then we'll get a null pointer exception
further down. Maybe we should just exit() and print a appropriate error message.

[ricow1, 2013/05/14 07:20:58]

Done.

[ricow1, 2013/05/14 07:20:58]

Done 

+      });
+    }
+    return new Future.immediate(_browserTestRunners[runtime]);
+  }
+
+  void startBrowserControllerTest(var test) {

[kustermann, 2013/05/13 16:00:00]

private?

[ricow1, 2013/05/14 07:20:58]

Done.

+    // Get the url.
+    // TODO(ricow): This is not needed when we have eliminated selenium.
+    var nextCommandIndex = test.commandOutputs.keys.length;
+    var url = test.commands[nextCommandIndex].toString().split("--out=")[1];
+    // Remove trailing "
+    url = url.split('"')[0];
+    var callback = (var output) {
+      new CommandOutput.fromCase(test,
+                                 test.commands[nextCommandIndex],
+                                 0,
+                                 false,
+                                 output == "TIMEOUT",
+                                 output.codeUnits,

[kustermann, 2013/05/13 16:00:00]

Please use encodeUtf8String or so. This is supposed to be a list of bytes not a
list of unicode points.

[ricow1, 2013/05/14 07:20:58]

Done.

+                                 [],
+                                 const Duration(seconds: 1),
+                                 false);
+      test.completedHandler(test);
+    };
+    BrowserTest browserTest = new BrowserTest(url,
+                                              callback,
+                                              test.timeout);

[kustermann, 2013/05/13 16:00:00]

Does this fit on one line?

[ricow1, 2013/05/14 07:20:58]

Yes

+    getBrowserTestRunner(test).then((testRunner) {
+      testRunner.queueTest(browserTest);
+    });
+  }
+
   void _tryRunTest() {
     _checkDone();
+    // We oversubscribe the cpus with the started browsers when we use our

[kustermann, 2013/05/13 16:00:00]

This comment seems to be wrong.

+    // internal browser driver. We use a simpler queue based setup.
+    // TODO(ricow): remove most of the hacked selenium code below when
+    // we have eliminated the need.
+
     if (_numProcesses < _maxProcesses && !_tests.isEmpty) {
       TestCase test = _tests.removeFirst();
       if (_listTests) {
         var fields = [test.displayName,
                       test.expectedOutcomes.join(','),
                       test.isNegative.toString()];
         fields.addAll(test.commands.last.arguments);
         print(fields.join('\t'));
         return;
       }
-      if (test.usesWebDriver && _needsSelenium && !_isSeleniumAvailable || (test
-          is BrowserTestCase && test.waitingForOtherTest)) {
+      if (test.usesWebDriver && _needsSelenium && !_isSeleniumAvailable || 
+          (test is BrowserTestCase && test.waitingForOtherTest)) {
         // The test is not yet ready to run. Put the test back in
         // the queue.  Avoid spin-polling by using a timeout.
         _tests.add(test);
         new Timer(new Duration(milliseconds: 100),
                   _tryRunTest);  // Don't lose a process.
         return;
       }
       // Before running any commands, we print out all commands if '--verbose'
       // was specified.
       if (_verbose && test.commandOutputs.length == 0) {
@@ skipping 40 matching lines @@
           _numProcesses--;
           if (isBrowserCommand) {
             _numBrowserProcesses--;
           }
           eventFinishedTestCase(test_arg);
           if (test_arg is BrowserTestCase) test_arg.notifyObservers();
           oldCallback(test_arg);
           _tryRunTest();
         };
         test.completedHandler = testCompleted;
-        _getBatchRunner(test).startTest(test);
+
+        if (test.usesBrowserControler) {

[kustermann, 2013/05/13 16:00:00]

I'm not sure if we should make this an attribute of TestCase. It seems to me
that 'usesBrowserControler' is a configuration wide setting. Maybe we should
move this attribute to the configuration?

[ricow1, 2013/05/14 07:20:58]

It is part of the configuration, see how we return it from the test case. I do,
however, think that this provides the best flexibility, we could change the way
we do this right?
Additionally, this is consistent with how we do it for other stuff
(usesWebDriver etc)

+          startBrowserControllerTest(test);
+        } else {
+          _getBatchRunner(test).startTest(test);
+        }
       } else {
         // Once we've actually failed a test, technically, we wouldn't need to
         // bother retrying any subsequent tests since the bot is already red.
         // However, we continue to retry tests until we have actually failed
         // four tests (arbitrarily chosen) for more debugable output, so that
         // the developer doesn't waste his or her time trying to fix a bunch of
         // tests that appear to be broken but were actually just flakes that
         // didn't get retried because there had already been one failure.
         bool allowRetry = _MAX_FAILED_NO_RETRY > _numFailedTests;
         runNextCommandWithRetries(test, allowRetry).then((TestCase testCase) {
@@ skipping 125 matching lines @@
     }
   }
 
   void eventAllTestsDone() {
     for (var listener in _eventListener) {
       listener.allDone();
     }
   }
 }