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

[Pavel Labath via lldb-commits]

labath added a comment.

> If we haven't already, we should probably have some kind of exception

>  wrapper around our decorators that catches non-unittest-related (i.e.

>  unexpected) exceptions and somehow makes them more prevalent - maybe a hard

>  error on the test or an abort or something.


+1

In the new test runner, the `raise Exception` thing will not actually make the test as a whole fail, but at least it will generate a lot of noise in the output, which will hopefully make someone notice it, so I guess it's better than nothing...


================
Comment at: packages/Python/lldbsuite/test/lldbtest.py:1102
@@ -1164,2 +1101,3 @@
+                return func(*args, **kwargs)
         return wrapper
 
----------------
zturner wrote:
> labath wrote:
> > This return statement is the root cause of the problem. If `func` is a class, you will replace it by a strange function-like object.
> This return statement is newly added anyway (and looks to be a mistake).  Does this mean if I remove the return statement, the all of the skip decorators will be able to be used at class level?
> 
I'm not exactly sure what you mean. You can't simply remove the return statement, as that would make the decorator not work. I would try to make this decorator call `unittest2.skipIf`, which knows how to do the right thing. So, something like
```
def skipTestIfFn(expected_fn, bugnumber=None):
  skip, reason = ...
  return unittest2.skipIf(skip, reason)
```
would work (I think), but you could run into some complications as now `expected_fn` gets evaluated at decorator application time rather than during test invocation.

You can try this out for your self. Remove the exception guard, apply the decorator on a class with the condition that is false, and make sure that the tests *do* run.

But maybe we should do that as a separate patch ?


http://reviews.llvm.org/D16741