Diff
Modified: trunk/Tools/ChangeLog (121815 => 121816)
--- trunk/Tools/ChangeLog 2012-07-04 00:03:17 UTC (rev 121815)
+++ trunk/Tools/ChangeLog 2012-07-04 00:14:25 UTC (rev 121816)
@@ -1,5 +1,31 @@
2012-07-03 Dirk Pranke <[email protected]>
+ nrwt: clean up exception handling and make sure we log some more failures
+ https://bugs.webkit.org/show_bug.cgi?id=90503
+
+ Reviewed by Ojan Vafai.
+
+ There were several places where exceptions weren't getting
+ logged, most notably if you passed a bad value to --platform.
+ This change tests that and cleans things up a bit; more cleanup
+ will be possible when we rework the manager_worker_broker code.
+
+ * Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py:
+ (_BrokerConnection.raise_exception):
+ (_InlineWorkerConnection.raise_exception):
+ * Scripts/webkitpy/layout_tests/controllers/worker.py:
+ (Worker.run):
+ (Worker.kill_driver):
+ * Scripts/webkitpy/layout_tests/port/factory.py:
+ (PortFactory.get):
+ * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+ (run):
+ (main):
+ * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
+ (MainTest.test_unsupported_platfrom):
+
+2012-07-03 Dirk Pranke <[email protected]>
+
nrwt: fix mock port
https://bugs.webkit.org/show_bug.cgi?id=90500
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py (121815 => 121816)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py 2012-07-04 00:03:17 UTC (rev 121815)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py 2012-07-04 00:14:25 UTC (rev 121816)
@@ -255,9 +255,10 @@
message_name, *message_args)
def raise_exception(self, exc_info):
- # Since tracebacks aren't picklable, send the extracted stack instead.
+ # Since tracebacks aren't picklable, send the extracted stack instead,
+ # but at least log the full traceback.
exception_type, exception_value, exception_traceback = sys.exc_info()
- stack_utils.log_traceback(_log.debug, exception_traceback)
+ stack_utils.log_traceback(_log.error, exception_traceback)
stack = traceback.extract_tb(exception_traceback)
self._broker.post_message(self._client, self._post_topic, 'exception', exception_type, exception_value, stack)
@@ -305,8 +306,12 @@
self._worker_connection.raise_exception(sys.exc_info())
finally:
_log.debug("%s done with message loop%s" % (self._name, exception_msg))
- self.worker.cleanup()
- self._worker_connection.post_message('done')
+ try:
+ self.worker.cleanup()
+ finally:
+ # Make sure we post a done so that we can flush the log messages
+ # and clean up properly even if we raise an exception in worker.cleanup().
+ self._worker_connection.post_message('done')
def handle_stop(self, source):
self._done = True
@@ -456,8 +461,11 @@
def raise_exception(self, exc_info):
# Since the worker is in the same process as the manager, we can
# raise the exception directly, rather than having to send it through
- # the queue. This allows us to preserve the traceback.
- raise exc_info[0], exc_info[1], exc_info[2]
+ # the queue. This allows us to preserve the traceback, but we log
+ # it anyway for consistency with the multiprocess case.
+ exception_type, exception_value, exception_traceback = sys.exc_info()
+ stack_utils.log_traceback(_log.error, exception_traceback)
+ raise exception_type, exception_value, exception_traceback
class _Process(multiprocessing.Process):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py (121815 => 121816)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py 2012-07-04 00:03:17 UTC (rev 121815)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py 2012-07-04 00:14:25 UTC (rev 121816)
@@ -134,10 +134,13 @@
return thread_timeout_sec
def kill_driver(self):
- if self._driver:
+ # Be careful about how and when we kill the driver; if driver.stop()
+ # raises an exception, this routine may get re-entered via __del__.
+ driver = self._driver
+ self._driver = None
+ if driver:
_log.debug("%s killing driver" % self._name)
- self._driver.stop()
- self._driver = None
+ driver.stop()
def run_test_with_timeout(self, test_input, timeout):
if self._options.run_singly:
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/factory.py (121815 => 121816)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/factory.py 2012-07-04 00:03:17 UTC (rev 121815)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/factory.py 2012-07-04 00:14:25 UTC (rev 121816)
@@ -112,7 +112,7 @@
if port_name.startswith(cls.port_name):
port_name = cls.determine_full_port_name(self._host, options, port_name)
return cls(self._host, port_name, options=options, **kwargs)
- raise NotImplementedError('unsupported port: %s' % port_name)
+ raise NotImplementedError('unsupported platform: "%s"' % port_name)
def all_port_names(self):
"""Return a list of all valid, fully-specified, "real" port names.
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py (121815 => 121816)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2012-07-04 00:03:17 UTC (rev 121815)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2012-07-04 00:14:25 UTC (rev 121816)
@@ -37,7 +37,8 @@
import sys
from webkitpy.common.host import Host
-from webkitpy.layout_tests.controllers.manager import Manager
+from webkitpy.common.system import stack_utils
+from webkitpy.layout_tests.controllers.manager import Manager, WorkerException, TestRunInterruptedException
from webkitpy.layout_tests.models import test_expectations
from webkitpy.layout_tests.port import port_options
from webkitpy.layout_tests.views import printing
@@ -46,6 +47,14 @@
_log = logging.getLogger(__name__)
+# This mirrors what the shell normally does.
+INTERRUPTED_EXIT_STATUS = signal.SIGINT + 128
+
+# This is a randomly chosen exit code that can be tested against to
+# indicate that an unexpected exception occurred.
+EXCEPTIONAL_EXIT_STATUS = 254
+
+
def lint(port, options):
host = port.host
if options.platform:
@@ -119,6 +128,11 @@
unexpected_result_count = manager.run()
_log.debug("Testing completed, Exit status: %d" % unexpected_result_count)
+ except Exception:
+ exception_type, exception_value, exception_traceback = sys.exc_info()
+ if exception_type not in (KeyboardInterrupt, TestRunInterruptedException, WorkerException):
+ stack_utils.log_traceback(_log.error, exception_traceback)
+ raise
finally:
printer.cleanup()
@@ -433,8 +447,8 @@
return option_parser.parse_args(args)
-def main():
- options, args = parse_args()
+def main(argv=None):
+ options, args = parse_args(argv)
if options.platform and 'test' in options.platform:
# It's a bit lame to import mocks into real code, but this allows the user
# to run tests against the test platform interactively, which is useful for
@@ -443,7 +457,14 @@
host = MockHost()
else:
host = Host()
- port = host.port_factory.get(options.platform, options)
+
+ try:
+ port = host.port_factory.get(options.platform, options)
+ except NotImplementedError, e:
+ # FIXME: is this the best way to handle unsupported port names?
+ print >> sys.stderr, str(e)
+ return EXCEPTIONAL_EXIT_STATUS
+
logging.getLogger().setLevel(logging.DEBUG if options.verbose else logging.INFO)
return run(port, options, args)
@@ -451,12 +472,7 @@
if '__main__' == __name__:
try:
sys.exit(main())
- except KeyboardInterrupt:
- # This mirrors what the shell normally does.
- INTERRUPTED_EXIT_STATUS = signal.SIGINT + 128
- sys.exit(INTERRUPTED_EXIT_STATUS)
- except Exception:
- # This is a randomly chosen exit code that can be tested against to
- # indicate that an unexpected exception occurred.
- EXCEPTIONAL_EXIT_STATUS = 254
+ except Exception, e:
+ if e.__class__ in (KeyboardInterrupt, TestRunInterruptedException):
+ sys.exit(INTERRUPTED_EXIT_STATUS)
sys.exit(EXCEPTIONAL_EXIT_STATUS)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py (121815 => 121816)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2012-07-04 00:03:17 UTC (rev 121815)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2012-07-04 00:14:25 UTC (rev 121816)
@@ -904,7 +904,21 @@
self.assertEquals(full_results['has_wdiff'], False)
self.assertEquals(full_results['has_pretty_patch'], False)
+ def test_unsupported_platform(self):
+ oc = outputcapture.OutputCapture()
+ try:
+ oc.capture_output()
+ res = run_webkit_tests.main(['--platform', 'foo'])
+ finally:
+ stdout, stderr, logs = oc.restore_output()
+ self.assertEquals(res, run_webkit_tests.EXCEPTIONAL_EXIT_STATUS)
+ self.assertEquals(stdout, '')
+ self.assertTrue('unsupported platform' in stderr)
+
+ # This is empty because we don't even get a chance to configure the logger before failing.
+ self.assertEquals(logs, '')
+
class EndToEndTest(unittest.TestCase):
def parse_full_results(self, full_results_text):
json_to_eval = full_results_text.replace("ADD_RESULTS(", "").replace(");", "")
