[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

[Jim Ingham via lldb-commits]

I thought we weren't talking about why you want this lldb-test, but rather why tests that poke deep into the lldb_private API's are or are not appropriate for your test dingus.  

The thing I worry about is if you start using this for very special purpose "reach deep into the lldb_private API's" type tests, then the command-set of lldb-test is not going to be a limited set of commands that are nicely composible, it is going to be a whole assortment of odd little one-off commands and trying to figure out how to use it is going to get harder and harder over time because it will be so noisy.  So if I were doing this I'd impose some discipline on myself to keep that from happening, and use unit tests for anything that was really esoteric.

But again, I'm unlikely to be the one that implements this so my opinions should have the weight of a kibitzer...

Jim


> On Nov 29, 2017, at 4:33 PM, Zachary Turner <zturner at google.com> wrote:
> 
> Many reasons.
> 
> 1) a test that looks like this:
> 
> ```
> RUN: yaml2obj %p/Inputs/compressed-section.yaml > %t/compressed-section.obj
> RUN: lldb-test %s | FileCheck %s
> 
> create-module -f %t/compressed-section.obj
> dump-module-sections compressed-section.obj
> 
> ; CHECK: Section: .hello_elf
> ; CHECK-NEXT: Size: 9
> ; CHECK-NEXT: Data: Hello ELF
> 
> ; CHECK: Section: .bogus
> ; CHECK-NEXT: Size: 0
> ```
> 
> takes about 1 minute to write, and a couple of seconds to understand, and is 12 lines of code, compared to a unit test which takes significantly longer to write and understand, and is (at least in this case) 37 lines not counting the code to implement the command
> 
> 2) this is a standard test format that is understood by hundreds of developers.
> 
> 3) It does not contribute to build time.
> 
> 4) Any work you do to implement the `create-module` and `dump-module-sections` testing command can later be used by any other test without any additional code.  With a reasonable set of commands you would reach a point where you were rarely having to update the tool.
> 
> 5) Parsing text output is often the most straightforward and easiest way to verify something.  Consider for example a testcommand like "dump-unwind-plan" where you could easily represent an unwind plan in text as a sequence of rules and/or heuristics.  Try doing that in code, for several hundred different test cases of obscure unwind scenarios and I think you'll quickly give up and decide it's more trouble than it's worth.
> 
> On Wed, Nov 29, 2017 at 4:14 PM Jim Ingham <jingham at apple.com> wrote:
> 
> 
> > On Nov 29, 2017, at 3:46 PM, Zachary Turner <zturner at google.com> wrote:
> >
> > FWIW, it can certainly use the SB API where it makes sense, but I think requiring that it only use the SB API would be very limiting and a big mistake.
> >
> > The entire point of a tool such as this is that it allows you to dig deep into internals that would be difficult to access otherwise.
> 
> I'm not sure about that.  Making a test that "digs deep into internals" in this method is almost certainly going to require writing a custom command in lldb-test to poke those API's.  How would this be any easier than writing a unit test?
> 
> Jim
> 
> 
> >
> > On Wed, Nov 29, 2017 at 3:23 PM Jason Molenda <jmolenda at apple.com> wrote:
> >
> >
> > > On Nov 29, 2017, at 2:51 PM, Zachary Turner via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> > >
> > >
> > >
> > > On Wed, Nov 29, 2017 at 1:59 PM Jim Ingham <jingham at apple.com> wrote:
> > > I'm mostly basing this concern on the bad effect this had on gdb all of whose testing was expect based command scraping.  gdb is a tool that's much closer to lldb than any of the compiler tools so that experience seems relevant.  It's been a decade or so since I worked on gdb, but back when I was working on it, you really had to tread very carefully if you wanted to change the output of, say, the break command, or to thread listings, etc, and a bunch of times I just had to bag some cleanup of output I wanted to do because fixing up all the tests was too time consuming.  Because Jason and I had both had this experience when we started working on lldb, we promised ourselves we wouldn't go down this path again...
> > >
> > >
> > > Couple of things:
> > >
> > > 1) I wouldn't dare to use this approach for anything that required interactivity.  If you need to run one command, extract a value from the output, and use that value as input to another command, I think that would be a big mistake.  I have no intention of ever proposing something like that.
> > >
> > > 2) FileCheck is very flexible in the way it matches output and tests can be written so that they are resilient to minor format tweaks.  I have no doubt that with pure regex matching, or with pretty much any other tool, you would have a really bad time.  Of course, that doesn't mean it would be hard to construct an example of a format change that would break a FileCheck test.  But I think it would happen *far* less frequently than it did on GDB.  That said, I still understand your concerns that it's fragile, so...
> > >
> > > 3) I would not be opposed to a tool called lldb-test, which was basically just LLDB with a different, and much more limited set of commands, and was completely non-interactive and would produce output in a format designed for being scraped, and which never had to be changed since it was never presented to the user.
> >
> >
> > 100% agree with #3.  We could go back and forth about using lldb-mi, but I think a specialized driver using SB API, designed for testing, would be a great approach.
> >
> >
>