test.dart: Add timeout for browsers that are started but do not fetch tests.

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 613 matching lines @@
 
 
 class Firefox extends Browser {
   static const String enablePopUp =
       'user_pref("dom.disable_open_during_load", false);';
   static const String disableDefaultCheck =
       'user_pref("browser.shell.checkDefaultBrowser", false);';
   static const String disableScriptTimeLimit =
       'user_pref("dom.max_script_run_time", 0);';
 
-  Future _createPreferenceFile(var path) {
+  void _createPreferenceFile(var path) {
     var file = new File("${path.toString()}/user.js");
     var randomFile = file.openSync(mode: FileMode.WRITE);
     randomFile.writeStringSync(enablePopUp);
     randomFile.writeStringSync(disableDefaultCheck);
     randomFile.writeStringSync(disableScriptTimeLimit);
     randomFile.close();
   }
 
   Future<bool> start(String url) {
     _logEvent("Starting firefox browser on: $url");
@@ skipping 30 matching lines @@
  */
 class BrowserTestingStatus {
   Browser browser;
   BrowserTest currentTest;
 
   // This is currently not used for anything except for error reporting.
   // Given the usefulness of this in debugging issues this should not be
   // removed even when we have really stable system.
   BrowserTest lastTest;
   bool timeout = false;
+  Timer nextTestTimeout;
+
   BrowserTestingStatus(Browser this.browser);
 }
 
 
 /**
  * Describes a single test to be run int the browser.
  */
 class BrowserTest {
   // TODO(ricow): Add timeout callback instead of the string passing hack.
   Function doneCallback;
@@ skipping 34 matching lines @@
       this.browserOutput, {this.didTimeout: false});
 }
 
 /**
  * Encapsulates all the functionality for running tests in browsers.
  * The interface is rather simple. After starting, the runner tests
  * are simply added to the queue and a the supplied callbacks are called
  * whenever a test completes.
  */
 class BrowserTestRunner {
+  static const int MAX_NEXT_TEST_TIMEOUTS = 10;
+  static const Duration NEXT_TEST_TIMEOUT = const Duration(seconds: 60);
+
   final Map globalConfiguration;
   final bool checkedMode; // needed for dartium
 
   String localIp;
   String browserName;
   int maxNumBrowsers;
   // Used to send back logs from the browser (start, stop etc)
   Function logger;
   int browserIdCount = 0;
 
   bool underTermination = false;
+  int numBrowserGetTestTimeouts = 0;
 
   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>();
@@ skipping 26 matching lines @@
       testingServer.testDoneCallBack = handleResults;
       testingServer.testStatusUpdateCallBack = handleStatusUpdate;
       testingServer.testStartedCallBack = handleStarted;
       testingServer.nextTestCallBack = getNextTest;
       return getBrowsers().then((browsers) {
         var futures = [];
         for (var browser in browsers) {
           var url = testingServer.getDriverUrl(browser.id);
           var future = browser.start(url).then((success) {
             if (success) {
-              browserStatus[browser.id] = new BrowserTestingStatus(browser);
+              var status = new BrowserTestingStatus(browser);
+              browserStatus[browser.id] = status;
+              status.nextTestTimeout = createNextTestTimer(status);
             }
             return success;
           });
           futures.add(future);
         }
         return Future.wait(futures).then((values) {
           return !values.contains(false);
         });
       });
     });
@@ skipping 62 matching lines @@
 
       if (status.currentTest.id != testId) {
         print("Expected test id ${status.currentTest.id} for"
               "${status.currentTest.url}");
         print("Got test id ${testId}");
         print("Last test id was ${status.lastTest.id} for "
               "${status.currentTest.url}");
         throw("This should never happen, wrong test id");
       }
       testCache[testId] = status.currentTest.url;
-      Stopwatch watch = new Stopwatch()..start();
 
       // Report that the test is finished now
       var browserTestOutput = new BrowserTestOutput(
           status.currentTest.delayUntilTestStarted,
           status.currentTest.stopwatch.elapsed,
           output,
           status.browser.testBrowserOutput);
       status.currentTest.doneCallback(browserTestOutput);
 
-      watch.stop();
       status.lastTest = status.currentTest;
       status.currentTest = null;
+      status.nextTestTimeout = createNextTestTimer(status);
     } else {
       print("\nThis is bad, should never happen, handleResult no test");
       print("URL: ${status.lastTest.url}");
       print(output);
       terminate().then((_) {
         exit(1);
       });
     }
   }
 
@@ skipping 42 matching lines @@
           status.currentTest.delayUntilTestStarted,
           status.currentTest.stopwatch.elapsed,
           lastKnownMessage,
           status.browser.testBrowserOutput,
           didTimeout: true);
       status.currentTest.doneCallback(browserTestOutput);
       status.currentTest = null;
 
       // We don't want to start a new browser if we are terminating.
       if (underTermination) return;
