> On Aug 15, 2018, at 12:12 PM, Jason Molenda <jmole...@apple.com> wrote: > > > >> On Aug 15, 2018, at 11:34 AM, Vedant Kumar <v...@apple.com> wrote: >> >> >> >>> On Aug 14, 2018, at 6:19 PM, Jason Molenda <jmole...@apple.com> wrote: >>> >>> It's more verbose, and it does mean test writers need to learn the public >>> API, but it's also much more stable and debuggable in the future. >> >> I'm not sure about this. Having looked at failing sb api tests for a while >> now, I find them about as easy to navigate and fix as FileCheck tests in >> llvm. > > I don't find that to be true. I see a failing test on line 79 or whatever, > and depending on what line 79 is doing, I'll throw in some > self.runCmd("bt")'s or self.runCmd("fr v") to the test, re-run, and see what > the relevant context is quickly. For most simple tests, I can usually spot > the issue in under a minute. dotest.py likes to eat output when it's run in > multiprocess mode these days, so I have to remember to add --no-multiprocess. > If I'm adding something that I think is generally useful to debug the test > case, I'll add a conditional block testing again self.TraceOn() and print > things that may help people who are running dotest.py with -t trace mode > enabled.
I do agree that there are effective ways of debugging sb api tests. Having worked with plenty of filecheck-based tests in llvm/clang/swift, I find them to be as easy (or easier for me personally) to debug. > Sometimes there is a test written so it has a "verify this value" function > that is run over a variety of different variables during the test timeframe, > and debugging that can take a little more work to understand the context that > is failing. But that kind of test would be harder (or at least much more > redundant) to express in a FileCheck style system anyway, so I can't ding it. Yep, sounds like a great candidate for a unit test or an SB API test. > As for the difficulty of writing SB API tests, you do need to know the > general architecture of lldb (a target has a process, a process has threads, > a thread has frames, a frame has variables), the public API which quickly > becomes second nature because it is so regular, and then there's the > testsuite specific setup and template code. But is that that intimidating to > anyone familiar with lldb? Not intimidating, no. Cumbersome and slow, absolutely. So much so that I don't see a way of adequately testing my patches this way. It would just take too much time. vedant > packages/Python/lldbsuite/test/sample_test/TestSampleTest.py is 50 lines > including comments; there's about ten lines of source related to initializing > / setting up the testsuite, and then 6 lines is what's needed to run to a > breakpoint, get a local variable, check the value. > > > J > > > >> >> >>> It's a higher up front cost but we're paid back in being able to develop >>> lldb more quickly in the future, where our published API behaviors are >>> being tested directly, and the things that must not be broken. >> >> I think the right solution here is to require API tests when new >> functionality is introduced. We can enforce this during code review. Making >> it impossible to write tests against the driver's output doesn't seem like >> the best solution. It means that far fewer tests will be written (note that >> a test suite run of lldb gives less than 60% code coverage). It also means >> that the driver's output isn't tested as much as it should be. >> >> >>> The lldb driver's output isn't a contract, and treating it like one makes >>> the debugger harder to innovate in the future. >> >> I appreciate your experience with this (pattern matching on driver input) in >> gdb. That said, I think there are reliable/maintainable ways to do this, and >> proven examples we can learn from in llvm/clang/etc. >> >> >>> It's also helpful when adding new features to ensure you've exposed the >>> feature through the API sufficiently. The first thing I thought to try >>> when writing the example below was SBFrame::IsArtificial() (see >>> SBFrame::IsInlined()) which doesn't exist. If a driver / IDE is going to >>> visually indicate artificial frames, they'll need that. >> >> Sure. That's true, we do need API exposure for new features, and again we >> can enforce that during code review. The reason you didn't find >> IsArtificial() is because it's sitting on my disk :). Haven't shared the >> patch yet. >> >> vedant >> >>> >>> J >>> >>>> On Aug 14, 2018, at 5:56 PM, Vedant Kumar <v...@apple.com> wrote: >>>> >>>> It'd be easy to update FileCheck tests when changing the debugger (this >>>> happens all the time in clang/swift). OTOH, the verbosity of the python >>>> API means that fewer tests get written. I see a real need to make >>>> expressive tests easier to write. >>>> >>>> vedant >>>> >>>>> On Aug 14, 2018, at 5:38 PM, Jason Molenda <jmole...@apple.com> wrote: >>>>> >>>>> I'd argue against this approach because it's exactly why the lit tests >>>>> don't run against the lldb driver -- they're hardcoding the output of the >>>>> lldb driver command into the testsuite and these will eventually make it >>>>> much more difficult to change and improve the driver as we've accumulated >>>>> this style of test. >>>>> >>>>> This is a perfect test for a normal SB API. Run to your breakpoints and >>>>> check the stack frames. >>>>> >>>>> f0 = thread.GetFrameAtIndex(0) >>>>> check that f0.GetFunctionName() == sink >>>>> check that f0.IsArtifical() == True >>>>> check that f0.GetLineEntry().GetLine() == expected line number >>>>> >>>>> >>>>> it's more verbose, but it's also much more explicit about what it's >>>>> checking, and easy to see what has changed if there is a failure. >>>>> >>>>> >>>>> J >>>>> >>>>>> On Aug 14, 2018, at 5:31 PM, Vedant Kumar via lldb-dev >>>>>> <lldb-dev@lists.llvm.org> wrote: >>>>>> >>>>>> Hello, >>>>>> >>>>>> I'd like to make FileCheck available within lldb inline tests, in >>>>>> addition to existing helpers like 'runCmd' and 'expect'. >>>>>> >>>>>> My motivation is that several tests I'm working on can't be made as >>>>>> rigorous as they need to be without FileCheck-style checks. In >>>>>> particular, the 'matching', 'substrs', and 'patterns' arguments to >>>>>> runCmd/expect don't allow me to verify the ordering of checked input, to >>>>>> be stringent about line numbers, or to capture & reuse snippets of text >>>>>> from the input stream. >>>>>> >>>>>> I'd curious to know if anyone else is interested or would be willing to >>>>>> review this (https://reviews.llvm.org/D50751). >>>>>> >>>>>> Here's an example of an inline test which benefits from FileCheck-style >>>>>> checking. This test is trying to check that certain frames appear in a >>>>>> backtrace when stopped inside of the "sink" function. Notice that >>>>>> without FileCheck, it's not possible to verify the order in which frames >>>>>> are printed, and that dealing with line numbers would be cumbersome. >>>>>> >>>>>> ``` >>>>>> --- >>>>>> a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp >>>>>> +++ >>>>>> b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp >>>>>> @@ -9,16 +9,21 @@ >>>>>> >>>>>> volatile int x; >>>>>> >>>>>> +// CHECK: frame #0: {{.*}}sink() at main.cpp:[[@LINE+2]] [opt] >>>>>> void __attribute__((noinline)) sink() { >>>>>> - x++; //% self.expect("bt", substrs = ['main', 'func1', 'func2', >>>>>> 'func3', 'sink']) >>>>>> + x++; //% self.filecheck("bt", "main.cpp") >>>>>> } >>>>>> >>>>>> +// CHECK-NEXT: frame #1: {{.*}}func3() {{.*}}[opt] [artificial] >>>>>> void __attribute__((noinline)) func3() { sink(); /* tail */ } >>>>>> >>>>>> +// CHECK-NEXT: frame #2: {{.*}}func2() at main.cpp:[[@LINE+1]] [opt] >>>>>> void __attribute__((disable_tail_calls, noinline)) func2() { func3(); /* >>>>>> regular */ } >>>>>> >>>>>> +// CHECK-NEXT: frame #3: {{.*}}func1() {{.*}}[opt] [artificial] >>>>>> void __attribute__((noinline)) func1() { func2(); /* tail */ } >>>>>> >>>>>> +// CHECK-NEXT: frame #4: {{.*}}main at main.cpp:[[@LINE+2]] [opt] >>>>>> int __attribute__((disable_tail_calls)) main() { >>>>>> func1(); /* regular */ >>>>>> return 0; >>>>>> ``` >>>>>> >>>>>> For reference, here's the output of the "bt" command: >>>>>> >>>>>> ``` >>>>>> runCmd: bt >>>>>> output: * thread #1, queue = 'com.apple.main-thread', stop reason = >>>>>> breakpoint 1.1 >>>>>> * frame #0: 0x000000010c6a6f64 a.out`sink() at main.cpp:14 [opt] >>>>>> frame #1: 0x000000010c6a6f70 a.out`func3() at main.cpp:15 [opt] >>>>>> [artificial] >>>>>> frame #2: 0x000000010c6a6f89 a.out`func2() at main.cpp:21 [opt] >>>>>> frame #3: 0x000000010c6a6f90 a.out`func1() at main.cpp:21 [opt] >>>>>> [artificial] >>>>>> frame #4: 0x000000010c6a6fa9 a.out`main at main.cpp:28 [opt] >>>>>> ``` >>>>>> >>>>>> thanks, >>>>>> vedant >>>>>> _______________________________________________ >>>>>> lldb-dev mailing list >>>>>> lldb-dev@lists.llvm.org >>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >>>>> >>>> >>> >> > _______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev