labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.
The interaction between the `add_test_categories` and the "magic test
multiplier" is causing problems, which is evident in the fact how you needed to
add the `@skip` decorator to make it work.
It sounds like we need to decide what will be the "correct" interaction of the
"test multiplier" and tests manually marked with a specific debug info.
I think the most sensible behavior would be to treat a manual debuginfo mark as
a signal that you don't want your test to be auto-multiplied, the test
multiplier could then check whether the test is already marked with one of the
debuginfo annotations, and if it is, then avoid touching it.
What do you think?
================
Comment at:
packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py:120
@@ -119,3 +119,3 @@
if f.endswith(".cpp") or f.endswith(".c"):
- @dwarf_test
+ @add_test_categories("dwarf")
@unittest2.skipIf(TestBase.skipLongRunningTest(), "Skip this long
running test")
----------------
You need `[]` around the `"dwarf"`. Otherwise, you will get errors here, when
the skipIf below does not actually skip.
================
Comment at:
packages/Python/lldbsuite/test/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py:10
@@ +9,3 @@
+ @add_test_categories("dwarf")
+ @skipIf(debug_info=not_in(["dwarf"]))
+ def test_limit_debug_info(self):
----------------
It should not be necessary to annotate and skip at the same time.
================
Comment at: packages/Python/lldbsuite/test/lldbtest.py:2226
@@ -2267,2 +2225,3 @@
for attrname, attrvalue in attrs.items():
if attrname.startswith("test") and not getattr(attrvalue,
"__no_debug_info_test__", False):
+ target_platform =
lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
----------------
Here, we would then also check `getCategoriesForTest(attrvalue)` for presence
of debug info markers. It looks like you will need to move that function from
test_result.py to somewhere else though...
================
Comment at: packages/Python/lldbsuite/test/lldbtest.py:2230
@@ +2229,3 @@
+ dont_do_dsym_test = any(platform in target_platform for
platform in ["linux", "freebsd", "windows"])
+ dont_do_dwo_test = any(platform in target_platform for
platform in ["darwin", "macosx", "ios"])
+
----------------
might as well use `getDarwinOSTriples()` here
================
Comment at: packages/Python/lldbsuite/test/lldbtest.py:2233
@@ +2232,3 @@
+ if not dont_do_dsym_test:
+ @add_test_categories(["dsym"])
+ @wraps(attrvalue)
----------------
This annotation will overwrite any previous category annotations that the user
has previously added. I wasn't sure whether we wanted this behavior when I was
writing the decorator, but now that we have these automatic categorizations, it
definitely sounds like a bad idea. `add_test_categories` should really "add"
categories to the test.
Since this is quite orthogonal to your patch, I'll whip up a separate patch to
do that.
http://reviews.llvm.org/D15428
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
