[Lldb-commits] [PATCH] D81516: [lldb/Test] Ensure inline tests have a unique build directory

[Pavel Labath via Phabricator via lldb-commits]

labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.

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...



================
Comment at: lldb/packages/Python/lldbsuite/test/lldbinline.py:209
     test_class = type(name, (InlineTest,), dict(test=test_func,
-        name=name, _build_dict=build_dict))
+        name=name, __inline_name__=name, _build_dict=build_dict))
 
----------------
`__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.
```



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81516/new/

https://reviews.llvm.org/D81516