Revision: 5482
Author:   [email protected]
Date:     Wed Jul 10 16:44:43 2013
Log:      limit number of failed browser test windows kept
https://codereview.appspot.com/11136043

I verified this does what I expect by running tests with
jsunit.pass = jsunit.fail;

R=kpreid2


http://code.google.com/p/google-caja/source/detail?r=5482

Modified:
 /trunk/tests/com/google/caja/plugin/BrowserTestCase.java
 /trunk/tests/com/google/caja/plugin/WebDriverHandle.java

=======================================
--- /trunk/tests/com/google/caja/plugin/BrowserTestCase.java Wed Jun 12 12:50:57 2013 +++ /trunk/tests/com/google/caja/plugin/BrowserTestCase.java Wed Jul 10 16:44:43 2013
@@ -153,7 +153,7 @@
       if (TestFlag.DEBUG_SERVER.truthy()) {
         Thread.currentThread().join();
       }
-      WebDriver driver = wdh.makeWindow();
+      WebDriver driver = wdh.begin();
       driver.get(page);
       if (TestFlag.DEBUG_BROWSER.truthy()) {
         Thread.currentThread().join();
@@ -164,12 +164,7 @@
     } finally {
       wdh.captureResults(label);
       localServer.stop();
-      // It's helpful for debugging to keep failed windows open.
-      if (!passed && !isKnownFailure && !TestFlag.BROWSER_CLOSE.truthy()) {
-        wdh.keepOpen();
-      } else {
-        wdh.closeWindow();
-      }
+      wdh.end(passed || isKnownFailure);
     }
     return result;
   }
=======================================
--- /trunk/tests/com/google/caja/plugin/WebDriverHandle.java Wed Jul 10 15:43:42 2013 +++ /trunk/tests/com/google/caja/plugin/WebDriverHandle.java Wed Jul 10 16:44:43 2013
@@ -50,7 +50,12 @@
   private RemoteWebDriver driver = null;
   private boolean canExecuteScript = true;

-  WebDriver makeWindow() {
+  // Don't keep more than this many failed test windows. (Otherwise
+  // a broken tree can overload a machine with browser windows.)
+  private static final int MAX_FAILS_KEPT = 9;
+  private static int fails_kept = 0;
+
+  WebDriver begin() {
     if (driver == null) {
       driver = makeDriver();
       reportVersion(driver);
@@ -111,19 +116,17 @@
     err.println(s);
   }

-  // makeWindow used to open a new window in an existing session, but
-  // there's no way to do that if executeScript() doesn't work. So now
-  // we just re-use the same window.
-  void closeWindow() {
+  void end(boolean passed) {
+    // If a test fails, drop the driver handle without close or quit,
+    // leaving the browser open, which is helpful for debugging.
+    if (!passed && !TestFlag.BROWSER_CLOSE.truthy()
+        && fails_kept++ < MAX_FAILS_KEPT) {
+      driver = null;
+    }
     if (driver != null) {
       driver.get("about:blank");
     }
   }
-
-  // drop the driver handle without close or quit, leaving the browser open
-  void keepOpen() {
-    driver = null;
-  }

   void release() {
     if (driver != null) {

--

--- You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to