[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I am not sure this actually creates enough separation. Plenty of tests have more then one test method per file:

  TestFoo.py:
  class FooTestCase(TestBase):
    NO_DEBUG_INFO_TESTCASE = True
    def test_bar(self):
      ...
    def test_baz(self):
      ...

If I understand correctly, you patch will run both `test_bar` and `test_baz` in the same build folder (TestFoo.default). Shouldn't we separate those as well ? (e.g. `TestFoo.test_bar`, `TestFoo.test_baz`) ?

The good thing about this is that you don't need to worry about debug info variants in this code at all. You just take the test name, and the debug info will be encoded in there (if I drop the NO_DEBUG_INFO_TESTCASE, the metaclass will "magically" create test methods "test_bar_dwarf", "test_bar_dwo", ...)

(Technically that still leaves the possibility that a single file will have two classes, and each of them will have the same method name, but no test currently does that, so we can just disallow that).



================
Comment at: packages/Python/lldbsuite/test/lldbinline.py:96-98
+            return "-N dwarf %s" % (testdir)
         else:
+            return "-N dsym %s" % (testdir)
----------------
xiaobai wrote:
> Good opportunity to move away from the old style of formatting strings to a newer style? It doesn't make a huge difference, I just think it'd be nice to do. :)
> `return "-N dwarf {}".format(testdir)`
> `return "-N dwarf {}".format(testdir)`
> 
> This is supported in Python 2.7, but I'm not sure if we can assume that version or greater to be present.
2.7 is definitely fine, and we use `{}` format in plently of places already. (But I don't have any beef with the %s format either).


https://reviews.llvm.org/D42763