Change Process.start to return a future that completes with a

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:io");
 #import("dart:isolate");
 #import("status_file_parser.dart");
 #import("test_progress.dart");
 #import("test_suite.dart");
 
 const int NO_TIMEOUT = 0;
 const int SLOW_TIMEOUT_MULTIPLIER = 4;
 
 typedef void TestCaseEvent(TestCase testCase);
 typedef void ExitCodeEvent(int exitCode);
-typedef bool EnqueueMoreWork(ProcessQueue queue);
+typedef void EnqueueMoreWork(ProcessQueue queue);
 
 /** A command executed as a step in a test case. */
 class Command {
   /** Path to the executable of this command. */
   String executable;
 
   /** Command line arguments to the executable. */
   List<String> arguments;
 
   /** The actual command line that will be executed. */
@@ skipping 566 matching lines @@
     }
   }
 
   /**
    * Process exit handler called at the end of every command. It internally
    * treats all but the last command as compilation steps. The last command is
    * the actual test and its output is analyzed in [testComplete].
    */
   void stepExitHandler(int exitCode) {
     process.close();
+    process = null;
     int totalSteps = testCase.commands.length;
     String suffix =' (step $currentStep of $totalSteps)';
-    if (currentStep == totalSteps) { // done with test command
+    if (timedOut) {
+      // Test timed out before it could complete.
+      testComplete(0, true);
+    } else if (currentStep == totalSteps) {
+      // Done with all test commands.
       testComplete(exitCode, false);
     } else if (exitCode != 0) {
+      // One of the steps failed.
       stderr.add('test.dart: Compilation failed$suffix, exit code $exitCode\n');
       testComplete(exitCode, true);
     } else {
+      // One compilation step successfully completed, move on to the
+      // next step.
       stderr.add('test.dart: Compilation finished $suffix\n');
       stdout.add('test.dart: Compilation finished $suffix\n');
       if (currentStep == totalSteps - 1 && testCase.usesWebDriver &&
           !testCase.configuration['noBatch']) {
         // Note: processQueue will always be non-null for runtime == ie, ff,
         // safari, chrome, opera. (It is only null for runtime == vm)
         // This RunningProcess object is done, and hands over control to
         // BatchRunner.startTest(), which handles reporting, etc.
         timeoutTimer.cancel();
         processQueue._getBatchRunner(testCase).startTest(testCase);
@@ skipping 19 matching lines @@
   void start() {
     Expect.isFalse(testCase.expectedOutcomes.contains(SKIP));
     stdout = new List<String>();
     stderr = new List<String>();
     currentStep = 0;
     startTime = new Date.now();
     runCommand(testCase.commands[currentStep++], stepExitHandler);
   }
 
   void runCommand(Command command, void exitHandler(int exitCode)) {
-    process = Process.start(command.executable, command.arguments);
-    process.onExit = exitHandler;
-    process.onError = (e) {
-      print("Error starting process:");
+    Future processFuture = Process.start(command.executable, command.arguments);
+    processFuture.then((Process p) {
+      process = p;
+      process.onExit = exitHandler;
+      var stdoutStringStream = new StringInputStream(process.stdout);
+      var stderrStringStream = new StringInputStream(process.stderr);
+      stdoutStringStream.onLine =
+          makeReadHandler(stdoutStringStream, stdout);
+      stderrStringStream.onLine =
+          makeReadHandler(stderrStringStream, stderr);
+      if (timeoutTimer == null) {
+        // Create one timeout timer when starting test case, remove it at end.
+        timeoutTimer = new Timer(1000 * testCase.timeout, timeoutHandler);
+      }
+      // If the timeout fired in between two commands, kill the just
+      // started process immediately.
+      if (timedOut) process.kill();
+    });
+    processFuture.handleException((e) {
+      print("Process error:");
       print("  Command: $command");
       print("  Error: $e");
       testComplete(-1, false);
-    };
-    InputStream stdoutStream = process.stdout;
-    InputStream stderrStream = process.stderr;
-    StringInputStream stdoutStringStream = new StringInputStream(stdoutStream);
-    StringInputStream stderrStringStream = new StringInputStream(stderrStream);
-    stdoutStringStream.onLine =
-        makeReadHandler(stdoutStringStream, stdout);
-    stderrStringStream.onLine =
-        makeReadHandler(stderrStringStream, stderr);
-    if (timeoutTimer == null) {
-      // Create one timeout timer when starting test case, remove it at end.
-      timeoutTimer = new Timer(1000 * testCase.timeout, timeoutHandler);
-    }
+      return true;
+    });
   }
 
   void timeoutHandler(Timer unusedTimer) {
     timedOut = true;
-    process.kill();
+    if (process != null) process.kill();
   }
 }
 
 /**
  * This class holds a value, that can be changed.  It is used when
  * closures need a shared value, that they can all change and read.
  */
 class MutableValue<T> {
   MutableValue(T this.value);
   T value;
@@ skipping 200 matching lines @@
           line = _stdoutStream.readLine();
         }
         line = _stderrStream.readLine();
         while (line != null) {
           _testStderr.add(line);
           line = _stderrStream.readLine();
         }
         _stderrDrained = true;
         _stdoutDrained = true;
         _process.close();
-        _startProcess(() { _reportResult(); });
+        _startProcess(_reportResult);
       } else {  // No active test case running.
         _process.close();
         _process = null;
       }
     }
     return handler;
   }
 
   void _timeoutHandler(ignore) {
     _process.onExit = makeExitHandler(">>> TEST TIMEOUT");
     _process.kill();
   }
 
