I noticed a few Python API tests (test/API/python_api) were not marked as NO_DEBUG_INFO_TESTCASE causing them to be rerun needlessly with different debug info variants. Instead of adding NO_DEBUG_INFO_TESTCASE, I decided it would be less error prone to convert all of them to a new test case format (PythonAPITest) which has that property set.
Details
Diff Detail
Event Timeline
this diff has made me wonder: should we have a NoDebugInfoTestCase that can be used by any test, and would replace assigning to NO_DEBUG_INFO_TESTCASE?
I was wondering the same thing. I decided against it because we already have NO_DEBUG_INFO_TESTCASE (test level) and @no_debug_info_test (function level) and I didn't want to add yet another option. Additionally, there's a few other patters that are common for the Python API tests (e.g. the self.source and self.line) that could be moved up into the base class in a follow up patch.
Note that we already kind of have that. The Base test class is the base of all our tests and does not support automatic test replication. Tests which inherit directly from that (lldb-server and lldb-vscode tests) for instance, don't get the replication even though they don't use NO_DEBUG_INFO_TESTCASE. The TestBase class (which we use for the "normal" SB API tests) adds a bunch of SB utility functions *and* it adds the ability to replicate tests. I think that separating the two parts would be completely reasonable, and would remove the need for most/all of the uses of NO_DEBUG_INFO_TESTCASE and @no_debug_info_test.
The ability to replicate tests is not even part of TestBase I think. Except for the attribute, it's all handled by the LLDBTestCaseFactory. If you think it's better to go the test case route (in favor of NO_DEBUG_INFO_TESTCASE) we can pull up everything but the factory and property from TestBase into Base (as you outlined) or keep them separate and just move the factory and property down to a (third) child class. Either way we should rename those 2 (3) classes to something more meaningful.
@labath: was your comment meant as a suggestion (i.e. we should make this change) or just informative in reply to Dave's question (i.e. we could make this change if we needed it)?
Well.. yes, the actual code lives there, but if you think about it from the POV of a test case author, then he has the choice of either inheriting from Base (and getting no replication), or from TestBase (and getting the replication unless he disables it). I would say that the LLDBTestCaseFactory is an implementation detail that should not be visible to the outside world.
If you think it's better to go the test case route (in favor of NO_DEBUG_INFO_TESTCASE) we can pull up everything but the factory and property from TestBase into Base (as you outlined) or keep them separate and just move the factory and property down to a (third) child class.
I wouldn't want to put everything into Base, precisely because we have these gdb-server, vscode, etc., which don't operate on the SB API, and that functionality makes no sense there.
Either way we should rename those 2 (3) classes to something more meaningful.
Oh yes.
@labath: was your comment meant as a suggestion (i.e. we should make this change) or just informative in reply to Dave's question (i.e. we could make this change if we needed it)?
Something in between, I guess. I would like it to work that way, but I'm not going to make anyone do that right now. It's also a part of a larger discussion of whether the debug info replication should be opt-in or opt-out. A separate class makes more sense for an opt-in world, whereas the status quo is kind of geared towards an opt-out mechanism.