[Lldb-commits] [lldb] 010d7a3 - [lldb/test] Catch invalid calls to expect()

Dave Lee via lldb-commits lldb-commits at lists.llvm.org
Mon Oct 5 12:42:06 PDT 2020


Author: Dave Lee
Date: 2020-10-05T12:41:52-07:00
New Revision: 010d7a388b146cafaf4bc0b28b952d5852d62b6a

URL: https://github.com/llvm/llvm-project/commit/010d7a388b146cafaf4bc0b28b952d5852d62b6a
DIFF: https://github.com/llvm/llvm-project/commit/010d7a388b146cafaf4bc0b28b952d5852d62b6a.diff

LOG: [lldb/test] Catch invalid calls to expect()

Add preconditions to `TestBase.expect()` that catch semantically invalid calls
that happen to succeed anyway. This also fixes the broken callsites caught by
these checks.

This prevents the following incorrect calls:

1. `self.expect("lldb command", "some substr")`
2. `self.expect("lldb command", "assert message", "some substr")`

Differential Revision: https://reviews.llvm.org/D88792

Added: 
    

Modified: 
    lldb/packages/Python/lldbsuite/test/lldbtest.py
    lldb/test/API/assert_messages_test/TestAssertMessages.py
    lldb/test/API/commands/frame/diagnose/bad-reference/TestBadReference.py
    lldb/test/API/commands/frame/diagnose/complicated-expression/TestComplicatedExpression.py
    lldb/test/API/commands/frame/diagnose/dereference-argument/TestDiagnoseDereferenceArgument.py
    lldb/test/API/commands/frame/diagnose/dereference-this/TestDiagnoseDereferenceThis.py
    lldb/test/API/commands/frame/diagnose/inheritance/TestDiagnoseInheritance.py
    lldb/test/API/commands/frame/diagnose/local-variable/TestLocalVariable.py
    lldb/test/API/commands/frame/diagnose/virtual-method-call/TestDiagnoseDereferenceVirtualMethodCall.py
    lldb/test/API/commands/settings/TestSettings.py
    lldb/test/API/driver/batch_mode/TestBatchMode.py
    lldb/test/API/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py
    lldb/test/API/functionalities/data-formatter/data-formatter-objc/cmtime/TestDataFormatterCMTime.py
    lldb/test/API/lang/cpp/constructors/TestCppConstructors.py
    lldb/test/API/macosx/macCatalyst/TestMacCatalyst.py
    lldb/test/API/types/TestRecursiveTypes.py

Removed: 
    


################################################################################
diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 2ee82295c553..2309b403cb99 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2424,6 +2424,22 @@ def expect(
         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:

diff  --git a/lldb/test/API/assert_messages_test/TestAssertMessages.py b/lldb/test/API/assert_messages_test/TestAssertMessages.py
index 6619a65ad69e..f8b6b33f297c 100644
--- a/lldb/test/API/assert_messages_test/TestAssertMessages.py
+++ b/lldb/test/API/assert_messages_test/TestAssertMessages.py
@@ -113,3 +113,20 @@ def test_expect(self):
 
                Expecting start string: "cat" (was not found)
                Reason for check goes here!""")
+
+        # Verify expect() preconditions.
+        # Both `patterns` and `substrs` cannot be of type string.
+        self.assert_expect_fails_with("any command",
+            dict(patterns="some substring"),
+            "patterns must be a collection of strings")
+        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"),
+            "must be a collection of strings")

diff  --git a/lldb/test/API/commands/frame/diagnose/bad-reference/TestBadReference.py b/lldb/test/API/commands/frame/diagnose/bad-reference/TestBadReference.py
index 737b297ed76b..2ed417be8781 100644
--- a/lldb/test/API/commands/frame/diagnose/bad-reference/TestBadReference.py
+++ b/lldb/test/API/commands/frame/diagnose/bad-reference/TestBadReference.py
@@ -22,4 +22,5 @@ def test_bad_reference(self):
         self.runCmd("run", RUN_SUCCEEDED)
         self.expect("thread list", "Thread should be stopped",
                     substrs=['stopped'])
-        self.expect("frame diagnose", "Crash diagnosis was accurate", "f->b")
+        self.expect("frame diagnose", "Crash diagnosis was accurate",
+                    substrs=["f->b"])

diff  --git a/lldb/test/API/commands/frame/diagnose/complicated-expression/TestComplicatedExpression.py b/lldb/test/API/commands/frame/diagnose/complicated-expression/TestComplicatedExpression.py
index 277fafd14b57..c1b0a0b61f47 100644
--- a/lldb/test/API/commands/frame/diagnose/complicated-expression/TestComplicatedExpression.py
+++ b/lldb/test/API/commands/frame/diagnose/complicated-expression/TestComplicatedExpression.py
@@ -25,4 +25,4 @@ def test_diagnose_dereference_argument(self):
         self.expect(
             "frame diagnose",
             "Crash diagnosis was accurate",
-            "f->b->d")
+            substrs=["f->b->d"])

diff  --git a/lldb/test/API/commands/frame/diagnose/dereference-argument/TestDiagnoseDereferenceArgument.py b/lldb/test/API/commands/frame/diagnose/dereference-argument/TestDiagnoseDereferenceArgument.py
index 5d5b3a0cf17f..e6f222a89c62 100644
--- a/lldb/test/API/commands/frame/diagnose/dereference-argument/TestDiagnoseDereferenceArgument.py
+++ b/lldb/test/API/commands/frame/diagnose/dereference-argument/TestDiagnoseDereferenceArgument.py
@@ -25,4 +25,4 @@ def test_diagnose_dereference_argument(self):
         self.expect(
             "frame diagnose",
             "Crash diagnosis was accurate",
-            "f->b->d")
+            substrs=["f->b->d"])

diff  --git a/lldb/test/API/commands/frame/diagnose/dereference-this/TestDiagnoseDereferenceThis.py b/lldb/test/API/commands/frame/diagnose/dereference-this/TestDiagnoseDereferenceThis.py
index b1f6b2c87943..e5d528fe3422 100644
--- a/lldb/test/API/commands/frame/diagnose/dereference-this/TestDiagnoseDereferenceThis.py
+++ b/lldb/test/API/commands/frame/diagnose/dereference-this/TestDiagnoseDereferenceThis.py
@@ -25,4 +25,4 @@ def test_diagnose_dereference_this(self):
         self.expect(
             "frame diagnose",
             "Crash diagnosis was accurate",
-            "this->a")
+            substrs=["this->a"])

diff  --git a/lldb/test/API/commands/frame/diagnose/inheritance/TestDiagnoseInheritance.py b/lldb/test/API/commands/frame/diagnose/inheritance/TestDiagnoseInheritance.py
index 2e5a5f19b940..f006db5219a4 100644
--- a/lldb/test/API/commands/frame/diagnose/inheritance/TestDiagnoseInheritance.py
+++ b/lldb/test/API/commands/frame/diagnose/inheritance/TestDiagnoseInheritance.py
@@ -22,4 +22,5 @@ def test_diagnose_inheritance(self):
         self.runCmd("run", RUN_SUCCEEDED)
         self.expect("thread list", "Thread should be stopped",
                     substrs=['stopped'])
-        self.expect("frame diagnose", "Crash diagnosis was accurate", "d")
+        self.expect("frame diagnose", "Crash diagnosis was accurate",
+                    substrs=["d"])

diff  --git a/lldb/test/API/commands/frame/diagnose/local-variable/TestLocalVariable.py b/lldb/test/API/commands/frame/diagnose/local-variable/TestLocalVariable.py
index 7e60467bf425..4fcfa77666c0 100644
--- a/lldb/test/API/commands/frame/diagnose/local-variable/TestLocalVariable.py
+++ b/lldb/test/API/commands/frame/diagnose/local-variable/TestLocalVariable.py
@@ -22,4 +22,5 @@ def test_local_variable(self):
         self.runCmd("run", RUN_SUCCEEDED)
         self.expect("thread list", "Thread should be stopped",
                     substrs=['stopped'])
-        self.expect("frame diagnose", "Crash diagnosis was accurate", "myInt")
+        self.expect("frame diagnose", "Crash diagnosis was accurate",
+                    substrs=["myInt"])

diff  --git a/lldb/test/API/commands/frame/diagnose/virtual-method-call/TestDiagnoseDereferenceVirtualMethodCall.py b/lldb/test/API/commands/frame/diagnose/virtual-method-call/TestDiagnoseDereferenceVirtualMethodCall.py
index 802bf1bd29d6..d29d69d73229 100644
--- a/lldb/test/API/commands/frame/diagnose/virtual-method-call/TestDiagnoseDereferenceVirtualMethodCall.py
+++ b/lldb/test/API/commands/frame/diagnose/virtual-method-call/TestDiagnoseDereferenceVirtualMethodCall.py
@@ -22,4 +22,5 @@ def test_diagnose_virtual_method_call(self):
         self.runCmd("run", RUN_SUCCEEDED)
         self.expect("thread list", "Thread should be stopped",
                     substrs=['stopped'])
-        self.expect("frame diagnose", "Crash diagnosis was accurate", "foo")
+        self.expect("frame diagnose", "Crash diagnosis was accurate",
+                    substrs=["foo"])

diff  --git a/lldb/test/API/commands/settings/TestSettings.py b/lldb/test/API/commands/settings/TestSettings.py
index fc872f6240fe..180d45e4e934 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -461,15 +461,15 @@ def test_settings_with_quotes(self):
         # if they are provided
         self.runCmd("settings set thread-format    'abc def'   ")
         self.expect("settings show thread-format",
-                    'thread-format (format-string) = "abc def"')
+                    startstr='thread-format (format-string) = "abc def"')
         self.runCmd('settings set thread-format    "abc def"   ')
         self.expect("settings show thread-format",
-                    'thread-format (format-string) = "abc def"')
+                    startstr='thread-format (format-string) = "abc def"')
         # Make sure when no quotes are provided that we maintain any trailing
         # spaces
         self.runCmd('settings set thread-format abc def   ')
         self.expect("settings show thread-format",
-                    'thread-format (format-string) = "abc def   "')
+                    startstr='thread-format (format-string) = "abc def   "')
         self.runCmd('settings clear thread-format')
 
     def test_settings_with_trailing_whitespace(self):

diff  --git a/lldb/test/API/driver/batch_mode/TestBatchMode.py b/lldb/test/API/driver/batch_mode/TestBatchMode.py
index df6cc87d3c34..e5364a460f9c 100644
--- a/lldb/test/API/driver/batch_mode/TestBatchMode.py
+++ b/lldb/test/API/driver/batch_mode/TestBatchMode.py
@@ -44,7 +44,7 @@ def test_batch_mode_run_crash(self):
         child.expect_exact('(char *) touch_me_not')
         # Then we should have a live prompt:
         self.expect_prompt()
-        self.expect("frame variable touch_me_not", substrs='(char *) touch_me_not')
+        self.expect("frame variable touch_me_not", substrs=['(char *) touch_me_not'])
 
     @expectedFlakeyFreeBSD("llvm.org/pr25172 fails rarely on the buildbot")
     def test_batch_mode_run_exit(self):

diff  --git a/lldb/test/API/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py b/lldb/test/API/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py
index e21f1c4553f5..a03e2addbe97 100644
--- a/lldb/test/API/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py
@@ -21,7 +21,7 @@ def testBreakpointByLineAndColumn(self):
         main_c = lldb.SBFileSpec("main.c")
         _, _, _, breakpoint = lldbutil.run_to_line_breakpoint(self,
                                                               main_c, 11, 50)
-        self.expect("fr v did_call", substrs='1')
+        self.expect("fr v did_call", substrs=['1'])
         in_then = False
         for i in range(breakpoint.GetNumLocations()):
             b_loc = breakpoint.GetLocationAtIndex(i).GetAddress().GetLineEntry()
@@ -35,7 +35,7 @@ def testBreakpointByLine(self):
         self.build()
         main_c = lldb.SBFileSpec("main.c")
         _, _, _, breakpoint = lldbutil.run_to_line_breakpoint(self, main_c, 11)
-        self.expect("fr v did_call", substrs='0')
+        self.expect("fr v did_call", substrs=['0'])
         in_condition = False
         for i in range(breakpoint.GetNumLocations()):
             b_loc = breakpoint.GetLocationAtIndex(i).GetAddress().GetLineEntry()

diff  --git a/lldb/test/API/functionalities/data-formatter/data-formatter-objc/cmtime/TestDataFormatterCMTime.py b/lldb/test/API/functionalities/data-formatter/data-formatter-objc/cmtime/TestDataFormatterCMTime.py
index 8943f8313f3c..d08ab16401fc 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-objc/cmtime/TestDataFormatterCMTime.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-objc/cmtime/TestDataFormatterCMTime.py
@@ -48,6 +48,6 @@ def test_nsindexpath_with_run_command(self):
         self.expect(
             'frame variable t4',
             substrs=['10 seconds', 'value = 10', 'timescale = 1', 'epoch = 0'])
-        self.expect('frame variable t5', '-oo')
-        self.expect('frame variable t6', '+oo')
-        self.expect('frame variable t7', 'indefinite')
+        self.expect('frame variable t5', substrs=['+oo'])
+        self.expect('frame variable t6', substrs=['-oo'])
+        self.expect('frame variable t7', substrs=['indefinite'])

diff  --git a/lldb/test/API/lang/cpp/constructors/TestCppConstructors.py b/lldb/test/API/lang/cpp/constructors/TestCppConstructors.py
index e330093a8467..3e368d7125a9 100644
--- a/lldb/test/API/lang/cpp/constructors/TestCppConstructors.py
+++ b/lldb/test/API/lang/cpp/constructors/TestCppConstructors.py
@@ -19,7 +19,7 @@ def test_constructors(self):
         self.expect_expr("ClassWithDeletedDefaultCtor(7).value", result_type="int", result_value="7")
 
         # FIXME: It seems we try to call the non-existent default constructor here which is wrong.
-        self.expect("expr ClassWithDefaultedCtor().foo()", error=True, substrs="Couldn't lookup symbols:")
+        self.expect("expr ClassWithDefaultedCtor().foo()", error=True, substrs=["Couldn't lookup symbols:"])
 
         # FIXME: Calling deleted constructors should fail before linking.
         self.expect("expr ClassWithDeletedCtor(1).value", error=True, substrs=["Couldn't lookup symbols:"])

diff  --git a/lldb/test/API/macosx/macCatalyst/TestMacCatalyst.py b/lldb/test/API/macosx/macCatalyst/TestMacCatalyst.py
index 555d5a13b555..520e92790bd2 100644
--- a/lldb/test/API/macosx/macCatalyst/TestMacCatalyst.py
+++ b/lldb/test/API/macosx/macCatalyst/TestMacCatalyst.py
@@ -24,8 +24,8 @@ def test_macabi(self):
         self.expect("image list -t -b",
                     patterns=[self.getArchitecture() +
                               r'.*-apple-ios.*-macabi a\.out'])
-        self.expect("fr v s", "Hello macCatalyst")
-        self.expect("p s", "Hello macCatalyst")
+        self.expect("fr v s", substrs=["Hello macCatalyst"])
+        self.expect("p s", substrs=["Hello macCatalyst"])
         self.check_debugserver(log)
 
     def check_debugserver(self, log):

diff  --git a/lldb/test/API/types/TestRecursiveTypes.py b/lldb/test/API/types/TestRecursiveTypes.py
index 69194cfb96ae..8e84a052a22b 100644
--- a/lldb/test/API/types/TestRecursiveTypes.py
+++ b/lldb/test/API/types/TestRecursiveTypes.py
@@ -50,5 +50,5 @@ def print_struct(self):
 
         self.runCmd("run", RUN_SUCCEEDED)
 
-        self.expect("print tpi", RUN_SUCCEEDED)
-        self.expect("print *tpi", RUN_SUCCEEDED)
+        self.expect("print tpi")
+        self.expect("print *tpi")


        


More information about the lldb-commits mailing list