-      var browser;
-      var new_id = id;
-      if (browserName == 'chromeOnAndroid') {
-        browser = new AndroidChrome(adbDeviceMapping[id]);
-      } else if (browserName == 'ContentShellOnAndroid') {
-        browser = new AndroidBrowser(adbDeviceMapping[id],
-                                     contentShellOnAndroidConfig,
-                                     checkedMode);
-      } else if (browserName == 'DartiumOnAndroid') {
-        browser = new AndroidBrowser(adbDeviceMapping[id],
-                                     dartiumOnAndroidConfig,
-                                     checkedMode);
-      } else {
-        browserStatus.remove(id);
-        browser = getInstance();
-        new_id = "BROWSER$browserIdCount";
-        browserIdCount++;
-        browserStatus[new_id] = new BrowserTestingStatus(browser);
+      restartBrowser(id);
+    });
+  }
+
+  void restartBrowser(String id) {
+    var browser;
+    var new_id = id;
+    if (browserName == 'chromeOnAndroid') {
+      browser = new AndroidChrome(adbDeviceMapping[id]);
+    } else if (browserName == 'ContentShellOnAndroid') {
+      browser = new AndroidBrowser(adbDeviceMapping[id],
+                                   contentShellOnAndroidConfig,
+                                   checkedMode);
+    } else if (browserName == 'DartiumOnAndroid') {
+      browser = new AndroidBrowser(adbDeviceMapping[id],
+                                   dartiumOnAndroidConfig,
+                                   checkedMode);
+    } else {
+      browserStatus.remove(id);
+      browser = getInstance();
+      new_id = "BROWSER$browserIdCount";
+      browserIdCount++;
+    }
+    browser.id = new_id;
+    var status = new BrowserTestingStatus(browser);
+    browserStatus[new_id] = status;
+    status.nextTestTimeout = createNextTestTimer(status);
+    browser.start(testingServer.getDriverUrl(new_id)).then((success) {
+      // We may have started terminating in the mean time.
+      if (underTermination) {
+        browser.close().then((success) {
+          if (status.nextTestTimeout != null) {
+            status.nextTestTimeout.cancel();
+            status.nextTestTimeout = null;
+          }

[kustermann, 2014/04/03 12:18:56]

You can move the cancelling before browser.close(), right? No need to wait until
it's dead.

+          // We should never hit this, print it out.
+          if (!success) {
+            print("Could not kill browser ($id) started due to timeout");
+          }
+        });
+        return;
       }
-      browser.id = new_id;
-      browser.start(testingServer.getDriverUrl(new_id)).then((success) {
-        // We may have started terminating in the mean time.
-        if (underTermination) {
-          browser.close().then((success) {
-            // We should never hit this, print it out.
-            if (!success) {
-              print("Could not kill browser ($id) started due to timeout");
-            }
-          });
-          return;
-        }
-        if (success) {
-          browserStatus[browser.id] = new BrowserTestingStatus(browser);
-        } else {
-          // TODO(ricow): Handle this better.
-          print("This is bad, should never happen, could not start browser");
-          exit(1);
-        }
-      });
+      if (!success) {
+        // TODO(ricow): Handle this better.
+        print("This is bad, should never happen, could not start browser");
+        exit(1);
+      }
     });
   }
 
   BrowserTest getNextTest(String browserId) {
-    if (testQueue.isEmpty) return null;
     var status = browserStatus[browserId];
     if (status == null) return null;
+    if (status.nextTestTimeout != null) {
+      status.nextTestTimeout.cancel();
+      status.nextTestTimeout = null;
+    }
+    if (testQueue.isEmpty) return null;
 
     // We are currently terminating this browser, don't start a new test.
     if (status.timeout) return null;
 
     BrowserTest test = testQueue.removeLast();
     if (status.currentTest == null) {
       status.currentTest = test;
       status.currentTest.lastKnownMessage = '';
     } else {
       // TODO(ricow): Handle this better.
@@ skipping 11 matching lines @@
     status.currentTest.stopwatch = new Stopwatch()..start();
 
     // Reset the test specific output information (stdout, stderr) on the
     // browser since a new test is begin started.
     status.browser.resetTestBrowserOutput();
 
     return test;
   }
 
   Timer createTimeoutTimer(BrowserTest test, BrowserTestingStatus status) {
-    return new Timer(
-        new Duration(seconds: test.timeout), () { handleTimeout(status); });
+    return new Timer(new Duration(seconds: test.timeout),
+                     () { handleTimeout(status); });
+  }
+
+  Timer createNextTestTimer(BrowserTestingStatus status) {
+    return new Timer(BrowserTestRunner.NEXT_TEST_TIMEOUT,
+                     () { handleNextTestTimeout(status); });
+  }
+
+  void handleNextTestTimeout(status) {
+    DebugLogger.warning(
+        "Browser timed out before getting next test. Restarting");
+    numBrowserGetTestTimeouts++;
+    if (numBrowserGetTestTimeouts >= MAX_NEXT_TEST_TIMEOUTS) {
+      DebugLogger.error(
+          "Too many browser timeouts before getting next test. Terminating");
+      terminate().then((_) => exit(1));
+    } else {
+      status.browser.close().then((_) => restartBrowser(status.browser.id));
+    }
   }
 
   void queueTest(BrowserTest test) {
     testQueue.add(test);
   }
 
   void printDoubleReportingTests() {
     if (doubleReportingOutputs.length == 0) return;
     // TODO(ricow): die on double reporting.
     // Currently we just report this here, we could have a callback to the
@@ skipping 12 matching lines @@
       DebugLogger.warning("");
     }
   }
 
   Future<bool> terminate() {
     var futures = [];
     underTermination = true;
     testingServer.underTermination = true;
     for (BrowserTestingStatus status in browserStatus.values) {
       futures.add(status.browser.close());
+      if (status.nextTestTimeout != null) {
+        status.nextTestTimeout.cancel();
+        status.nextTestTimeout = null;
+      }
     }
     return Future.wait(futures).then((values) {
       testingServer.httpServer.close();
       testingServer.errorReportingServer.close();
       printDoubleReportingTests();
       return !values.contains(false);
     });
   }
 
   Browser getInstance() {
@@ skipping 378 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;
   }
 }