tfiala created this revision.
tfiala added reviewers: labath, zturner.
tfiala added subscribers: lldb-commits, zturner.

This change enhances the LLDB test infrastructure to convert
load-time exceptions in a given Python test module into errors.
Before this change, specifying a non-existent test decorator,
or otherwise having some load-time error in a python test module,
would not get flagged as an error.

With this change, typos and other load-time errors in a python
test file get converted to errors and reported by the
test runner.

This change also includes test infrastructure tests that include
covering the new work here.  I'm going to wait until we have
these infrastructure tests runnable on the main platforms before
I try to work that into all the normal testing workflows.

The test infrastructure tests can be run by using the standard python module 
testing practice of doing the following:

cd packages/Python/lldbsuite/test_event
python -m unittest discover -s test/src -p 'Test*.py'

Those tests run the dotest inferior with a known broken test and verify that 
the errors are caught.  These tests did not pass until I modified dotest.py to 
capture them properly.

@zturner, if you have the chance, if you could try those steps above (the 
python -m unittest ... line) on Windows, that would be great if we can address 
any python2/3/Windows bits there.  I don't think there's anything fancy, but I 
didn't want to hook it into test flow until I know it works there.

I'll be slowly adding more tests that cover some of the other breakage I've 
occasionally seen that didn't get collected as part of the summarization.  This 
is the biggest one I'm aware of.

http://reviews.llvm.org/D20193

Files:
  packages/Python/lldbsuite/test/dotest.py
  packages/Python/lldbsuite/test/issue_verification/TestInvalidDecorator.py.park
  packages/Python/lldbsuite/test_event/event_builder.py
  packages/Python/lldbsuite/test_event/formatter/pickled.py
  
packages/Python/lldbsuite/test_event/test/resources/invalid_decorator/TestInvalidDecorator.py
  packages/Python/lldbsuite/test_event/test/src/TestCatchInvalidDecorator.py
  packages/Python/lldbsuite/test_event/test/src/event_collector.py

