Thanks for the context. On Tue, Sep 19, 2017 at 11:58 AM, Jim Ingham <jing...@apple.com> wrote:
> > > On Sep 19, 2017, at 11:25 AM, Leonard Mosescu <mose...@google.com> > wrote: > > > > These are all great suggestions, thanks everyone! > > > > > We should have a test. The test would need to use pexpect or something > similar. If anyone has any pointer on how to make a test for this, please > chime in. I was thinking just a pexpect based test. > > I love tests but what exactly do we want to test here? Sorry if I'm > missing something obvious - are you talking about splitting a string into > chunks? The actual interruption? ... > > > > Also, I like the idea of exposing this though the SB APIs, but that's a > significant expansion of the original goal which is to improve the > interactive LLDB debugger. > > This is sort of a side point, but one of the goals of lldb is to make the > command set extensible by adding commands implemented with the SB API's > either from Python or from C++ (Python is more common but both are > possible.) And there are a bunch of groups here at Apple that have all > sorts of special purpose LLDB commands implemented this way. The xnu.py > and Co. that gets distributed with the kernel development kit has a wide > array of examples of this that are publicly available if you want to have a > look. > > So the SB API's are in fact an intrinsic part of the interactive LLDB > debugger. We aren't there yet (there isn't an SB API to define command > options & arguments so SB added commands don't parse like lldb commands do) > but the end goal is that it you can't tell whether commands are from SB > API's or are builtin. > > BTW, I think the fact that we use lldb_private API's is an unfortunate > artifact of how lldb was implemented. Having to use our vended API's to > implement the commands would make them better implemented - since we would > really be on the hook for them, and would reduce a lot of code duplication > where we do things one way in the commands and slightly differently in the > equivalent SB API's, and reduce the testing surface considerably. I'm not > sure this is a project we will ever actually get to, but it is a good > aspirational goal to keep in mind. > > We didn't do it that way originally because we needed something we could > poke at early on in the development of lldb, at a point where it would have > been premature to design the SB API's. Then lldb went from skunkworks > project to mission critical debugger too quickly for us to be able to > revise the decision. > > Jim > > > > > It may be nice looking at SB APIs later on, but I'm afraid that trying > to pile up everything in one change is just going to spiral the complexity > out of control. > > > > On Tue, Sep 19, 2017 at 11:15 AM, Zachary Turner <ztur...@google.com> > wrote: > > That would work, but I think it's adding many more pieces to the test. > Now there's threads, a Python layer, and multiprocess dotest infrastructure > in the equation. Each providing an additional source of flakiness and > instability. > > > > If all it is is a pass-through, then just calling the function it passes > through to is a strictly better test, since it's focusing the test on the > underlying piece of functionality and removing additional sources of > failure and instability from the test. > > > > On Tue, Sep 19, 2017 at 11:02 AM Jim Ingham <jing...@apple.com> wrote: > > I'd really prefer to do this as/along with an SB API test since we also > need commands made through the SB API to be interruptible, and we should > test that that also works. So this kills two birds with one stone. > > > > In general, when developing any high-level features in lldb one of the > questions you should ask yourself is whether this is useful at the SB API > layer. In this case it obviously is (actually I'm a little embarrassed I > didn't think of this requirement). If so, then it's simplest to test it at > that layer. If the SB API is anything more than a pass-through, then you > might also want to write a unit test for the underlying C++ API's. But in > this case the SB function will just call the underlying CommandInterpreter > one, so testing at the SB layer is sufficient. > > > > Jim > > > > > On Sep 19, 2017, at 10:45 AM, Zachary Turner via Phabricator < > revi...@reviews.llvm.org> wrote: > > > > > > zturner added a comment. > > > > > > In https://reviews.llvm.org/D37923#875322, @clayborg wrote: > > > > > >> We should have a test. The test would need to use pexpect or > something similar. If anyone has any pointer on how to make a test for > this, please chime in. I was thinking just a pexpect based test. > > > > > > > > > This kind of thing is perfect for a unit test, but I'm not sure how > easy that would be with the current design. You'd basically do something > like: > > > > > > struct MockStream { > > > explicit MockStream(CommandInterpreter &Interpreter, int > InterruptAfter) > > > : CommandInterpreter(Interpreter), > InterruptAfter(InterruptAfter) {} > > > > > > CommandInterpreter &Interpreter; > > > const int InterruptAfter; > > > int Lines = 0; > > > std::string Buffer; > > > > > > void Write(StringRef S) { > > > ++Lines; > > > if (Lines >= InterruptAfter) { > > > Interpreter.Interrupt(); > > > return; > > > } > > > Buffer += S; > > > } > > > }; > > > > > > TEST_F(CommandInterruption) { > > > CommandInterpreter Interpreter; > > > MockStream Stream(Interpreter, 3); > > > Interpreter.PrintCommandOutput(Stream, "a\nb\nc\nd\ne\nf\n"); > > > EXPECT_EQ(Stream.Lines == 3); > > > EXPECT_EQ(Stream.Buffer == "a\nb\nc\n"); > > > } > > > > > > > > > https://reviews.llvm.org/D37923 > > > > > > > > > > > > > > >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
