On Thu, Jan 7, 2021 at 3:37 PM Jim Ingham via lldb-commits <lldb-commits@lists.llvm.org> wrote: > > > > > On Jan 7, 2021, at 2:29 PM, David Blaikie via Phabricator via lldb-commits > > <lldb-commits@lists.llvm.org> wrote: > > > > dblaikie added a comment. > > > > In D94063#2485271 <https://reviews.llvm.org/D94063#2485271>, @labath wrote: > > > >> In D94063#2483546 <https://reviews.llvm.org/D94063#2483546>, @dblaikie > >> wrote: > >> > >>> If it's better to write it using C++ source and custom clang flags I can > >>> do that instead (it'll be an -mllvm flag - looks like there's one other > >>> test that does that: > >>> `lldb/test/API/lang/objc/forward-decl/TestForwardDecl.py: > >>> dict(CFLAGS_EXTRAS="-dwarf-version=5 -mllvm -accel-tables=Dwarf"))`) - > >>> means the test will be a bit more convoluted to tickle the subprogram > >>> ranges, but not much - just takes two functions+function-sections. > >> > >> I certainly wouldn't want to drop the existing test. > > > > Ah, what's the tradeoff between the different test types here? > > This is my take (this has been a contentious issue so I'm sure there are > other takes...): > > The "Shell" tests use pattern matching against the lldb command line output. > They are useful for testing the details of the command interaction. You can > also do that using pexpect in the API tests, but the Python 2.7 version of > pexpect seemed really flakey so we switched to shell tests for this sort of > thing. > > Because you are matching against text output that isn't API, they are less > stable. For instance if we changed anything in the "break set" output, your > test would fail(*). And because you are picking details out of that text, > the tests are less precise. You either have to match more of the command > line than you are actually testing for, which isn't a good practice, or you > run the risk of finding the text you were looking for in a directory path or > other unrelated part of the output. Also they are harder to debug if you > can't reproduce the failure locally, since it isn't easy to add internal > checks/output to the test to try hypotheses. Whenever I have run into > failures of this sort the first thing I do is convert the test to an API > test... > > But the main benefit of the "Shell" tests is that you can write tests without > having to know Python or learn the lldb Python API's. And if you are coming > from clang you already know how FileCheck tests work, so that's a bonus. I > think it's legit to require that folks actually working on lldb learn the SB > API's. But we were persuaded that it wasn't fair to impose that on people > not working on lldb, and yet such folks do sometimes need to write tests for > lldb... So for simple tests, the Shell tests are an okay option. But > really, there's nothing you can do in a Shell test that you can't do in an > API test. > > The "API" tests use the Python SB API's - though they also have the ability > to run commands and do expect type checks on the output so for single > commands they work much as the shell tests do (there's even a FileCheck style > assert IIRC). They are a little more verbose than shell tests (though we've > reduced the boilerplate significantly over the years). And of course you > have to know the SB API's. But for instance, if you wanted to know that a > breakpoint was set on line 5 of foo.c, you can set the breakpoint, then ask > the resultant SBBreakpoint object what it's file & line numbers were > directly. So once you've gotten familiar with the setup, IMO you can write > much higher quality tests with the API tests. > > > Jim > > (*) I am personally not at all in favor of the Shell tests, but that's in > part because back in the day I was asked to make a simple and useful change > to the output of the gdb "break" command but then righting the gdb testsuite > - which is all based on expecting the results of various gdb commands - was > so tedious that we ended up dropping the change instead. I don't want to get > to that place with lldb, but the hope is that as long as we mostly write API > tests, we can avoid encumbering the current command outputs too heavily...
Thanks for the context, I really appreciate it. The aspect I was especially curious about here was less in regards to the mechanics of how the behavior is examined/tested (between shell and SB API) but more in regards to source code versus assembly - where the assembly can more explicitly target some DWARF feature, but isn't especially portable - whereas the source code could be portable to test on different architectures, but might require either more contortions to reliably produce the desired DWARF, or possibly extra compiler flags (that was especialyl of interest since Pavel mentioned these tests could be portable across compilers, so how would I specify any necessary custom flags to get clang to produce the desired DWARF (& the tests would be fairly useless for other compilers without those flags/features, or might require very different ways to produce similarly "interesting" DWARF)) - Dave > > > Jim > > > > >> However, it could be useful to have c++ test too. This one could feature a > >> more complicated executable, and be more open-ended/exploratory and test > >> end-to-end functionality (including compiler integration), instead of a > >> targeted "did we parse DW_AT_ranges correctly" regression test. Then this > >> test could go into the `API` test category, as we have the ability to run > >> those kinds of tests against different compilers. > > > > Does this include support for custom compiler flags (it'd currently take a > > non-official/internal-only llvm flag to create the DW_AT_ranges on a > > subprogram that I have in mind, for instance)? > > > >> However, all of that is strictly optional. > > > > I'll consider it for a separate commit. > > > > > > Repository: > > rG LLVM Github Monorepo > > > > CHANGES SINCE LAST ACTION > > https://reviews.llvm.org/D94063/new/ > > > > https://reviews.llvm.org/D94063 > > > > _______________________________________________ > > lldb-commits mailing list > > lldb-commits@lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > > _______________________________________________ > lldb-commits mailing list > lldb-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
