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

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

> I am not sure this actually creates enough separation.

That's a good point. If I manage to extract the testname somehow via Python reflection magic, I could also get rid of the unintuitive self.mydir tuple. I'll give that a try.



================
Comment at: packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py:39-40
             (_COMP_DIR_SYM_LINK_PROP, pwd_symlink))
-        lldbutil.run_break_set_by_file_and_line(self, self.src_path, self.line)
+        src_path = self.getBuildArtifact(_SRC_FILE)
+        lldbutil.run_break_set_by_file_and_line(self, src_path, self.line)
 
----------------
xiaobai wrote:
> Instead of making a temporary variable `src_path` that gets used once in each instance, why not just insert `self.getBuildArtifact(_SRC_FILE)` directly into `lldbutil.run_break_set_by_file_and_line()`?
While most tests have their sources in the source directory, some use generated source file, which are located in the build directory, so that is unfortunately not an option.


================
Comment at: packages/Python/lldbsuite/test/lldbinline.py:96-98
+            return "-N dwarf %s" % (testdir)
         else:
+            return "-N dsym %s" % (testdir)
----------------
labath wrote:
> 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).
In this case I think this should just be `return "-N dsym " + testdir`


https://reviews.llvm.org/D42763