Page MenuHomePhabricator

[lldb/Docs] Add some more info about the test suite layout
ClosedPublic

Authored by JDevlieghere on Apr 15 2020, 1:47 PM.

Details

Summary

I gave a short presentation internally with an overview of our test suite. Adrian suggested converting (part of it) for the lldb website, which I thought was a great idea.

Diff Detail

Event Timeline

JDevlieghere created this revision.Apr 15 2020, 1:47 PM

This is great!

lldb/docs/resources/test.rst
15

th*r*ough

what does "which is binary" mean?

32

located

70

We should also mention that the API tests will build the inferior for many different configurations

90

mention that these are end-to-end tests that compile programs from source, run them, and debug the processes?

102

mention that there is an an actual, literal, example test, too?

150

double space

JDevlieghere marked 6 inline comments as done.

Thanks for the review!

jankratochvil added inline comments.Apr 15 2020, 2:35 PM
lldb/docs/resources/test.rst
18
21

s/are are/are/

22
43

s#lldb/test/shell#lldb/test/Shell#

44

s/build/built/

45
58

s/```/``/

59
102
107

s/fictive tests/fictive test/

115
120

Some example: @expectedFailureAll(archs=["aarch64"], oslist=["linux"])

125

Probably: @expectedFailure(checking_function_name)

129

s/tests/test/

134

s/things/thing/

154

expressivity?

JDevlieghere marked 17 inline comments as done.

Thanks for the review, Jan!

lldb/docs/resources/test.rst
45

You can't combine preformatted text with links, so I've added the link above and kept it preformatted here.

59

Same issue as mentioned earlier. Let me know if you prefer the link over the preformatted text.

jankratochvil marked an inline comment as done.Apr 15 2020, 3:44 PM
jankratochvil added inline comments.
lldb/docs/resources/test.rst
45

OK, sure I am fine with that, thanks.

59

Up to you but I find it convenient for lazy readers to have the links there to get more involved into the framework.

jankratochvil accepted this revision.Apr 15 2020, 4:02 PM
This revision is now accepted and ready to land.Apr 15 2020, 4:02 PM

This is great, thanks for doing this. You point people at lldbutils.py as a source for utility functions (like print_stacktrace, etc...) But it is maybe also worth pointing them at the lldbtest.py for extensions runCmd and some of the other testing primitives we've added to the unittest2 framework.

lldb/docs/resources/test.rst
119

doubly

labath accepted this revision.Apr 16 2020, 2:11 AM

I think this is a really good writeup. Thanks for doing this.

lldb/docs/resources/test.rst
156–157

As we usually maintain our own tests, I would say "maintenance time" is a subset of "developer time". I guess you wanted to say something like that it's easy to set up, but increases the runtime of the tests and has a large ongoing cost.

160

s/prevent/remove (obviate) ?

Another interesting aspect of the "shell" tests is that there you can often get away with a broken/incomplete binary, whereas the "api" tests almost always require a fully functional executable. This enables testing of (some) aspects of handling of binaries with non-native architectures or operating systems. I see this as a big advantage of the shell tests. And a bit of a missed opportunity too -- we don't have many tests like this right now, which means is fairly hard to contribute something to lldb without having access to mac, linux, and windows machines (arm boxes are useful too, at times)...

kwk added a subscriber: kwk.EditedApr 16 2020, 6:01 AM

This looks good overall. I would add a section describing which test suite to use when you're interested in a particular DWARF feature for example. I heard from my GDB colleagues that the don't use a compiler, because that might change and produce a different DWARF. Instead they use a DWARF assembler (IIRC). I guess the analogy to LLDB would be yaml2obj which I see not mentioned here. Does it make sense to at least reference it in a section?

I forgot to mention that I'd love to have this at hand when I began developing for LLDB. So thank you very much for writing this up!!!

lldb/docs/resources/test.rst
86

s/Thy/They

88

I personally struggled with the way inferiors are being build. The Makefile includes another Makefile and was more like a framework than simple make. You had to set special variables in order for the included Makefile to pick it up. That level of indirection made it quite complicated for me to get what I wanted. To put it differently, it would be nice if you could describe what the Makefile should look like or what is expected.

94

s/begin/being

95

s/sources/source
s/file/files

122
142

Ah, there you're mentioning it.

kwk awarded a token.Apr 16 2020, 6:04 AM
JDevlieghere marked 11 inline comments as done.Apr 16 2020, 10:02 AM

Thanks a lot for the feedback everyone!

kwk added a comment.Apr 20 2020, 10:30 PM

I've marked some mistakes that were not addressed yet but are marked as "Done".

lldb/docs/resources/test.rst
86

Not done yet.

94

Not done yet.

95

Not done yet.

In D78242#1993781, @kwk wrote:

I've marked some mistakes that were not addressed yet but are marked as "Done".

The diff wasn't updated with the committed changes because I forgot the Differential Revision: trailer. If you look at https://reviews.llvm.org/rG3a6b60fa623da6e311b61c812932917085067cd3 you'll see they're fixed :-)