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):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@121816
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
2012-07-03 Dirk Pranke <dpranke@chromium.org>
+ 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 <dpranke@chromium.org>
+
nrwt: fix mock port
https://bugs.webkit.org/show_bug.cgi?id=90500
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)
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
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):
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:
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.
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
_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:
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()
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
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)
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)
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):