[Lldb-commits] [PATCH] D16741: Make many of the skip decorators use common code

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Mon Feb 1 16:00:52 PST 2016


zturner marked an inline comment as done.

================
Comment at: packages/Python/lldbsuite/test/lldbtest.py:560
@@ -559,3 +559,1 @@
     """Decorate the item as a debugserver test."""
-    if isinstance(func, type) and issubclass(func, unittest2.TestCase):
-        raise Exception("@debugserver_test can only be used to decorate a test method")
----------------
labath wrote:
> Please don't remove these. They make sure that you do not attempt to apply this decorator to a class. If the decorator is not ready for that (and the current skipIfTestFn does not appear to be), it will just cause the whole cause the whole class to be silently ignored (we've had this happen before).
> 
> I think it's ok to centralize the check in skipTestIfFn.
Why don't we want this behavior?  With this check, an exception is being thrown if the condition is true.  Currently the test suite is not failing anywhere with this exception, meaning the check is never true.  And it would only be true if we put the decorator on the actual class.  Which we already do in some places (see `TestAddDsymCommand.py`) and is actually a very useful way of skipping or xfailing an entire class.  I want to be able to skip / xfail entire classes *more* easily, not less easily.



http://reviews.llvm.org/D16741





More information about the lldb-commits mailing list