Force close of browser Windows when complete.

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 492 matching lines @@
   Function _enqueueMoreWork;
   Queue<TestCase> _tests;
   ProgressIndicator _progress;
   String _temporaryDirectory;
   // For dartc batch processing we keep a list of batch processes.
   List<DartcBatchRunnerProcess> _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;
+  /**
+   * String indicating the browser used to run the tests. Empty if no browser 
+   * used.
+   */
+  String browserUsed;
 
   ProcessQueue(int this._maxProcesses,
                String progress,
                Date startTime,
                bool printTiming,
                Function this._enqueueMoreWork,
                [bool this._verbose = false,
                 bool this._listTests = false,
                 bool this._keepGeneratedTests = false])
       : _tests = new Queue<TestCase>(),
         _progress = new ProgressIndicator.fromName(progress,
                                                    startTime,
                                                    printTiming),
         _batchProcesses = new List<DartcBatchRunnerProcess>(),
         _testCache = new Map<String, List<TestInformation>>() {
     if (!_enqueueMoreWork(this)) _progress.allDone();
+    browserUsed = '';
   }
 
   /**
    * Registers a TestSuite so that all of its tests will be run.
    */
   void addTestSuite(TestSuite testSuite) {
     _activeTestListers++;
     testSuite.forEachTest(_runTest, _testCache, globalTemporaryDirectory,
                           _testListerDone);
+    testSuite.notifyIfBrowserCleanupNeeded(this);

[Jennifer Messerly, 2012/02/14 02:17:25]

an alternate idea: instead of changing test_suite, could you change _runTest in
this class?

From there you can access test.configuration, which should give you:

if (test is BrowserTestCase) { // or test.configuration['component'] ==
'webdriver'
  browserUsed = test.configuration['browser'];
}

[Emily Fortuna, 2012/02/14 23:55:44]

Done.

   }
 
   void _testListerDone() {
     _activeTestListers--;
     _checkDone();
   }
 
   String globalTemporaryDirectory() {
     if (_temporaryDirectory != null) return _temporaryDirectory;
 
     if (new Platform().operatingSystem() == 'windows') {
       throw new Exception(
           'Test suite requires temporary directory. Not supported on Windows.');
     }
     var tempDir = new Directory('');
     tempDir.createTempSync();
     _temporaryDirectory = tempDir.path;
     return _temporaryDirectory;
   }
 
+  /**
+   * Sometimes Webdriver doesn't close every browser window when it's done 
+   * with a test. At the end of all tests we clear out any neglected processes
+   * that are still running. 
+   */
+  void killZombieBrowsers() {
+    String chromeName = 'chrome';
+    if (new Platform().operatingSystem() == 'macos') {
+      chromeName = 'Google\ Chrome';
+    }
+    Map<String, List<String>> processNames = {'ie': ['iexplore'], 'safari': 
+        ['Safari'], 'ff': ['firefox'], 'chrome': ['chromedriver', chromeName]};
+    if (new Platform().operatingSystem() == 'windows') {
+      for (String name in processNames[browserUsed]) {
+        Process process = new Process.start(
+            'C:\\Windows\\System32\\taskkill.exe', ['/F', '/IM', name + '.exe',
+            '/T']);
+        process.exitHandler = (exitCode) {
+          process.close();
+          _progress.allDone();

[Jennifer Messerly, 2012/02/14 02:17:25]

is it okay that we're potentially calling allDone here multiple times? in other
words, if chromedriver and chrome are being killed, each one will call
_progress.allDone()

[Bill Hesse, 2012/02/14 12:46:00]

I don't think this is OK.

+        };
+        process.errorHandler = (error) {
+          _progress.allDone();
+        };
+      }
+    } else {
+      for (String name in processNames[browserUsed]) {
+        Process process = new Process.start('killall', ['-9', name]);
+        process.exitHandler = (exitCode) {
+          process.close();
+          _progress.allDone();
+        };
+        process.errorHandler = (error) {
+          _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)) {
       _progress.allTestsKnown();
       if (_tests.isEmpty() && _numProcesses == 0) {
         _terminateDartcBatchRunners();
         if (_keepGeneratedTests || _temporaryDirectory == null) {
-          _progress.allDone();
+          if (browserUsed != '') {
+            killZombieBrowsers();
+          } else {
+            _progress.allDone();
+          }

[Jennifer Messerly, 2012/02/14 02:17:25]

it'd be good to pull this pattern out into a function

[Emily Fortuna, 2012/02/14 23:55:44]

Done.

         } else if (!_temporaryDirectory.startsWith('/tmp/') ||
                    _temporaryDirectory.contains('/../')) {
           // Let's be extra careful, since rm -rf is so dangerous.
           print('Temporary directory $_temporaryDirectory unsafe to delete!');
-          _progress.allDone();
+          if (browserUsed != '') {
+            killZombieBrowsers();
+          } else {
+            _progress.allDone();
+          }
         } else {
           // TODO(dart:1211): Use delete(recursive=true) in Dart when it is
           // implemented, and add Windows support.
           var deletion =
               new Process.start('/bin/rm', ['-rf', _temporaryDirectory]);
           deletion.exitHandler = (int exitCode) {
             if (exitCode == 0) {
               if (!_listTests) {  // Output of --list option is used by scripts.
                 print('\nTemporary directory $_temporaryDirectory deleted.');
               }
             } else {
               print('\nDeletion of temp dir $_temporaryDirectory failed.');
             }
-            _progress.allDone();
+            if (browserUsed != '') {
+              killZombieBrowsers();
+            } else {
+              _progress.allDone();
+            }
           };
         }
       }
     }
   }
 
   void _runTest(TestCase test) {
     _progress.testAdded();
     _tests.add(test);
     _tryRunTest();
@@ skipping 45 matching lines @@
           test.displayName != 'dartc/junit_tests') {
         _ensureDartcBatchRunnersStarted(test.executablePath);
         _getDartcBatchRunnerProcess().startTest(test);
       } else {
         new RunningProcess(test).start();
       }
       _numProcesses++;
     }
   }
 }