This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Fine tune expect() validation
AbandonedPublic

Authored by kastiglione on Nov 10 2020, 11:37 AM.

Details

Summary

Restore ability to call expect() with a message and no matcher.

After the changes in D88792, expect can no longer be called as:

self.expect("some command", "some message")

After speaking to @jingham, I now understand that an uncommon use case is to
check only that a command succeeds, without validating its output. The changes
in D88792 prevent such expectations.

This change restores the ability to call expect this way, but with the
requirement that the msg argument be a keyword argument. Requiring msg be a
keyword argument provides the ability to ensure expect's arguments are
explict, and not ambiguous. Example:

self.expect("some command", msg="some message")

When lldb supports only Python 3, the easy solution will be to change the
signature of expect by require all arguments be keyword ags using its *
syntax:

def expect(
        self,
        str,
        *,
        msg=None,
        patterns=None,
        startstr=None,
        endstr=None,
        substrs=None,
        trace=False,
        error=False,
        ordered=True,
        matching=True,
        exe=True,
        inHistory=False):

But the * syntax is not supported in Python 2. To work around this, this change renames expect to _expect_impl, and implements expect as a separate function with a signature that separates positional args from keyword args, in order to perform validation.

Diff Detail

Event Timeline

kastiglione created this revision.Nov 10 2020, 11:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 10 2020, 11:37 AM
kastiglione requested review of this revision.Nov 10 2020, 11:37 AM

Isn't that what we use runCmd for?

@teemperor maybe? Do you mean validation should go in runCmd? Or do you mean something else?

@teemperor maybe? Do you mean validation should go in runCmd? Or do you mean something else?

I meant that to my knowledge command execution where the only expectation is that they succeed (so, no output checking) are supposed to be done via runCmd. expect is also documented as Similar to runCmd; with additional expect style output matching ability., so if we allow calling expect without the additional expect style matching ability, then we might as well remove runCmd. So IMHO the current implementation of expect is just fine.

That's much better. I'll change those callers can be changed to runCmd.

kastiglione abandoned this revision.Nov 10 2020, 4:42 PM
labath added a subscriber: labath.Nov 11 2020, 12:15 AM

Yes, runCmd is the way to go here...

Thanks for pointing this in the right direction @labath, @teemperor.