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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: packages/Python/lldbsuite/test/lldbinline.py:96-98
+            return "-N dwarf %s" % (testdir)
         else:
+            return "-N dsym %s" % (testdir)
----------------
labath wrote:
> aprantl wrote:
> > 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`
> :D 
+1. I agree with Pavel that we should adopt the new formatters. Let's be idiomatic :-) 


================
Comment at: packages/Python/lldbsuite/test/lldbtest.py:728
+        if not os.path.isdir(path):
+            raise OSError(errno.ENOTDIR, "%s is not a directory"%path)
  
----------------
`"{} is not a directory".format(path)`


https://reviews.llvm.org/D42763