[lldb/Test] Introduce "assertSuccess"

Summary:
A lot of our tests do 'self.assertTrue(error.Success()'. The problem
with that is that when this fails, it produces a completely useless
error message (False is not True) and the most important piece of
information -- the actual error message -- is completely hidden.

Sometimes we mitigate that by including the error message in the "msg"
argument, but this has two additional problems:
- as the msg argument is evaluated unconditionally, one needs to be
  careful to not trigger an exception when the operation was actually
  successful.
- it requires more typing, which means we often don't do it

assertSuccess solves these problems by taking the entire SBError object
as an argument. If the operation was unsuccessful, it can format a
reasonable error message itself. The function still accepts a "msg"
argument, which can include any additional context, but this context now
does not need to include the error message.

To demonstrate usage, I replace a number of existing assertTrue
assertions with the new function. As this process is not easily
automatable, I have just manually updated a representative sample. In
some cases, I did not update the code to use assertSuccess, but I went
for even higher-level assertion apis (runCmd, expect_expr), as these are
even shorter, and can produce even better failure messages.

Reviewers: teemperor, JDevlieghere

Subscribers: arphaman, lldb-commits

Tags: #lldb

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

(cherry picked from commit 3567497)

Author: labath

lldb/packages/Python/lldbsuite/test/lldbtest.py

@@ -2394,9 +2394,7 @@ def expect_expr(
             self.assertEqual(error_msg, eval_result.GetError().GetCString())
             return
 
-        if not eval_result.GetError().Success():
-            self.assertTrue(eval_result.GetError().Success(),
-                "Unexpected failure with msg: " + eval_result.GetError().GetCString())
+        self.assertSuccess(eval_result.GetError())
 
         if result_type:
             self.assertEqual(result_type, eval_result.GetDisplayTypeName())
@@ -2448,6 +2446,13 @@ def run_platform_command(self, cmd):
         err = platform.Run(shell_command)
         return (err, shell_command.GetStatus(), shell_command.GetOutput())
 
+    """Assert that an lldb.SBError is in the "success" state."""
+    def assertSuccess(self, obj, msg=None):
+        if not obj.Success():
+            error = obj.GetCString()
+            self.fail(self._formatMessage(msg,
+                "'{}' is not success".format(error)))
+
     # =================================================
     # Misc. helper methods for debugging test execution
     # =================================================

lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py

@@ -52,10 +52,7 @@ def call_function(self):
                                       'Stop here in main.', self.main_source_spec)
 
         # Make sure the SIGCHLD behavior is pass/no-stop/no-notify:
-        return_obj = lldb.SBCommandReturnObject()
-        self.dbg.GetCommandInterpreter().HandleCommand(
-            "process handle SIGCHLD -s 0 -p 1 -n 0", return_obj)
-        self.assertTrue(return_obj.Succeeded(), "Set SIGCHLD to pass, no-stop")
+        self.runCmd("process handle SIGCHLD -s 0 -p 1 -n 0")
 
         # The sigchld_no variable should be 0 at this point.
         self.sigchld_no = target.FindFirstGlobalVariable("sigchld_no")
@@ -107,11 +104,7 @@ def call_function(self):
 
         # Now set the signal to print but not stop and make sure that calling
         # still works:
