[lldb][test] Modernize assertEqual(value, bool) #82526
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Any time we see the pattern `assertEqual(value, bool)`, we can replace that with `assert<bool>(value)`. Likewise for `assertNotEqual`.
Technically this relaxes the test a bit, as we may want to make sure `value` is either `True` or `False`, and not something that implicitly converts to a bool. For example, `assertEqual("foo", True)` will fail, but `assertTrue("foo")` will not. The cases here seem correct.
Followup to 9c24688. I patched `teyit` with a `visit_assertEqual` node handler to generate this.
|
@llvm/pr-subscribers-lldb Author: Jordan Rupprecht (rupprecht) ChangesAny time we see the pattern Technically this relaxes the test a bit, as we may want to make sure There are two such places that this patch does not transform, since it seems intentional that we want the result to be a bool: Followup to 9c24688. I patched Patch is 28.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82526.diff 19 Files Affected:
diff --git a/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py b/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
index 2868ec5ffdbdf8..b8cc87c93ba61d 100644
--- a/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
+++ b/lldb/test/API/commands/expression/call-throws/TestCallThatThrows.py
@@ -46,7 +46,7 @@ def call_function(self):
value = frame.EvaluateExpression("[my_class callMeIThrow]", options)
self.assertTrue(value.IsValid())
- self.assertEqual(value.GetError().Success(), False)
+ self.assertFalse(value.GetError().Success())
self.check_after_call()
diff --git a/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py b/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
index 307d4521427dcf..eb812f1902e662 100644
--- a/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
+++ b/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
@@ -54,7 +54,7 @@ def expr_options_test(self):
# First make sure we can call the function with the default option set.
options = lldb.SBExpressionOptions()
# Check that the default is to allow JIT:
- self.assertEqual(options.GetAllowJIT(), True, "Default is true")
+ self.assertTrue(options.GetAllowJIT(), "Default is true")
# Now use the options:
result = frame.EvaluateExpression("call_me(10)", options)
@@ -64,9 +64,7 @@ def expr_options_test(self):
# Now disallow JIT and make sure it fails:
options.SetAllowJIT(False)
# Check that we got the right value:
- self.assertEqual(
- options.GetAllowJIT(), False, "Got False after setting to False"
- )
+ self.assertFalse(options.GetAllowJIT(), "Got False after setting to False")
# Again use it and ensure we fail:
result = frame.EvaluateExpression("call_me(10)", options)
@@ -79,7 +77,7 @@ def expr_options_test(self):
# Finally set the allow JIT value back to true and make sure that works:
options.SetAllowJIT(True)
- self.assertEqual(options.GetAllowJIT(), True, "Set back to True correctly")
+ self.assertTrue(options.GetAllowJIT(), "Set back to True correctly")
# And again, make sure this works:
result = frame.EvaluateExpression("call_me(10)", options)
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 6f083222227fbf..fb6fc07d2d4437 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -35,17 +35,13 @@ def test_enable_disable(self):
)
def verify_key_in_dict(self, key, d, description):
- self.assertEqual(
- key in d,
- True,
- 'make sure key "%s" is in dictionary %s' % (key, description),
+ self.assertIn(
+ key, d, 'make sure key "%s" is in dictionary %s' % (key, description)
)
def verify_key_not_in_dict(self, key, d, description):
- self.assertEqual(
- key in d,
- False,
- 'make sure key "%s" is in dictionary %s' % (key, description),
+ self.assertNotIn(
+ key, d, 'make sure key "%s" is in dictionary %s' % (key, description)
)
def verify_keys(self, dict, description, keys_exist, keys_missing=None):
@@ -120,9 +116,7 @@ def test_expressions_frame_var_counts(self):
self.verify_success_fail_count(stats, "frameVariable", 1, 0)
# Test that "stopCount" is available when the process has run
- self.assertEqual(
- "stopCount" in stats, True, 'ensure "stopCount" is in target JSON'
- )
+ self.assertIn("stopCount", stats, 'ensure "stopCount" is in target JSON')
self.assertGreater(
stats["stopCount"], 0, 'make sure "stopCount" is greater than zero'
)
@@ -484,9 +478,9 @@ def test_dsym_binary_has_symfile_in_stats(self):
exe = self.getBuildArtifact(exe_name)
dsym = self.getBuildArtifact(exe_name + ".dSYM")
# Make sure the executable file exists after building.
- self.assertEqual(os.path.exists(exe), True)
+ self.assertTrue(os.path.exists(exe))
# Make sure the dSYM file exists after building.
- self.assertEqual(os.path.isdir(dsym), True)
+ self.assertTrue(os.path.isdir(dsym))
# Create the target
target = self.createTestTarget(file_path=exe)
@@ -532,9 +526,9 @@ def test_no_dsym_binary_has_symfile_identifiers_in_stats(self):
exe = self.getBuildArtifact(exe_name)
dsym = self.getBuildArtifact(exe_name + ".dSYM")
# Make sure the executable file exists after building.
- self.assertEqual(os.path.exists(exe), True)
+ self.assertTrue(os.path.exists(exe))
# Make sure the dSYM file doesn't exist after building.
- self.assertEqual(os.path.isdir(dsym), False)
+ self.assertFalse(os.path.isdir(dsym))
# Create the target
target = self.createTestTarget(file_path=exe)
@@ -585,11 +579,11 @@ def test_had_frame_variable_errors(self):
dsym = self.getBuildArtifact(exe_name + ".dSYM")
main_obj = self.getBuildArtifact("main.o")
# Make sure the executable file exists after building.
- self.assertEqual(os.path.exists(exe), True)
+ self.assertTrue(os.path.exists(exe))
# Make sure the dSYM file doesn't exist after building.
- self.assertEqual(os.path.isdir(dsym), False)
+ self.assertFalse(os.path.isdir(dsym))
# Make sure the main.o object file exists after building.
- self.assertEqual(os.path.exists(main_obj), True)
+ self.assertTrue(os.path.exists(main_obj))
# Delete the main.o file that contains the debug info so we force an
# error when we run to main and try to get variables
@@ -604,7 +598,7 @@ def test_had_frame_variable_errors(self):
# Make sure we have "debugInfoHadVariableErrors" variable that is set to
# false before failing to get local variables due to missing .o file.
- self.assertEqual(exe_stats["debugInfoHadVariableErrors"], False)
+ self.assertFalse(exe_stats["debugInfoHadVariableErrors"])
# Verify that the top level statistic that aggregates the number of
# modules with debugInfoHadVariableErrors is zero
@@ -624,7 +618,7 @@ def test_had_frame_variable_errors(self):
# Make sure we have "hadFrameVariableErrors" variable that is set to
# true after failing to get local variables due to missing .o file.
- self.assertEqual(exe_stats["debugInfoHadVariableErrors"], True)
+ self.assertTrue(exe_stats["debugInfoHadVariableErrors"])
# Verify that the top level statistic that aggregates the number of
# modules with debugInfoHadVariableErrors is greater than zero
diff --git a/lldb/test/API/commands/trace/TestTraceSave.py b/lldb/test/API/commands/trace/TestTraceSave.py
index ef1ab2f7aa41c8..af38669cb4fce3 100644
--- a/lldb/test/API/commands/trace/TestTraceSave.py
+++ b/lldb/test/API/commands/trace/TestTraceSave.py
@@ -179,11 +179,11 @@ def testSaveTrace(self):
res = lldb.SBCommandReturnObject()
ci.HandleCommand("thread trace dump instructions -c 10 --forwards", res)
- self.assertEqual(res.Succeeded(), True)
+ self.assertTrue(res.Succeeded())
first_ten_instructions = res.GetOutput()
ci.HandleCommand("thread trace dump instructions -c 10", res)
- self.assertEqual(res.Succeeded(), True)
+ self.assertTrue(res.Succeeded())
last_ten_instructions = res.GetOutput()
# Now, save the trace to <trace_copy_dir>
@@ -203,11 +203,11 @@ def testSaveTrace(self):
# Compare with instructions saved at the first time
ci.HandleCommand("thread trace dump instructions -c 10 --forwards", res)
- self.assertEqual(res.Succeeded(), True)
+ self.assertTrue(res.Succeeded())
self.assertEqual(res.GetOutput(), first_ten_instructions)
ci.HandleCommand("thread trace dump instructions -c 10", res)
- self.assertEqual(res.Succeeded(), True)
+ self.assertTrue(res.Succeeded())
self.assertEqual(res.GetOutput(), last_ten_instructions)
def testSaveKernelTrace(self):
diff --git a/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py b/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
index 0ab11a427c1009..d120692e4d6e63 100644
--- a/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
+++ b/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
@@ -40,7 +40,7 @@ def address_breakpoints(self):
bkpt = target.BreakpointCreateByAddress(illegal_address)
# Verify that breakpoint is not resolved.
for bp_loc in bkpt:
- self.assertEqual(bp_loc.IsResolved(), False)
+ self.assertFalse(bp_loc.IsResolved())
else:
self.fail(
"Could not find an illegal address at which to set a bad breakpoint."
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
index 620f648d51fd29..ea242952e409ec 100644
--- a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -572,9 +572,9 @@ def verify_source_map_deduce_statistics(self, target, expected_count):
res = target.GetStatistics().GetAsJSON(stream)
self.assertTrue(res.Success())
debug_stats = json.loads(stream.GetData())
- self.assertEqual(
- "targets" in debug_stats,
- True,
+ self.assertIn(
+ "targets",
+ debug_stats,
'Make sure the "targets" key in in target.GetStatistics()',
)
target_stats = debug_stats["targets"][0]
@@ -659,9 +659,9 @@ def test_breakpoint_statistics_hitcount(self):
res = target.GetStatistics().GetAsJSON(stream)
self.assertTrue(res.Success())
debug_stats = json.loads(stream.GetData())
- self.assertEqual(
- "targets" in debug_stats,
- True,
+ self.assertIn(
+ "targets",
+ debug_stats,
'Make sure the "targets" key in in target.GetStatistics()',
)
target_stats = debug_stats["targets"][0]
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py b/lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
index 330f916a907e6d..0f9510c4507d0e 100644
--- a/lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
@@ -389,7 +389,7 @@ def do_check_configuring_names(self):
)
def check_permission_results(self, bp_name):
- self.assertEqual(bp_name.GetAllowDelete(), False, "Didn't set allow delete.")
+ self.assertFalse(bp_name.GetAllowDelete(), "Didn't set allow delete.")
protected_bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10)
protected_id = protected_bkpt.GetID()
@@ -402,14 +402,11 @@ def check_permission_results(self, bp_name):
self.assertSuccess(success, "Couldn't add this name to the breakpoint")
self.target.DisableAllBreakpoints()
- self.assertEqual(
- protected_bkpt.IsEnabled(),
- True,
- "Didnt' keep breakpoint from being disabled",
+ self.assertTrue(
+ protected_bkpt.IsEnabled(), "Didnt' keep breakpoint from being disabled"
)
- self.assertEqual(
+ self.assertFalse(
unprotected_bkpt.IsEnabled(),
- False,
"Protected too many breakpoints from disabling.",
)
@@ -418,14 +415,11 @@ def check_permission_results(self, bp_name):
result = lldb.SBCommandReturnObject()
self.dbg.GetCommandInterpreter().HandleCommand("break disable", result)
self.assertTrue(result.Succeeded())
- self.assertEqual(
- protected_bkpt.IsEnabled(),
- True,
- "Didnt' keep breakpoint from being disabled",
+ self.assertTrue(
+ protected_bkpt.IsEnabled(), "Didnt' keep breakpoint from being disabled"
)
- self.assertEqual(
+ self.assertFalse(
unprotected_bkpt.IsEnabled(),
- False,
"Protected too many breakpoints from disabling.",
)
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestJLink6Armv7RegisterDefinition.py b/lldb/test/API/functionalities/gdb_remote_client/TestJLink6Armv7RegisterDefinition.py
index eb7c036c8d6002..3a426620af5591 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestJLink6Armv7RegisterDefinition.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestJLink6Armv7RegisterDefinition.py
@@ -198,7 +198,7 @@ def QListThreadsInStopReply(self):
error = lldb.SBError()
data = lldb.SBData()
data.SetData(error, val, lldb.eByteOrderBig, 4)
- self.assertEqual(r1_valobj.SetData(data, error), True)
+ self.assertTrue(r1_valobj.SetData(data, error))
self.assertSuccess(error)
r1_valobj = process.GetThreadAtIndex(0).GetFrameAtIndex(0).FindRegister("r1")
diff --git a/lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py b/lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py
index 4214bd108bfc5d..abf4cf3944e14a 100644
--- a/lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py
+++ b/lldb/test/API/functionalities/module_cache/simple_exe/TestModuleCacheSimple.py
@@ -66,18 +66,16 @@ def test(self):
# get a different creation and modification time for the file since some
# OSs store the modification time in seconds since Jan 1, 1970.
os.remove(exe)
- self.assertEqual(
- os.path.exists(exe),
- False,
- "make sure we were able to remove the executable",
+ self.assertFalse(
+ os.path.exists(exe), "make sure we were able to remove the executable"
)
time.sleep(2)
# Now rebuild the binary so it has a different content which should
# update the UUID to make the cache miss when it tries to load the
# symbol table from the binary at the same path.
self.build(dictionary={"CFLAGS_EXTRAS": "-DEXTRA_FUNCTION"})
- self.assertEqual(
- os.path.exists(exe), True, "make sure executable exists after rebuild"
+ self.assertTrue(
+ os.path.exists(exe), "make sure executable exists after rebuild"
)
# Make sure the modification time has changed or this test will fail.
exe_mtime_2 = os.path.getmtime(exe)
@@ -99,9 +97,8 @@ def test(self):
main_module = target.GetModuleAtIndex(0)
self.assertTrue(main_module.IsValid())
main_module.GetNumSymbols()
- self.assertEqual(
+ self.assertTrue(
os.path.exists(symtab_cache_path),
- True,
'make sure "symtab" cache files exists after cache is updated',
)
symtab_mtime_2 = os.path.getmtime(symtab_cache_path)
diff --git a/lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py b/lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py
index eee91bfadead97..851097bdfecf2d 100644
--- a/lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py
+++ b/lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py
@@ -33,47 +33,47 @@ def test_stats_api(self):
stream = lldb.SBStream()
res = stats.GetAsJSON(stream)
debug_stats = json.loads(stream.GetData())
- self.assertEqual(
- "targets" in debug_stats,
- True,
+ self.assertIn(
+ "targets",
+ debug_stats,
'Make sure the "targets" key in in target.GetStatistics()',
)
- self.assertEqual(
- "modules" in debug_stats,
- True,
+ self.assertIn(
+ "modules",
+ debug_stats,
'Make sure the "modules" key in in target.GetStatistics()',
)
stats_json = debug_stats["targets"][0]
- self.assertEqual(
- "expressionEvaluation" in stats_json,
- True,
+ self.assertIn(
+ "expressionEvaluation",
+ stats_json,
'Make sure the "expressionEvaluation" key in in target.GetStatistics()["targets"][0]',
)
- self.assertEqual(
- "frameVariable" in stats_json,
- True,
+ self.assertIn(
+ "frameVariable",
+ stats_json,
'Make sure the "frameVariable" key in in target.GetStatistics()["targets"][0]',
)
expressionEvaluation = stats_json["expressionEvaluation"]
- self.assertEqual(
- "successes" in expressionEvaluation,
- True,
+ self.assertIn(
+ "successes",
+ expressionEvaluation,
'Make sure the "successes" key in in "expressionEvaluation" dictionary"',
)
- self.assertEqual(
- "failures" in expressionEvaluation,
- True,
+ self.assertIn(
+ "failures",
+ expressionEvaluation,
'Make sure the "failures" key in in "expressionEvaluation" dictionary"',
)
frameVariable = stats_json["frameVariable"]
- self.assertEqual(
- "successes" in frameVariable,
- True,
+ self.assertIn(
+ "successes",
+ frameVariable,
'Make sure the "successes" key in in "frameVariable" dictionary"',
)
- self.assertEqual(
- "failures" in frameVariable,
- True,
+ self.assertIn(
+ "failures",
+ frameVariable,
'Make sure the "failures" key in in "frameVariable" dictionary"',
)
diff --git a/lldb/test/API/functionalities/thread/backtrace_limit/TestBacktraceLimit.py b/lldb/test/API/functionalities/thread/backtrace_limit/TestBacktraceLimit.py
index 98baea45ce8949..fded504f7c6122 100644
--- a/lldb/test/API/functionalities/thread/backtrace_limit/TestBacktraceLimit.py
+++ b/lldb/test/API/functionalities/thread/backtrace_limit/TestBacktraceLimit.py
@@ -23,5 +23,5 @@ def test_backtrace_depth(self):
interp.HandleCommand(
"settings set target.process.thread.max-backtrace-depth 30", result
)
- self.assertEqual(True, result.Succeeded())
+ self.assertTrue(result.Succeeded())
self.assertEqual(30, thread.GetNumFrames())
diff --git a/lldb/test/API/macosx/arm-corefile-regctx/TestArmMachoCorefileRegctx.py b/lldb/test/API/macosx/arm-corefile-regctx/TestArmMachoCorefileRegctx.py
index 1ecb0f466e78dd..4190ea3ac33184 100644
--- a/lldb/test/API/macosx/arm-corefile-regctx/TestArmMachoCorefileRegctx.py
+++ b/lldb/test/API/macosx/arm-corefile-regctx/TestArmMachoCorefileRegctx.py
@@ -28,7 +28,7 @@ def test_armv7_corefile(self):
target = self.dbg.CreateTarget("")
err = lldb.SBError()
process = target.LoadCore(self.corefile)
- self.assertEqual(process.IsValid(), True)
+ self.assertTrue(process.IsValid())
thread = process.GetSelectedThread()
frame = thread.GetSelectedFrame()
@@ -51,7 +51,7 @@ def test_arm64_corefile(self):
target = self.dbg.CreateTarget("")
err = lldb.SBError()
process = target.LoadCore(self.corefile)
- self.assertEqual(process.IsValid(), True)
+ self.assertTrue(process.IsValid())
thread = process.GetSelectedThread()
frame = thread.GetSelectedFrame()
diff --git a/lldb/test/API/macosx/lc-note/addrable-bits/TestAddrableBitsCorefile.py b/lldb/test/API/macosx/lc-note/addrable-bits/TestAddrableBitsCorefile.py
i...
[truncated]
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Any time we see the pattern
assertEqual(value, bool), we can replace that withassert<bool>(value). Likewise forassertNotEqual.Technically this relaxes the test a bit, as we may want to make sure
valueis eitherTrueorFalse, and not something that implicitly converts to a bool. For example,assertEqual("foo", True)will fail, butassertTrue("foo")will not. In most cases, this distinction is not important.There are two such places that this patch does not transform, since it seems intentional that we want the result to be a bool:
llvm-project/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
Line 90 in 5daf200
llvm-project/lldb/test/API/commands/settings/TestSettings.py
Line 940 in 5daf200
Followup to 9c24688. I patched
teyitwith avisit_assertEqualnode handler to generate this.