labath added a subscriber: labath.
labath added a comment.

I am glad to see more testing of the modules debugging. I have a couple of 
small comments though:

- `-fmodules`: Why is it not being added to CXXFLAGS? Is this how clang is 
supposed to be invoked? (I am not very familiar clang modules)
- there is a `@skipUnlessClangModules` decorator in decorators.py. As far as I 
can see, this patch should now make it obsolete. It seems that it can be 
removed and all invocations replaced with `add_test_categories(["gmodules"])`

And one meta-comment not directly related to this patch:
We already run most of the tests two times. Now we will be doing it once more, 
which will increase the test times even more. I think it's important for each 
debug info format to have good coverage, but I also feel that there are tests 
which have nothing to do with debug info (or their connection to debug info is 
only very peripheral), and it does not make to sense to slow down the tests 
runs by running those tests so many times. We already have a (not very elegant, 
but working) mechanism to avoid this (`NO_DEBUG_INFO_TESTCASE` member). I 
propose that we be more aggressive in using it for new tests which do not 
specifically test debug info. Also when looking at existing tests, we should 
re-evaluate whether the test really needs to be run that many times (right now, 
the largest candidate that comes to mind is TestConcurrentEvents, but I am sure 
there are others I can't think of by name now).


================
Comment at: packages/Python/lldbsuite/test/lldbtest.py:1488
@@ +1487,3 @@
+                    @wraps(attrvalue)
+                    def dwarf_test_method(self, attrvalue=attrvalue):
+                        self.debug_info = "gmodules"
----------------
Shouldn't this be `gmodules_test_method`?


http://reviews.llvm.org/D19998



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to