Don't depend on implementation details of unittest2 for our custom decorators

[zturner]

Summary

The specific exception types that are thrown internally by unittest2 are considered implementation details and even documented as such in the source code of the library. This patch attempts to make this correct by removing reliance on these implementation details, and deferring to the library implementation.

I'm not sure I have a good grasp of how all these decorators are supposed to work, but I think I did this right. Please verify carefully that I didn't mess anything up.

The goal of this patch is to make a conditionalExpectedFailure decorator that has the following behavior:

1. If the "condition" is true (i.e. this is an expected failure), reuse the library's expectedFailure decorator.
2. If the condition is false (i.e. this is not an expected failure), just call the method.

[labath]

From what I can tell, it should just work. I assume you've done the obvious checks, like deliberately failing a test and seeing it registers as failed, etc. I think we should just commit it, and carefully observe the buildbots.

[tberghammer]

packages/Python/lldbsuite/test/lldbtest.py

605

You are swallowing the bug number here

Based on the implementation of unittest2.expectedFailure I think you should write the following to preserve it (I haven't tested it):

unittest2.expectedFailure(bugnumber)(func)

[labath]

(The upstream unittest does not seem to have the bugnumber feature. I am assuming the intention here is to make this upstream compatible, in hope of moving over there at some point.)

[zturner]

In D14406#283034, @labath wrote:

(The upstream unittest does not seem to have the bugnumber feature. I am assuming the intention here is to make this upstream compatible, in hope of moving over there at some point.)

I can leave the bugnumber thing in there for now, just to reduce the impact of this change and the risk of messing something up.

[tberghammer]

If the purpose of the change to get closer to upstream then I am fine with removing the bug number here. In general I don't feel it is a that high risk change, but I might miss something.

[zturner]

packages/Python/lldbsuite/test/lldbtest.py

605

Actually I think it already has the same sematnics. When I use your version it doesn't work. I think it works with my version because of the if six.callable(bugnumber) check on line 610 (which is cut out in the context here, but you can check it).

I'll put it in like this and see what happens

packages/Python/lldbsuite/test/lldbtest.py

         def wrapper(*args, **kwargs):
             from unittest2 import case
             self = args[0]
-            try:
-                func(*args, **kwargs)
-            except Exception:
-                if expected_fn(self):
-                    raise case._ExpectedFailure(sys.exc_info(), bugnumber)
-                else:
-                    raise
             if expected_fn(self):
-                raise case._UnexpectedSuccess(sys.exc_info(), bugnumber)
+                xfail_func = unittest2.expectedFailure(func)
+                xfail_func(*args, **kwargs)
+            else:
+                func(*args, **kwargs)
         return wrapper
     # if bugnumber is not-callable(incluing None), that means decorator function is called with optional arguments
     # return decorator in this case, so it will be used to decorating original method