> 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
