[Lldb-commits] [PATCH] D15451: Make test categories composable

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Mon Dec 14 04:32:12 PST 2015


labath added inline comments.

================
Comment at: packages/Python/lldbsuite/test/lldbtest.py:518
@@ +517,3 @@
+        if hasattr(func, "categories"):
+            cat.extend(func.categories)
+        func.categories = cat
----------------
tberghammer wrote:
> tfiala wrote:
> > This code could potentially accept a single category and drop the need for the list.  It could check the type of the input (i.e. the categories input) and, if a single element rather than a list, it could just cat.append() it.
> > 
> > That would enable a single argument passed to @add_test_categories()
> > 
> > Totally minor and you can skip this, it just makes for a nice usability enhancement provided the decorator is documented as such.
> +1: I think we will (accidentally) call the function by a single string in some case so accepting it would be a good idea.
The test_categories.validate will catch the case where you forget to put the braces and throw errors at you, so it's not possible to get unexpected behavior here.

I may be old-fashioned, but I like when things have a single type, which is obvious from the code. Look at all the cases where we do `callable(bugnumber)` and tell me if it's obvious why a "bug number" should be "callable". Plus, none of our other decorators taking lists have this kind of magic in them (imagine how would this complicate the logic of expectedFailureAll).

If we want to provide a simpler syntax for adding a single category, i would go for adding a separate `@add_test_category` decorator for that.


http://reviews.llvm.org/D15451





More information about the lldb-commits mailing list