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