labath added inline comments.
================ Comment at: packages/Python/lldbsuite/test/api/listeners/TestListener.py:26 TestBase.setUp(self) - self.build() ---------------- aprantl wrote: > labath wrote: > > I'm confused by these changes. I was under the impression that setUp() runs > > before each test method (and hence this should be NFC). Can you explain the > > reasoning behind this? > (see also my previous comment) > `setUp` runs before everything else, but it runs once per testcase and > `self.testMethodName` is not initialized yet. Therefore we can only run > `self.build()` in `self.setUp()` in NO_DEBUG_INFO_TESTCASEs. I'm not sure where you got the idea that testMethodName is not initialized. It's initialized as one of the first things in Base.setUp (lldbtest.py:776). This also demonstrates that the setUp method is run once *per test function* (as otherwise testMethodName would make no sense; also check out third_party/Python/module/unittest2/unittest2/case.py:379 to see how tests are run). The problem I think you are having is that self.**debug_info** is not initialized. I was curious at hard it would be to make this work, so I played around with it a bit and the result was D42836. It's not that I think this change is that bad (although I would prefer if we could keep doing these things in setUp), but I think this could also help make happen the ideas about building binaries once per test case (class) -- the way I'd do that is that I would enumerate all variants that are needed to build in setUpClass (and build them); then the actual test could fetch the corresponding binary according to the variant. ================ Comment at: packages/Python/lldbsuite/test/lldbtest.py:721-728 + try: + os.makedirs(path) + except OSError as e: + import errno + if e.errno != errno.EEXIST: + raise + if not os.path.isdir(path): ---------------- Can we use the (new) lldbutil.mkdir_p here? If there's some circular dependency, maybe we could move the function somewhere else? ================ Comment at: packages/Python/lldbsuite/test/lldbtest.py:1517-1518 testdir = self.mydir + if not testname: + testname = self.testMethodName if self.debug_info: ---------------- Why was this necessary? Could we either always pass the name by argument, or always fetch it from `self`? Maybe it will be possible after `D42836`? https://reviews.llvm.org/D42763 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits