> On Aug 14, 2018, at 6:39 PM, Zachary Turner <ztur...@google.com> wrote:
> 
> Having bugs also makes the debugger harder to innovate in the future because 
> it’s, not having tests leads to having bugs, and sb api tests leads to not 
> having tests.

Yeah, this might be a bit of an under-appreciated point.

If there's a high up front cost to writing tests, not as many of them will be 
written.

I'm having a hard time imagining writing sb api tests which can exercise every 
tricky path through a new patchset I'm working on. Without being able to write 
tests against more compact input (a la FileCheck), I really don't see a way 
forward.


> At the end of the day, it doesn’t matter how stable the tests are if there 
> arent enough of them. There should be about 10x-20x as many tests as there 
> are currently, and that will simply never happen under the current approach. 
> If it means we need to have multiple different styles of test, so be it. The 
> problem we face right now has nothing to do with command output changing, and 
> in fact I don’t that we’ve *ever* had this problem. So we should be focusing 
> on problems we have, not problems we don’t have.
> 
> Note that it is not strictly necessary for a test to check the debuggers 
> command output. There could be a different set of commands whose only purpose 
> is to print information for the purposes of debugging. One idea would be to 
> introduce the notion of a debugger object model, where you print various 
> aspects of the debuggers state with an object like syntax. For example,
> 
> (lldb) p debugger.targets
> ~/foo (running, pid: 123)
> 
> (lldb) p debugger.targets[0].threads[0].frames[1]
> int main(int argc=3, char **argv=0x12345678) + 0x72
> 
> (lldb) p debugger.targets[0].threads[0].frames[1].params[0]
> int argc=3
> 
> (lldb) p debugger.targets[0].breakpoints
> [1] main.cpp:72
> 
> Etc. you can get arbitrarily granular and dxpose every detail of the 
> debuggers internal state this way, and the output is so simple that you never 
> have to worry about it changing.
> 
> That said, I think history has shown that limiting ourselves to sb api tests, 
> despite all the theoretical benefits, leads to insufficient test coverage. So 
> while it has benefits, it also has problems for which we need a better 
> solution

Yeah.

vedant

> On Tue, Aug 14, 2018 at 6:19 PM Jason Molenda via lldb-dev 
> <lldb-dev@lists.llvm.org <mailto:lldb-dev@lists.llvm.org>> 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.  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.  The lldb driver's output 
> isn't a contract, and treating it like one makes the debugger harder to 
> innovate in the future.
> 
> 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.
> 
> J
> 
> > On Aug 14, 2018, at 5:56 PM, Vedant Kumar <v...@apple.com 
> > <mailto: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 
> >> <mailto: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 <mailto: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 
> >>> <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 <mailto:lldb-dev@lists.llvm.org>
> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev 
> >>> <http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev>
> >> 
> > 
> 
> _______________________________________________
> lldb-dev mailing list
> lldb-dev@lists.llvm.org <mailto:lldb-dev@lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev 
> <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

Reply via email to