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

[Todd Fiala via lldb-commits]

On Mon, Dec 14, 2015 at 5:39 AM, Tamas Berghammer <tberghammer at google.com>
wrote:

> 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).
>

I agree with Tamas on this.  We don't  get much static-time feedback from
dynamic scripting languages, so I'd rather err on the side of making it
easier for the user of the feature rather than making them think in a
static-language mindset without static-language help to point out misuses.
In these cases, where we're talking about "one or more of these" (where
these is some kind of argument, like one or more OSes as in oslist) I'd
prefer them to take a single or plural literal in the future.  We can wrap
them in a python sequence helper that handles accepting a singular or
plural form of argument.

e.g.

def list_of(singular_or_plural_arg):
    # Check type in here, return list/sequence if it is one, otherwise wrap
in list.
    ...

def some_decorator_logic(os_one_or_more):
    # Do something with the singular or plural form of os_one_or_more,
    # Use the list_of() helper to make sure we always treat in sequence
context.
    for os in list_of(os_one_or_more):
        # do something with the one or more OS entries.

Something like that.  We probably have a few categories of helpers we'd
need (checking if an entry is in a one-or-more set, etc.)

-Todd


>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D15451
>
>
>
>


-- 
-Todd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20151214/aeefc7ef/attachment-0001.html>