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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
