Fix issues with test.dart that appeared on Mac.

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.
@@ skipping 729 matching lines @@
         _startProcess(() {
           doStartTest(testCase);
         });
       };
       _process.kill();
     } else {
       doStartTest(testCase);
     }
   }
 
-  void terminate() {
-    if (_process !== null) {
-      bool closed = false;
-      _process.onExit = (exitCode) {
-        closed = true;
-        _process.close();
-      };
-      if (_isWebDriver) {

[Emily Fortuna, 2012/10/06 00:11:55]

DBC -- why are no longer doing this graceful shutdown for the browser tests?
This can result in 100s of browser windows open on test failures.

[Mads Ager (google), 2012/10/06 06:24:09]

Because the TODO has been fixed and a kill sends the right signal to do a
graceful shutdown. Additionally, this code makes test.dart hang for 30 seconds
no matter if the processes are dead or not because we have to wait for the 30
second timeout to fire.

-        // Use a graceful shutdown so our Selenium script can close
-        // the open browser processes. TODO(jmesserly): Send a signal once
-        // that's supported, see dartbug.com/1756.
-        _process.stdin.write('--terminate\n'.charCodes());
-
-        // In case the run_selenium process didn't close, kill it after 30s
-        int shutdownMillisecs = 30000;
-        new Timer(shutdownMillisecs, (e) { if (!closed) _process.kill(); });
-      } else {
-        _process.kill();
-      }
-    }
+  Future terminate() {
+    if (_process == null) return new Future.immediate(true);
+    Completer completer = new Completer();
+    _process.onExit = (exitCode) {
+      _process.close();
+      completer.complete(true);
+    };
+    _process.kill();
+    return completer.future;
   }
 
   void doStartTest(TestCase testCase) {
     _startTime = new Date.now();
     _testStdout = [];
     _testStderr = [];
     _status = null;
     _stdoutDrained = false;
     _stderrDrained = false;
     _ignoreStreams = new MutableValue<bool>(false);  // Captured by closures.
@@ skipping 234 matching lines @@
   void _testListerDone() {
     _activeTestListers--;
     _checkDone();
   }
 
   /**
    * Perform any cleanup needed once all tests in a TestSuite have completed
    * and notify our progress indicator that we are done.
    */
   void _cleanupAndMarkDone() {
-    // _progress.allDone() exits the process, so we have to call the
-    // _allDone callback before.
     _allDone();
     if (browserUsed != '' && _seleniumServer != null) {
       _seleniumServer.kill();
     } else {
       _progress.allDone();
     }
   }
 
   void _checkDone() {
     // When there are no more active test listers ask for more work
     // from process queue users.
     if (_activeTestListers == 0) {
      _enqueueMoreWork(this);
     }
     // If there is still no work, we are done.
     if (_activeTestListers == 0) {
       _progress.allTestsKnown();
       if (_tests.isEmpty() && _numProcesses == 0) {
-        _terminateBatchRunners();
-        _cleanupAndMarkDone();
+        _terminateBatchRunners().then((_) => _cleanupAndMarkDone());
       }
     }
   }
 
   /**
    * True if we are using a browser + platform combination that needs the
    * Selenium server jar.
    */
   bool get _needsSelenium => Platform.operatingSystem == 'macos' &&
       browserUsed == 'safari';
@@ skipping 110 matching lines @@
         StringInputStream stderrStringStream =
             new StringInputStream(_seleniumServer.stderr);
         stdoutStringStream.onLine =
             makeSeleniumServerHandler(stdoutStringStream);
         stderrStringStream.onLine =
             makeSeleniumServerHandler(stderrStringStream);
       }
     };
   }
 
-  void _terminateBatchRunners() {
+  Future _terminateBatchRunners() {
+    var futures = new List();
     for (var runners in _batchProcesses.getValues()) {
       for (var runner in runners) {
-        runner.terminate();
+        futures.add(runner.terminate());
       }
     }
+    return Futures.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);
@@ skipping 53 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++;
     }
   }
 }