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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to