[lldb/Test] Ensure inline tests have a unique build directory

[JDevlieghere]

Summary

Inline tests have one method named 'test' which means that multiple inline tests in the same file end up sharing the same build directory per variant.

The output bellow illustrates that even though BasicEntryValues_GNU and BasicEntryValues_V5 have unique names, the test method for the dwarf variant is called test_dwarf in both cases.

PASS: LLDB (/Users/jonas/llvm/build-ra/bin/clang-11-x86_64) :: test_dwarf (lldbsuite.test.lldbtest.BasicEntryValues_GNU)
PASS: LLDB (/Users/jonas/llvm/build-ra/bin/clang-11-x86_64) :: test_gmodules (lldbsuite.test.lldbtest.BasicEntryValues_GNU)
PASS: LLDB (/Users/jonas/llvm/build-ra/bin/clang-11-x86_64) :: test_dsym (lldbsuite.test.lldbtest.BasicEntryValues_V5)
PASS: LLDB (/Users/jonas/llvm/build-ra/bin/clang-11-x86_64) :: test_dwarf (lldbsuite.test.lldbtest.BasicEntryValues_V5)
PASS: LLDB (/Users/jonas/llvm/build-ra/bin/clang-11-x86_64) :: test_gmodules (lldbsuite.test.lldbtest.BasicEntryValues_V5)

The 'inline_name' attribute ensures that the (unique) test name is used instead.

[JDevlieghere]

I discovered this because TestBasicEntryValues.py was failing with the reproducers. Because the paths to the binaries were the same, we'd end up with only the GNU variant of the binary in the VFS.

[vsk]

lldb/packages/Python/lldbsuite/test/lldbtest.py

1750

Is inline_suffix = attrs.get('__inline_name__') no good?

1778

Would it be all right to unconditionally set method_name = name + attrname + cat?

[labath]

I noticed this issue after committing the patch that caused it. I wasn't rushing to fix it because it did not seem like a big problem. But, of course it is a big problem for reproducers...

That said, I think the proposed fix is more complicated than needed and would not handle @no_debug_info_test tests (because the fixup happens in the code which does the magic test replication).

It should be sufficient to add this to the definition of InlineTest class:

def getBuildDirBasename(self):
    return self.__class__.__name__ + "." + self.testMethodName

That will result in paths like:

lldb-test-build.noindex/functionalities/param_entry_vals/basic_entry_values/BasicEntryValues_GNU.test_dwarf/a.out
lldb-test-build.noindex/functionalities/param_entry_vals/basic_entry_values/BasicEntryValues_V5.test_dwarf/a.out

Which is better than lldb-test-build.noindex/functionalities/param_entry_vals/basic_entry_values/lldbsuite.test.lldbtest.test_dwarf/a.out (status quo) and lldb-test-build.noindex/functionalities/param_entry_vals/basic_entry_values/lldbsuite.test.lldbtest.test_BasicEntryValues_V5_dwarf/a.out (what this patch would produce).

Btw, this problem isn't really specific to inline tests. The same problem could happen with regular tests if one defined two classes with identically named test methods in the same file. Which means we could try to fix this even more generally by making the default getBuildDirBasename implementation take the class name into account. However, multiple classes in a single file are fairly strange, and this solution would make the build dir paths even longer than they are now, so fixing it this way may not be worth it...

lldb/packages/Python/lldbsuite/test/lldbinline.py

209

__inline_name__ is not a good name according to https://www.python.org/dev/peps/pep-0008/:

__double_leading_and_trailing_underscore__: "magic" objects or attributes that live in user-controlled namespaces. E.g. __init__, __import__ or __file__. Never invent such names; only use them as documented.

[JDevlieghere]

Implement Pavel's approach

[JDevlieghere]

lldb/packages/Python/lldbsuite/test/lldbinline.py

209

I did it for consistency with __no_debug_info_test__. Based on that PEP we should rename it to __no_debug_info_test?

[labath]

lldb/packages/Python/lldbsuite/test/lldbinline.py

209

I think a single underscore should be enough. I am not sure the mangling magic invoked by a double underscore will work for this case:

Note 2: Name mangling can make certain uses, such as debugging and __getattr__(), less convenient.

The PEP is very cautious about (not) encouraging double underscores, and a single leading underscore already says this is private.

[JDevlieghere]

lldb/packages/Python/lldbsuite/test/lldbinline.py

209

Alright, I'll fix that in a follow-up commit. I just wasn't sure if we relied on it, but if we do we'll find out ;-)

lldb/packages/Python/lldbsuite/test/lldbinline.py

def MakeInlineTest(__file, __globals, decorators=None, name=None,
     if name is None:
         # Derive the test name from the current file name
         file_basename = os.path.basename(__file)
         name, _ = os.path.splitext(file_basename)
 
     test_func = ApplyDecoratorsToFunction(InlineTest._test, decorators)
     # Build the test case
     test_class = type(name, (InlineTest,), dict(test=test_func,
-        name=name, _build_dict=build_dict))
+        name=name, __inline_name__=name, _build_dict=build_dict))
 
     # Add the test case to the globals, and hide InlineTest
     __globals.update({name: test_class})
 
     # Keep track of the original test filename so we report it
     # correctly in test results.
     test_class.test_filename = __file
     test_class.mydir = TestBase.compute_mydir(__file)
     return test_class

lldb/packages/Python/lldbsuite/test/lldbtest.py

 
     def __new__(cls, name, bases, attrs):
         original_testcase = super(
             LLDBTestCaseFactory, cls).__new__(
             cls, name, bases, attrs)
         if original_testcase.NO_DEBUG_INFO_TESTCASE:
             return original_testcase
 
+        # Inline tests have one method named 'test' which means that multiple
+        # inline tests in the same file end up sharing the same build directory
+        # per variant. The '__inline_name__' attribute ensures that the
+        # (unique) test name is used instead.
+        inline_name = None
+        for attrname, attrvalue in attrs.items():
+            if attrname == "__inline_name__":
+                inline_name = "test_" + attrvalue
+
         newattrs = {}
         for attrname, attrvalue in attrs.items():
             if attrname.startswith("test") and not getattr(
                     attrvalue, "__no_debug_info_test__", False):
 
                 # If any debug info categories were explicitly tagged, assume that list to be
                 # authoritative.  If none were specified, try with all debug
                 # info formats.
                 all_dbginfo_categories = set(test_categories.debug_info_categories)
                 categories = set(
                     getattr(
                         attrvalue,
                         "categories",
                         [])) & all_dbginfo_categories
                 if not categories:
                     categories = all_dbginfo_categories
 
                 for cat in categories:
                     @decorators.add_test_categories([cat])
                     @wraps(attrvalue)
                     def test_method(self, attrvalue=attrvalue):
                         return attrvalue(self)
 
-                    method_name = attrname + "_" + cat
+                    test_name = inline_name if inline_name else attrname
+                    method_name = test_name + "_" + cat
                     test_method.__name__ = method_name
                     test_method.debug_info = cat
                     newattrs[method_name] = test_method
 
             else:
                 newattrs[attrname] = attrvalue
         return super(
             LLDBTestCaseFactory,