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

[Tamas Berghammer via lldb-commits]

tberghammer 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
----------------
labath wrote:
> 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.
I think the pythonic way is to accept (almost) any type of argument including a single element string and any iterable (list, set, generator expression, etc...). In long term I would like to see all of our decorator taking single and multi argument at the same time but we can make that change later with refactoring all decorator to support it (it can be handled easily by a utility function).


Repository:
  rL LLVM

http://reviews.llvm.org/D15451