Fix selenium to detect and handle browser crashes. (issue 12051037)

[ri...@google.com]

Reviewers: kustermann, ahe, Emily Fortuna, Mads Ager,

Description:
Fix selenium to detect and handle browser crashes.

Some context:
On windows, selenium will be hanging a chrome browser
crashes and additional to this we would actually not get information
that the browser crashed, we would get this as a failing test.

This change basically sets up a python timer that will trigger us to
close the browser using browser.service.stop() (I suspect that when
the browser crashes and does not respond in time selenium will call
browser.quit which will be blocking on trying to kill the remote
service).

In addition, this changes the test_runner to handle a crash exitcode
correctly (we will set it to -10 in the bacth runner, I don't know
why, but I dont want to change that in this cl). The current
hasCrashed getter will mask out errors that it does not recognize as
valid windows crashes (of which -10 is not one since it will have a
bunch of set bits in the middle of the bit pattern). I changed this to
explictily recognize the -10 value.
Input on why we have that value is welcome

Please review this at https://codereview.chromium.org/12051037/

SVN Base: http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Affected files:
M tools/testing/dart/test_runner.dart
M tools/testing/run_selenium.py


Index: tools/testing/dart/test_runner.dart
===================================================================
--- tools/testing/dart/test_runner.dart (revision 17449)
+++ tools/testing/dart/test_runner.dart (working copy)
@@ -27,6 +27,8 @@
const int NO_TIMEOUT = 0;
const int SLOW_TIMEOUT_MULTIPLIER = 4;

+const int CRASHING_BROWSER_EXITCODE = -10;
+
typedef void TestCaseEvent(TestCase testCase);
typedef void ExitCodeEvent(int exitCode);
typedef void EnqueueMoreWork(ProcessQueue queue);
@@ -566,6 +568,11 @@
if (exitCode == 3) {
return !timedOut;
}
+ // TODO(ricow): Remove this dirty hack ones we have a selenium
+ // replacement.
+ if (exitCode == CRASHING_BROWSER_EXITCODE) {
+ return !timedOut;
+ }
return (!timedOut && (exitCode < 0) && ((0x3FFFFF00 & exitCode) ==
0));
}
return !timedOut && ((exitCode < 0));
@@ -1274,7 +1281,7 @@

var outcome = _status.split(" ")[2];
var exitCode = 0;
- if (outcome == "CRASH") exitCode = -10;
+ if (outcome == "CRASH") exitCode = CRASHING_BROWSER_EXITCODE;
if (outcome == "FAIL" || outcome == "TIMEOUT") exitCode = 1;
new CommandOutput.fromCase(_currentTest,
_command,
Index: tools/testing/run_selenium.py
===================================================================
--- tools/testing/run_selenium.py (revision 17372)
+++ tools/testing/run_selenium.py (working copy)
@@ -43,6 +43,7 @@
import threading

TIMEOUT_ERROR_MSG = 'FAIL (timeout)'
+CRASH_ERROR_MSG = 'CRASH'

def correctness_test_done(source):
"""Checks if test has completed."""
@@ -84,12 +85,21 @@
if refresh:
browser.refresh()
try:
+ def pythonTimeout():
+ close_browser(browser)
+ # If the browser is crashing selenium may not time out.
+ # Explicitly catch this case with a python timer.
+ t = threading.Timer(timeout, pythonTimeout)
+ t.start()
test_done = CONFIGURATIONS[mode]
element = WebDriverWait(browser, float(timeout)).until(
lambda driver: test_done(driver.page_source))
+ t.cancel()
return browser.page_source
except selenium.common.exceptions.TimeoutException:
return TIMEOUT_ERROR_MSG
+ except:
+ return CRASH_ERROR_MSG

def run_test_in_browser_selenium_rc(sel, html_out, timeout, mode, refresh):
""" Run the desired test in the browser using Selenium 1.0 syntax, and
wait
@@ -226,8 +236,19 @@
if (type(browser) is not selenium.webdriver.chrome.webdriver.WebDriver
and
type(browser) is not selenium.webdriver.ie.webdriver.WebDriver):
browser.close()
- browser.quit()

+ # The builtin quit call will call close on the RemoteDriver which
+ # may hang. Explicitly call browser.service.stop()
+ if (type(browser) is selenium.webdriver.chrome.webdriver.WebDriver):
+ # We may have called stop before if chrome was hanging.
+ try:
+ browser.service.stop()
+ except:
+ print("Trying to close browser that has already been closed")
+ pass
+ else:
+ browser.quit()
+
def report_results(mode, source, browser):
# TODO(vsm): Add a failure check for Dromaeo.
if mode != 'correctness':
@@ -321,6 +342,11 @@
print '>>> TEST PASS'
elif source == TIMEOUT_ERROR_MSG:
print '>>> TEST TIMEOUT'
+ elif source == CRASH_ERROR_MSG:
+ print '>>> TEST CRASH'
+ # The browser crashed, set the browser to None so that we will
+ # create a new instance on next iteration.
+ browser = None
else:
print '>>> TEST FAIL'
sys.stdout.flush()

[kuste...@google.com]

lgtm

https://codereview.chromium.org/12051037/

[efor...@google.com]

lgtm



https://codereview.chromium.org/12051037/