-        self.dbg.GetCommandInterpreter().HandleCommand(
-            "process handle SIGCHLD -s 0 -p 1 -n 1", return_obj)
-        self.assertTrue(
-            return_obj.Succeeded(),
-            "Set SIGCHLD to pass, no-stop, notify")
+        self.runCmd("process handle SIGCHLD -s 0 -p 1 -n 1")
 
         value = frame.EvaluateExpression(
             "call_me (%d)" %
@@ -135,29 +128,20 @@ def call_function(self):
         # Okay, now set UnwindOnError to true, and then make the signal behavior to stop
         # and see that now we do stop at the signal point:
 
-        self.dbg.GetCommandInterpreter().HandleCommand(
-            "process handle SIGCHLD -s 1 -p 1 -n 1", return_obj)
-        self.assertTrue(
-            return_obj.Succeeded(),
-            "Set SIGCHLD to pass, stop, notify")
+        self.runCmd("process handle SIGCHLD -s 1 -p 1 -n 1")
 
         value = frame.EvaluateExpression(
             "call_me (%d)" %
             (num_sigchld), options)
-        self.assertTrue(
-            value.IsValid() and value.GetError().Success() == False)
+        self.assertTrue(value.IsValid())
+        self.assertFalse(value.GetError().Success())
 
         # Set signal handling back to no-stop, and continue and we should end
         # up back in out starting frame:
-        self.dbg.GetCommandInterpreter().HandleCommand(
-            "process handle SIGCHLD -s 0 -p 1 -n 1", return_obj)
-        self.assertTrue(
-            return_obj.Succeeded(),
-            "Set SIGCHLD to pass, no-stop, notify")
+        self.runCmd("process handle SIGCHLD -s 0 -p 1 -n 1")
 
         error = process.Continue()
-        self.assertTrue(
-            error.Success(),
+        self.assertSuccess(error,
             "Continuing after stopping for signal succeeds.")
 
         frame = self.thread.GetFrameAtIndex(0)

lldb/test/API/commands/expression/context-object-objc/TestContextObjectObjc.py

@@ -44,13 +44,13 @@ def test_context_object_objc(self):
         # Test retrieving of an objcClass's property through the self pointer
         value = obj_val.EvaluateExpression("self.property")
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
         self.assertEqual(value.GetValueAsSigned(), 2222)
 
         # Test objcClass's methods evaluation through the self pointer
         value = obj_val.EvaluateExpression("[self method]")
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
         self.assertEqual(value.GetValueAsSigned(), 3333)
 
         # Test if we can use a computation result reference object correctly
@@ -63,12 +63,12 @@ def test_context_object_objc(self):
         # Test an expression evaluation on it
         value = obj_val.EvaluateExpression("1")
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
 
         # Test retrieving of a field on it
         value = obj_val.EvaluateExpression("field")
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
         self.assertEqual(value.GetValueAsSigned(), 1111)
 
     def setUp(self):

lldb/test/API/commands/expression/context-object/TestContextObject.py

@@ -32,19 +32,19 @@ def test_context_object(self):
         # Test retrieveing of a field (not a local with the same name)
         value = obj_val.EvaluateExpression("field")
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
         self.assertEqual(value.GetValueAsSigned(), 1111)
 
         # Test functions evaluation
         value = obj_val.EvaluateExpression("function()")
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
         self.assertEqual(value.GetValueAsSigned(), 2222)
 
         # Test that we retrieve the right global
         value = obj_val.EvaluateExpression("global.field")
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
         self.assertEqual(value.GetValueAsSigned(), 1111)
 
         #
@@ -57,7 +57,7 @@ def test_context_object(self):
         # Test retrieveing of a field
         value = obj_val.EvaluateExpression("field_int")
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
         self.assertEqual(value.GetValueAsSigned(), 5555)
 
         #
@@ -87,7 +87,7 @@ def test_context_object(self):
         # Test retrieveing of an element's field
         value = obj_val.GetValueForExpressionPath("[7]").EvaluateExpression("field")
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
         self.assertEqual(value.GetValueAsSigned(), 1111)
 
         #
@@ -105,7 +105,7 @@ def test_context_object(self):
         # Test retrieveing of a dereferenced object's field
         value = obj_val.Dereference().EvaluateExpression("field")
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
         self.assertEqual(value.GetValueAsSigned(), 1111)
 
         #
@@ -135,7 +135,7 @@ def test_context_object(self):
         # Test retrieveing of a dereferenced object's field
         value = obj_val.Dereference().EvaluateExpression("field")
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
         self.assertEqual(value.GetValueAsSigned(), 1111)
 
     def setUp(self):

lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py

@@ -58,7 +58,7 @@ def expr_options_test(self):
 
         # Now use the options:
         result = frame.EvaluateExpression("call_me(10)", options)
-        self.assertTrue(result.GetError().Success(), "expression succeeded")
+        self.assertSuccess(result.GetError())
         self.assertEqual(result.GetValueAsSigned(), 18, "got the right value.")
 
         # Now disallow JIT and make sure it fails:
@@ -77,6 +77,6 @@ def expr_options_test(self):
 
         # And again, make sure this works:
         result = frame.EvaluateExpression("call_me(10)", options)
-        self.assertTrue(result.GetError().Success(), "expression succeeded")
+        self.assertSuccess(result.GetError())
         self.assertEqual(result.GetValueAsSigned(), 18, "got the right value.")
 

lldb/test/API/commands/expression/options/TestExprOptions.py

@@ -46,14 +46,14 @@ def test_expr_options(self):
         # Make sure we can evaluate a C++11 expression.
         val = frame.EvaluateExpression('foo != nullptr')
         self.assertTrue(val.IsValid())
-        self.assertTrue(val.GetError().Success())
+        self.assertSuccess(val.GetError())
         self.DebugSBValue(val)
 
         # Make sure it still works if language is set to C++11:
         options.SetLanguage(lldb.eLanguageTypeC_plus_plus_11)
         val = frame.EvaluateExpression('foo != nullptr', options)
         self.assertTrue(val.IsValid())
-        self.assertTrue(val.GetError().Success())
+        self.assertSuccess(val.GetError())
         self.DebugSBValue(val)
 
         # Make sure it fails if language is set to C:
@@ -80,7 +80,7 @@ def test_expr_options_lang(self):
         options.SetLanguage(lldb.eLanguageTypeC_plus_plus_11)
         val = frame.EvaluateExpression('id == 0', options)
         self.assertTrue(val.IsValid())
-        self.assertTrue(val.GetError().Success())
+        self.assertSuccess(val.GetError())
         self.DebugSBValue(val)
 
         # Make sure we can't retrieve `id` variable if language is set to ObjC:

lldb/test/API/commands/expression/pr35310/TestExprsBug35310.py

@@ -23,16 +23,8 @@ def test_issue35310(self):
         """
         self.build()
 
-        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+        lldbutil.run_to_source_breakpoint(self,
                                           '// Break here', self.main_source_spec)
-        frame = thread.GetFrameAtIndex(0)
 
-        value = frame.EvaluateExpression("a.test_abi_tag()")
-        self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
-        self.assertEqual(value.GetValueAsSigned(0), 1)
-
-        value = frame.EvaluateExpression("a.test_asm_name()")
-        self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
-        self.assertEqual(value.GetValueAsSigned(0), 2)
+        self.expect_expr("a.test_abi_tag()", result_value='1')
+        self.expect_expr("a.test_asm_name()", result_value='2')

lldb/test/API/commands/expression/scoped_enums/TestScopedEnumType.py

@@ -23,15 +23,8 @@ def test(self):
         self.expect("expr f == Foo::FooBar",
                 substrs=['(bool) $0 = true'])
 
-        value = frame.EvaluateExpression("f == Foo::FooBar")
-        self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
-        self.assertEqual(value.GetValueAsUnsigned(), 1)
-
-        value = frame.EvaluateExpression("b == BarBar")
-        self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
-        self.assertEqual(value.GetValueAsUnsigned(), 1)
+        self.expect_expr("f == Foo::FooBar", result_value='true')
+        self.expect_expr("b == BarBar", result_value='true')
 
         ## b is not a Foo
         value = frame.EvaluateExpression("b == Foo::FooBar")

lldb/test/API/commands/expression/timeout/TestCallWithTimeout.py

@@ -58,7 +58,7 @@ def test(self):
         options.SetTimeoutInMicroSeconds(1000000)
         value = frame.EvaluateExpression("wait_a_while (1000)", options)
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())
 
         # Now do the same thingwith the command line command, and make sure it
         # works too.
@@ -76,4 +76,4 @@ def test(self):
         options.SetOneThreadTimeoutInMicroSeconds(500000)
         value = frame.EvaluateExpression("wait_a_while (1000)", options)
         self.assertTrue(value.IsValid())
-        self.assertTrue(value.GetError().Success())
+        self.assertSuccess(value.GetError())

lldb/test/API/commands/expression/unwind_expression/TestUnwindExpression.py

@@ -45,9 +45,7 @@ def test_conditional_bktp(self):
 
         main_frame = self.thread.GetFrameAtIndex(0)
         val = main_frame.EvaluateExpression("second_function(47)", options)
-        self.assertTrue(
-            val.GetError().Success(),
-            "We did complete the execution.")
+        self.assertSuccess(val.GetError(), "We did complete the execution.")
         self.assertEquals(47, val.GetValueAsSigned())
 
 
@@ -91,8 +89,8 @@ def do_unwind_test(self, thread, bkpt, timeout):
 
         # Now unwind the expression, and make sure we got back to where we
         # started.
-        error = thread.UnwindInnermostExpression()
-        self.assertTrue(error.Success(), "We succeeded in unwinding")
+        self.assertSuccess(thread.UnwindInnermostExpression(),
+                "We succeeded in unwinding")
 
         cur_frame = thread.GetFrameAtIndex(0)
         self.assertTrue(

lldb/test/API/commands/expression/weak_symbols/TestWeakSymbols.py

@@ -39,7 +39,7 @@ def run_weak_var_check (self, weak_varname, present):
         # the bug that expressions with no result currently return False for Success()...
         expr = "if (&" + weak_varname + " != NULL) { present_weak_int = 10; } else { present_weak_int = 20;}; 10"
         result = self.frame.EvaluateExpression(expr)
-        self.assertTrue(result.GetError().Success(), "absent_weak_int expr failed: %s"%(result.GetError().GetCString()))
+        self.assertSuccess(result.GetError(), "absent_weak_int expr failed")
         self.assertEqual(value.GetValueAsSigned(), correct_value, "Didn't change present_weak_int correctly.")
 
     def do_test(self):

lldb/test/API/commands/register/register/register_command/TestRegisters.py

@@ -235,12 +235,12 @@ def fp_special_purpose_register_read(self):
         error = lldb.SBError()
         reg_value_fstat_initial = value.GetValueAsUnsigned(error, 0)
 
-        self.assertTrue(error.Success(), "reading a value for fstat")
+        self.assertSuccess(error, "reading a value for fstat")
         value = currentFrame.FindValue("ftag", lldb.eValueTypeRegister)
         error = lldb.SBError()
         reg_value_ftag_initial = value.GetValueAsUnsigned(error, 0)
 
-        self.assertTrue(error.Success(), "reading a value for ftag")
+        self.assertSuccess(error, "reading a value for ftag")
         fstat_top_pointer_initial = (reg_value_fstat_initial & 0x3800) >> 11
 
         # Execute 'si' aka 'thread step-inst' instruction 5 times and with
@@ -292,7 +292,7 @@ def fp_register_write(self):
                 0, # launch flags
                 True, # stop at entry
                 error)
-        self.assertTrue(error.Success(), "Launch succeeds. Error is :" + str(error))
+        self.assertSuccess(error, "Launch succeeds")
 
         self.assertTrue(
             process.GetState() == lldb.eStateStopped,

lldb/test/API/python_api/hello_world/TestHelloWorld.py

@@ -140,7 +140,8 @@ def test_with_attach_to_process_with_name_api(self):
         target.ConnectRemote(listener, None, None, error)
 
         process = target.AttachToProcessWithName(listener, name, False, error)
-        self.assertTrue(error.Success() and process, PROCESS_IS_VALID)
+        self.assertSuccess(error)
+        self.assertTrue(process, PROCESS_IS_VALID)
 
         # Verify that after attach, our selected target indeed matches name.
         self.expect(