This NFC commit is in preparation for https://reviews.llvm.org/D42281 (compile the LLDB tests out-of-tree).
Details
- Reviewers
labath jingham zturner jasonmolenda clayborg davide - Commits
- rG595048f3ecf3: Wrap all references to build artifacts in the LLDB testsuite (NFC)
rLLDB323007: Wrap all references to build artifacts in the LLDB testsuite (NFC)
rL323007: Wrap all references to build artifacts in the LLDB testsuite (NFC)
Diff Detail
Event Timeline
os.path.join(os.getcwd(), os.environ["LLDB_BUILD"], self.mydir, "a.out") is getting quite long. I think we should make a utility function out of that.
Maybe self.getBuildArtifact("a.out") ? We could even make "a.out" the default value of the argument...
packages/Python/lldbsuite/test/android/platform/TestDefaultCacheLineSize.py | ||
---|---|---|
22 | Do we want os.environ["LLDB_BUILD"] to be interpreted relative to os.getcwd(), what's the value of os.getcwd() at the point when this is executed? |
We have a lot of ugly boilerplate in the testsuite. I added:
(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, "Set a breakpoint here", self.main_source_file)
Because that's what a lot of tests did. I converted a handful of tests and then ran out of time.
If we're going to touch how all the tests get started with their executable it would be nice to see how many of them can be easily converted to this interface. I can also add a file & line version of this, though in fact most tests that use file & line breakpoints actually use a source match to get a line number and then feed the line to lldb, so they could easily be converted to this.
Then all the business of locating the binary could be centralized.
I guess it's a little late for this comment, however...
I think lldbutil.run_to_source_breakpoint is definitely worth standardizing on, but as it stands, this patch lgtm as an initial step.
Awesome. This will make flipping the out-of-tree switch much easier.
Making more tests use lldbutil.run_to_source_breakpoint would be nice, but I suspect that cannot be done with a shellscript, so I wouldn't tie that to this.
I didn't intend to block this patch, just to point out that this was really work you shouldn't have had to do, and that as we touch these files we should clean them up so next time we don't have to. It will also make the test files easier to read.
I skimmed through every test*.py file while preparing this patch and noticed that there seem to be 3-4 templates that many tests are derived from. Refactoring all the common code would be very good thing to do. I will try to get some of that done in subsequent patches.
While we're on that topic, I would actually propose going one step further than lldbutil.run_to_source_breakpoint. I would propose creating a new test class (StandardLaunchTestCase ?) . This class would assume a certain executable name (a.out), a certain source file name (main.cpp, but this can be taken from a class property, so it can be overriden to main.mm for objc tests) and a certain breakpoint string.
That way, it can do the "build, set breakpoint, launch, validate stop reason" dance without any user input in the setUp() method, and the test can start with the process primed and ready and just focus on the thing it's testing
Do we want os.environ["LLDB_BUILD"] to be interpreted relative to os.getcwd(), what's the value of os.getcwd() at the point when this is executed?