Index: packages/Python/lldbsuite/test_event/test/src/event_collector.py
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test_event/test/src/event_collector.py
@@ -0,0 +1,83 @@
+from __future__ import absolute_import
+from __future__ import print_function
+
+import os
+import subprocess
+import sys
+
+from six.moves import cPickle
+
+
+def path_to_dotest_py():
+    return os.path.join(
+        os.path.dirname(__file__),
+        os.path.pardir,
+        os.path.pardir,
+        os.path.pardir,
+        os.path.pardir,
+        os.path.pardir,
+        os.path.pardir,
+        "test",
+        "dotest.py")
+
+
+def pickled_events_filename():
+    return "/tmp/lldb_test_event_pickled_event_output.dat"
+
+
+def _collect_events_with_command(command):
+    # Delete the events file if it exists
+    if os.path.exists(pickled_events_filename()):
+        os.remove(pickled_events_filename())
+
+    if os.path.exists(pickled_events_filename()):
+        raise Exception("pickled events filename should not exist!")
+
+    # Run the single test with dotest.py, outputting
+    # the raw pickled events to a temp file.
+    with open(os.devnull, 'w') as dev_null_file:
+        status = subprocess.call(
+            command,
+            stdout=dev_null_file,
+            stderr=dev_null_file)
+
+    # Unpickle the events
+    events = []
+    if os.path.exists(pickled_events_filename()):
+        with open(pickled_events_filename(), "rb") as events_file:
+            while True:
+                try:
+                    # print("reading event")
+                    event = cPickle.load(events_file)
+                    # print("read event: {}".format(event))
+                    if event:
+                        events.append(event)
+                except EOFError:
+                    # This is okay.
+                    break
+    return events
+
+def collect_events_whole_file(test_filename):
+    command = [
+        sys.executable,
+        path_to_dotest_py(),
+        "--inferior",
+        "--results-formatter=lldbsuite.test_event.formatter.pickled.RawPickledFormatter",
+        "--results-file={}".format(pickled_events_filename()),
+        "-p", os.path.basename(test_filename),
+        os.path.dirname(test_filename)
+        ]
+    return _collect_events_with_command(command)
+
+
+def collect_events_for_directory_with_filter(test_filename, filter):
+    command = [
+        sys.executable,
+        path_to_dotest_py(),
+        "--inferior",
+        "--results-formatter=lldbsuite.test_event.formatter.pickled.RawPickledFormatter",
+        "--results-file={}".format(pickled_events_filename()),
+        "-f", filter,
+        os.path.dirname(test_filename)
+        ]
+    return _collect_events_with_command(command)
Index: packages/Python/lldbsuite/test_event/test/src/TestCatchInvalidDecorator.py
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test_event/test/src/TestCatchInvalidDecorator.py
@@ -0,0 +1,72 @@
+#!/usr/bin/env python
+"""
+Tests that the event system reports issues during decorator
+handling as errors.
+"""
+# System-provided imports
+import os
+import unittest
+
+# Local-provided imports
+import event_collector
+
+
+class TestCatchInvalidDecorator(unittest.TestCase):
+
+    def test_with_whole_file(self):
+        """
+        Test that a non-existent decorator generates a test-event error
+        when running all tests in the file.
+        """
+        # Determine the test case file we're using.
+        test_file = os.path.join(
+            os.path.dirname(__file__),
+            os.path.pardir,
+            "resources",
+            "invalid_decorator",
+            "TestInvalidDecorator.py")
+
+        # Collect all test events generated for this file.
+        events = event_collector.collect_events_whole_file(test_file)
+
+        # Filter out test events.
+        results = [
+            event
+            for event in events
+            if event.get("event", None) in ['job_result', 'test_result']]
+
+        self.assertTrue(
+            len(results) > 0,
+            "At least one job or test result should have been returned")
+
+    def test_with_function_filter(self):
+        """
+        Test that a non-existent decorator generates a test-event error
+        when running a filtered test.
+        """
+        # Determine the test case file we're using.
+        test_file_dir = os.path.join(
+            os.path.dirname(__file__),
+            os.path.pardir,
+            "resources",
+            "invalid_decorator")
+
+        # Collect all test events generated during running of tests
+        # in a given directory using a test name filter.  Internally,
+        # this runs through a different code path that needs to be
+        # set up to catch exceptions.
+        events = event_collector.collect_events_for_directory_with_filter(
+            test_file_dir,
+            "NonExistentDecoratorTestCase.test")
+
+        # Filter out test events.
+        results = [
+            event
+            for event in events
+            if event.get("event", None) in ['job_result', 'test_result']]
+
+        self.assertTrue(
+            len(results) > 0,
+            "At least one job or test result should have been returned")
+if __name__ == "__main__":
+    unittest.main()
Index: packages/Python/lldbsuite/test_event/test/resources/invalid_decorator/TestInvalidDecorator.py
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test_event/test/resources/invalid_decorator/TestInvalidDecorator.py
@@ -0,0 +1,13 @@
+from __future__ import print_function
+from lldbsuite.test import lldbtest
+from lldbsuite.test import decorators
+
+
+class NonExistentDecoratorTestCase(lldbtest.TestBase):
+
+    mydir = lldbtest.TestBase.compute_mydir(__file__)
+
+    @decorators.nonExistentDecorator(bugnumber="yt/1300")
+    def test(self):
+        """Verify non-existent decorators are picked up by test runner."""
+        pass
Index: packages/Python/lldbsuite/test_event/formatter/pickled.py
===================================================================
--- packages/Python/lldbsuite/test_event/formatter/pickled.py
+++ packages/Python/lldbsuite/test_event/formatter/pickled.py
@@ -33,6 +33,7 @@
     def __init__(self, out_file, options):
         super(RawPickledFormatter, self).__init__(out_file, options)
         self.pid = os.getpid()
+        self.use_send = out_file is not None and "send" in dir(out_file)
 
     def handle_event(self, test_event):
         super(RawPickledFormatter, self).handle_event(test_event)
@@ -50,8 +51,14 @@
         # Tack on the pid.
         test_event["pid"] = self.pid
 
-        # Send it as {serialized_length_of_serialized_bytes}{serialized_bytes}
-        import struct
-        msg = cPickle.dumps(test_event)
-        packet = struct.pack("!I%ds" % len(msg), len(msg), msg)
-        self.out_file.send(packet)
+        # If sending over a socket, we need to format this
+        # in a way that can be collected properly on the other
+        # end.
+        if self.use_send:
+            # Send it as {serialized_length_of_serialized_bytes}{serialized_bytes}
+            import struct
+            msg = cPickle.dumps(test_event)
+            packet = struct.pack("!I%ds" % len(msg), len(msg), msg)
+            self.out_file.send(packet)
+        else:
+            cPickle.dump(test_event, self.out_file)
Index: packages/Python/lldbsuite/test_event/event_builder.py
===================================================================
--- packages/Python/lldbsuite/test_event/event_builder.py
+++ packages/Python/lldbsuite/test_event/event_builder.py
@@ -321,6 +321,19 @@
         return event
 
     @staticmethod
+    def event_for_job_test_add_error(test_filename, exception, backtrace):
+        event = EventBuilder.bare_event(EventBuilder.TYPE_JOB_RESULT)
+        event["status"] = EventBuilder.STATUS_ERROR
+        if test_filename is not None:
+            event["test_filename"] = EventBuilder._assert_is_python_sourcefile(test_filename)
+        if exception is not None and "__class__" in dir(exception):
+            event["issue_class"] = exception.__class__
+        event["issue_message"] = exception
+        if backtrace is not None:
+            event["issue_backtrace"] = backtrace
+        return event
+
+    @staticmethod
     def event_for_job_exceptional_exit(
             pid, worker_index, exception_code, exception_description,
             test_filename, command_line):
Index: packages/Python/lldbsuite/test/issue_verification/TestInvalidDecorator.py.park
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/issue_verification/TestInvalidDecorator.py.park
@@ -0,0 +1,13 @@
+from __future__ import print_function
+from lldbsuite.test import lldbtest
+from lldbsuite.test import decorators
+
+
+class NonExistentDecoratorTestCase(lldbtest.TestBase):
+
+    mydir = lldbtest.TestBase.compute_mydir(__file__)
+
+    @decorators.nonExistentDecorator(bugnumber="yt/1300")
+    def test(self):
+        """Verify non-existent decorators are picked up by test runner."""
+        pass
Index: packages/Python/lldbsuite/test/dotest.py
===================================================================
--- packages/Python/lldbsuite/test/dotest.py
+++ packages/Python/lldbsuite/test/dotest.py
@@ -715,7 +715,22 @@
             for filterspec in configuration.filters:
                 # Optimistically set the flag to True.
                 filtered = True
-                module = __import__(base)
+                try:
+                    module = __import__(base)
+                except Exception as ex:
+                    # We hit here when there is a failure to load in a module,
+                    # as will happen with invalid decorators.  This path happens
+                    # when using '-f' filters.
+                    test_filename = os.path.abspath(os.path.join(dir, name))
+                    if test_filename.endswith(".pyc"):
+                        test_filename = test_filename[0:-1]
+                    if configuration.results_formatter_object is not None:
+                        import traceback
+                        backtrace = traceback.format_exc()
+                        configuration.results_formatter_object.handle_event(
+                            EventBuilder.event_for_job_test_add_error(
+                                test_filename, ex, backtrace))
+                    raise
                 parts = filterspec.split('.')
                 obj = module
                 for part in parts:
@@ -737,13 +752,23 @@
             if configuration.filters and not filtered:
                 continue
 
-            # Add either the filtered test case(s) (which is done before) or the entire test class.
             if not filterspec or not filtered:
-                # A simple case of just the module name.  Also the failover case
-                # from the filterspec branch when the (base, filterspec) combo
-                # doesn't make sense.
-                configuration.suite.addTests(unittest2.defaultTestLoader.loadTestsFromName(base))
-
+                # Add the entire file's worth of tests since we're not filtered.
+                # Also the failover case when the filterspec branch
+                # (base, filterspec) combo doesn't make sense.
+                try:
+                    configuration.suite.addTests(unittest2.defaultTestLoader.loadTestsFromName(base))
+                except Exception as ex:
+                    test_filename = os.path.abspath(os.path.join(dir, name))
+                    if test_filename.endswith(".pyc"):
+                        test_filename = test_filename[0:-1]
+                    if configuration.results_formatter_object is not None:
+                        import traceback
+                        backtrace = traceback.format_exc()
+                        configuration.results_formatter_object.handle_event(
+                            EventBuilder.event_for_job_test_add_error(
+                                test_filename, ex, backtrace))
+                    raise
 
 def disabledynamics():
     import lldb
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to