kastiglione created this revision.
kastiglione added reviewers: jingham, teemperor, aprantl, JDevlieghere.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
kastiglione requested review of this revision.
Restore ability to call `expect()` with a message and no matcher.
After the changes in D88792 <https://reviews.llvm.org/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 <https://reviews.llvm.org/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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D91193
Files:
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/API/assert_messages_test/TestAssertMessages.py
Index: lldb/test/API/assert_messages_test/TestAssertMessages.py
===================================================================
--- lldb/test/API/assert_messages_test/TestAssertMessages.py
+++ lldb/test/API/assert_messages_test/TestAssertMessages.py
@@ -122,10 +122,6 @@
self.assert_expect_fails_with("any command",
dict(substrs="some substring"),
"substrs must be a collection of strings")
- # Prevent `self.expect("cmd", "substr")`
- self.assert_expect_fails_with("any command",
- dict(msg="some substring"),
- "expect() missing a matcher argument")
# Prevent `self.expect("cmd", "msg", "substr")`
self.assert_expect_fails_with("any command",
dict(msg="a message", patterns="some substring"),
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2388,7 +2388,35 @@
self.assertTrue(cmd_status == 0)
- def expect(
+
+ def expect(self, str, *args, **kwargs):
+ "Validate `expect` arguments before calling `_expect_impl`"
+ # Support success/fail only checks like:
+ # self.expect("lldb command")
+ if not args and not kwargs:
+ return self._expect(str)
+
+ # Prevent this ambiguous mistake:
+ # self.expect("lldb command", "some string")
+ # Instead, this invocation must be changed to either:
+ # self.expect("lldb command", msg="some string")
+ # or use a matcher of some kind, such as:
+ # self.expect("lldb command", substrs=["some string"])
+ if args:
+ needed = ("patterns", "startstr", "endstr", "substrs", "error")
+ assert any(arg in kwargs for arg in needed), \
+ "expect() requires a keyword argument"
+
+ # Check `patterns` and `substrs` are not accidentally given as strings.
+ assert not isinstance(kwargs.get("patterns"), six.string_types), \
+ "patterns must be a collection of strings"
+ assert not isinstance(kwargs.get("substrs"), six.string_types), \
+ "substrs must be a collection of strings"
+
+ return self._expect(str, **kwargs)
+
+
+ def _expect_impl(
self,
str,
msg=None,
@@ -2429,22 +2457,6 @@
set to False, the 'str' is treated as a string to be
matched/not-matched
against the golden input.
"""
- # Catch cases where `expect` has been miscalled. Specifically, prevent
- # this easy to make mistake:
- # self.expect("lldb command", "some substr")
- # The `msg` parameter is used only when a failed match occurs. A failed
- # match can only occur when one of `patterns`, `startstr`, `endstr`, or
- # `substrs` has been given. Thus, if a `msg` is given, it's an error to
- # not also provide one of the matcher parameters.
- if msg and not (patterns or startstr or endstr or substrs or error):
- assert False, "expect() missing a matcher argument"
-
- # Check `patterns` and `substrs` are not accidentally given as strings.
- assert not isinstance(patterns, six.string_types), \
- "patterns must be a collection of strings"
- assert not isinstance(substrs, six.string_types), \
- "substrs must be a collection of strings"
-
trace = (True if traceAlways else trace)
if exe:
Index: lldb/test/API/assert_messages_test/TestAssertMessages.py
===================================================================
--- lldb/test/API/assert_messages_test/TestAssertMessages.py
+++ lldb/test/API/assert_messages_test/TestAssertMessages.py
@@ -122,10 +122,6 @@
self.assert_expect_fails_with("any command",
dict(substrs="some substring"),
"substrs must be a collection of strings")
- # Prevent `self.expect("cmd", "substr")`
- self.assert_expect_fails_with("any command",
- dict(msg="some substring"),
- "expect() missing a matcher argument")
# Prevent `self.expect("cmd", "msg", "substr")`
self.assert_expect_fails_with("any command",
dict(msg="a message", patterns="some substring"),
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2388,7 +2388,35 @@
self.assertTrue(cmd_status == 0)
- def expect(
+
+ def expect(self, str, *args, **kwargs):
+ "Validate `expect` arguments before calling `_expect_impl`"
+ # Support success/fail only checks like:
+ # self.expect("lldb command")
+ if not args and not kwargs:
+ return self._expect(str)
+
+ # Prevent this ambiguous mistake:
+ # self.expect("lldb command", "some string")
+ # Instead, this invocation must be changed to either:
+ # self.expect("lldb command", msg="some string")
+ # or use a matcher of some kind, such as:
+ # self.expect("lldb command", substrs=["some string"])
+ if args:
+ needed = ("patterns", "startstr", "endstr", "substrs", "error")
+ assert any(arg in kwargs for arg in needed), \
+ "expect() requires a keyword argument"
+
+ # Check `patterns` and `substrs` are not accidentally given as strings.
+ assert not isinstance(kwargs.get("patterns"), six.string_types), \
+ "patterns must be a collection of strings"
+ assert not isinstance(kwargs.get("substrs"), six.string_types), \
+ "substrs must be a collection of strings"
+
+ return self._expect(str, **kwargs)
+
+
+ def _expect_impl(
self,
str,
msg=None,
@@ -2429,22 +2457,6 @@
set to False, the 'str' is treated as a string to be matched/not-matched
against the golden input.
"""
- # Catch cases where `expect` has been miscalled. Specifically, prevent
- # this easy to make mistake:
- # self.expect("lldb command", "some substr")
- # The `msg` parameter is used only when a failed match occurs. A failed
- # match can only occur when one of `patterns`, `startstr`, `endstr`, or
- # `substrs` has been given. Thus, if a `msg` is given, it's an error to
- # not also provide one of the matcher parameters.
- if msg and not (patterns or startstr or endstr or substrs or error):
- assert False, "expect() missing a matcher argument"
-
- # Check `patterns` and `substrs` are not accidentally given as strings.
- assert not isinstance(patterns, six.string_types), \
- "patterns must be a collection of strings"
- assert not isinstance(substrs, six.string_types), \
- "substrs must be a collection of strings"
-
trace = (True if traceAlways else trace)
if exe:
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