-  void _startProcess(then) {
-    _process = Process.start(_executable, _batchArguments);
-    _stdoutStream = new StringInputStream(_process.stdout);
-    _stderrStream = new StringInputStream(_process.stderr);
-    _process.onExit = makeExitHandler(">>> TEST CRASH");
-    _process.onError = (e) {
-      print("Error starting process:");
+  _startProcess(callback) {
+    Future processFuture = Process.start(_executable, _batchArguments);
+    processFuture.then((Process p) {
+      _process = p;
+      _stdoutStream = new StringInputStream(_process.stdout);
+      _stderrStream = new StringInputStream(_process.stderr);
+      _process.onExit = makeExitHandler(">>> TEST CRASH");
+      callback();
+    });
+    processFuture.handleException((e) {
+      print("Process error:");
       print("  Command: $_executable ${Strings.join(_batchArguments, ' ')}");
       print("  Error: $e");
       // If there is an error starting a batch process, chances are that
       // it will always fail. So rather than re-trying a 1000+ times, we
       // exit.
       exit(1);
-    };
-    _process.onStart = then;
+      return true;
+    });
   }
 }
 
 /**
  * ProcessQueue is the master control class, responsible for running all
  * the tests in all the TestSuites that have been registered.  It includes
  * a rate-limited queue to run a limited number of tests in parallel,
  * a ProgressIndicator which prints output when tests are started and
  * and completed, and a summary report when all tests are completed,
  * and counters to determine when all of the tests in all of the test suites
@@ skipping 128 matching lines @@
     if (!_isSeleniumAvailable && !_startingServer) {
       _startingServer = true;
 
       // Check to see if the jar was already running before the program started.
       String cmd = 'ps';
       var arg = ['aux'];
       if (Platform.operatingSystem == 'windows') {
         cmd = 'tasklist';
         arg.add('/v');
       }
-      Process p = Process.start(cmd, arg);
-      final StringInputStream stdoutStringStream =
-          new StringInputStream(p.stdout);
-      p.onError = (e) {
+
+      Future processFuture = Process.start(cmd, arg);
+      processFuture.then((Process p) {
+        final StringInputStream stdoutStringStream =
+            new StringInputStream(p.stdout);
+        stdoutStringStream.onLine = () {
+          var line = stdoutStringStream.readLine();
+          while (null != line) {
+            var regexp = const RegExp(r".*selenium-server-standalone.*");
+            if (regexp.hasMatch(line)) {
+              _seleniumAlreadyRunning = true;
+              resumeTesting();
+            }
+            line = stdoutStringStream.readLine();
+          }
+          if (!_isSeleniumAvailable) {
+            _startSeleniumServer();
+          }
+        };
+      });
+      processFuture.handleException((e) {
         print("Error starting process:");
         print("  Command: $cmd ${Strings.join(arg, ' ')}");
         print("  Error: $e");
         // TODO(ahe): How to report this as a test failure?
         exit(1);
-      };
-      stdoutStringStream.onLine = () {
-        var line = stdoutStringStream.readLine();
-        while (null != line) {
-          if (const RegExp(r".*selenium-server-standalone.*").hasMatch(line)) {
-            _seleniumAlreadyRunning = true;
-            resumeTesting();
-          }
-          line = stdoutStringStream.readLine();
-        }
-        if (!_isSeleniumAvailable) {
-          _startSeleniumServer();
-        }
-      };
+        return true;
+      });
     }
   }
 
   void _runTest(TestCase test) {
     if (test.usesWebDriver) {
       browserUsed = test.configuration['browser'];
       if (_needsSelenium) _ensureSeleniumServerRunning();
     }
     _progress.testAdded();
     _tests.add(test);
@@ skipping 28 matching lines @@
   void _startSeleniumServer() {
     // Get the absolute path to the Selenium jar.
     String filePath = TestUtils.testScriptPath;
     String pathSep = Platform.pathSeparator;
     int index = filePath.lastIndexOf(pathSep);
     filePath = '${filePath.substring(0, index)}${pathSep}testing${pathSep}';
     var lister = new Directory(filePath).list();
     lister.onFile = (String file) {
       if (const RegExp(r"selenium-server-standalone-.*\.jar").hasMatch(file)
           && _seleniumServer == null) {
-        _seleniumServer = Process.start('java', ['-jar', file]);
-        _seleniumServer.onError = (e) {
-          print("Error starting process:");
+        Future processFuture = Process.start('java', ['-jar', file]);
+        processFuture.then((Process server) {
+          _seleniumServer = server;
+          // Heads up: there seems to an obscure data race of some form in
+          // the VM between launching the server process and launching the test
+          // tasks that disappears when you read IO (which is convenient, since
+          // that is our condition for knowing that the server is ready).
+          StringInputStream stdoutStringStream =
+              new StringInputStream(_seleniumServer.stdout);
+          StringInputStream stderrStringStream =
+              new StringInputStream(_seleniumServer.stderr);
+          stdoutStringStream.onLine =
+              makeSeleniumServerHandler(stdoutStringStream);
+          stderrStringStream.onLine =
+              makeSeleniumServerHandler(stderrStringStream);
+        });
+        processFuture.handleException((e) {
+          print("Process error:");
           print("  Command: java -jar $file");
           print("  Error: $e");
           // TODO(ahe): How to report this as a test failure?
           exit(1);
-        };
-        // Heads up: there seems to an obscure data race of some form in
-        // the VM between launching the server process and launching the test
-        // tasks that disappears when you read IO (which is convenient, since
-        // that is our condition for knowing that the server is ready).
-        StringInputStream stdoutStringStream =
-            new StringInputStream(_seleniumServer.stdout);
-        StringInputStream stderrStringStream =
-            new StringInputStream(_seleniumServer.stderr);
-        stdoutStringStream.onLine =
-            makeSeleniumServerHandler(stdoutStringStream);
-        stderrStringStream.onLine =
-            makeSeleniumServerHandler(stderrStringStream);
+          return true;
+        });
       }
     };
   }
 
   Future _terminateBatchRunners() {
     var futures = new List();
     for (var runners in _batchProcesses.getValues()) {
       for (var runner in runners) {
         futures.add(runner.terminate());
       }
@@ skipping 65 matching lines @@
         // 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 > _progress.numFailedTests;
         new RunningProcess(test, allowRetry, this).start();
       }
       _numProcesses++;
     }
   }
 }