labath added inline comments.
================
Comment at: packages/Python/lldbsuite/test/decorators.py:527
@@ -525,3 +526,3 @@
-def skipUnlessClangModules():
+def skipUnlessClangModules(func):
"""Decorate the item to skip test unless Clang -gmodules flag is
supported."""
----------------
tfiala wrote:
> labath wrote:
> > This whole decorator can be removed. It's invocations can be replaced with
> > `add_test_categories(["gmodules"])` (cf. `lldbtest.py:1453`, it will avoid
> > duplicating the test if it is already annotated with categories).
> Yes, I see now. I did not understand add_test_categories(["gmodules"]) was a
> valid way to mark a test as gmodules-only.
>
> It does look a little weird that we need to do:
> @no_debug_info_test
> @add_test_categories(["gmodules"])
>
> I wonder if we want a combined decorator that is effectively:
> @valid_debug_info([...])
>
> that specifies that this test is only valid for those debug info entries
> specified. Then the entry becomes a single decorator:
> @valid_debug_info(["gmodules"])
>
> What do you think?
You shouldn't need both decorators. The test multiplicator will first check
whether whether the test is already explicitly annotated with some debug info
category (lldbtest.py:1446) and multiply only into the categories that were not
annotated.
Now that I look at it closer, I see that the code there is not entirely correct
in case you annotate a test with two categories (it will create two tests, but
both of them will be annotated with both categories). However, this is a topic
for another change, and should not prevent you from using it now.
So, yes, you should only need one decorator, and things should just work. As
for the current syntax, I am open to making any changes to it, but I want to
avoid having two ways of doing this in parallel.
http://reviews.llvm.org/D19998
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
