Reviewers: kpreid2,

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

Please review this at https://codereview.appspot.com/11136043/

Affected files:
  M     tests/com/google/caja/plugin/BrowserTestCase.java
  M     tests/com/google/caja/plugin/WebDriverHandle.java


Index: tests/com/google/caja/plugin/BrowserTestCase.java
===================================================================
--- tests/com/google/caja/plugin/BrowserTestCase.java   (revision 5479)
+++ tests/com/google/caja/plugin/BrowserTestCase.java   (working copy)
@@ -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;
   }
Index: tests/com/google/caja/plugin/WebDriverHandle.java
===================================================================
--- tests/com/google/caja/plugin/WebDriverHandle.java   (revision 5479)
+++ tests/com/google/caja/plugin/WebDriverHandle.java   (working copy)
@@ -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,20 +116,18 @@
     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) {
       try {


--

--- 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