-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[lldb][test] Switch LLDB API tests from vendored unittest2 to unittest #79945
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-lldb Author: Jordan Rupprecht (rupprecht) ChangesThis removes the dependency LLDB API tests have on lldb/third_party/Python/module/unittest2, and instead uses the standard one provided by Python. This does not actually remove the vendored dep yet, nor update the docs. I'll do both those once this sticks. Non-trivial changes to call out:
I tested this w/ Patch is 31.80 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/79945.diff 20 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py b/lldb/packages/Python/lldbsuite/test/configuration.py
index 2a4b9b3c070c7..685f491c85fe1 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -12,14 +12,14 @@
# Third-party modules
-import unittest2
+import unittest
# LLDB Modules
import lldbsuite
# The test suite.
-suite = unittest2.TestSuite()
+suite = unittest.TestSuite()
# The list of categories we said we care about
categories_list = None
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py
index 0fb146913388e..a5e1fa51cf6e6 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -11,7 +11,7 @@
import subprocess
# Third-party modules
-import unittest2
+import unittest
# LLDB modules
import lldb
@@ -115,11 +115,11 @@ def _compiler_supports(
def expectedFailureIf(condition, bugnumber=None):
def expectedFailure_impl(func):
- if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+ if isinstance(func, type) and issubclass(func, unittest.TestCase):
raise Exception("Decorator can only be used to decorate a test method")
if condition:
- return unittest2.expectedFailure(func)
+ return unittest.expectedFailure(func)
return func
if callable(bugnumber):
@@ -130,14 +130,14 @@ def expectedFailure_impl(func):
def expectedFailureIfFn(expected_fn, bugnumber=None):
def expectedFailure_impl(func):
- if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+ if isinstance(func, type) and issubclass(func, unittest.TestCase):
raise Exception("Decorator can only be used to decorate a test method")
@wraps(func)
def wrapper(*args, **kwargs):
xfail_reason = expected_fn(*args, **kwargs)
if xfail_reason is not None:
- xfail_func = unittest2.expectedFailure(func)
+ xfail_func = unittest.expectedFailure(func)
xfail_func(*args, **kwargs)
else:
func(*args, **kwargs)
@@ -157,11 +157,11 @@ def wrapper(*args, **kwargs):
def skipTestIfFn(expected_fn, bugnumber=None):
def skipTestIfFn_impl(func):
- if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+ if isinstance(func, type) and issubclass(func, unittest.TestCase):
reason = expected_fn()
# The return value is the reason (or None if we don't skip), so
# reason is used for both args.
- return unittest2.skipIf(condition=reason, reason=reason)(func)
+ return unittest.skipIf(condition=reason, reason=reason)(func)
@wraps(func)
def wrapper(*args, **kwargs):
@@ -191,7 +191,7 @@ def wrapper(*args, **kwargs):
def _xfailForDebugInfo(expected_fn, bugnumber=None):
def expectedFailure_impl(func):
- if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+ if isinstance(func, type) and issubclass(func, unittest.TestCase):
raise Exception("Decorator can only be used to decorate a test method")
func.__xfail_for_debug_info_cat_fn__ = expected_fn
@@ -205,7 +205,7 @@ def expectedFailure_impl(func):
def _skipForDebugInfo(expected_fn, bugnumber=None):
def skipImpl(func):
- if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+ if isinstance(func, type) and issubclass(func, unittest.TestCase):
raise Exception("Decorator can only be used to decorate a test method")
func.__skip_for_debug_info_cat_fn__ = expected_fn
@@ -434,7 +434,7 @@ def add_test_categories(cat):
cat = test_categories.validate(cat, True)
def impl(func):
- if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+ if isinstance(func, type) and issubclass(func, unittest.TestCase):
raise Exception(
"@add_test_categories can only be used to decorate a test method"
)
@@ -465,7 +465,7 @@ def should_skip_benchmarks_test():
def no_debug_info_test(func):
"""Decorate the item as a test what don't use any debug info. If this annotation is specified
then the test runner won't generate a separate test for each debug info format."""
- if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+ if isinstance(func, type) and issubclass(func, unittest.TestCase):
raise Exception(
"@no_debug_info_test can only be used to decorate a test method"
)
@@ -631,7 +631,7 @@ def is_out_of_tree_debugserver():
def skipIfRemote(func):
"""Decorate the item to skip tests if testing remotely."""
- return unittest2.skipIf(lldb.remote_platform, "skip on remote platform")(func)
+ return unittest.skipIf(lldb.remote_platform, "skip on remote platform")(func)
def skipIfNoSBHeaders(func):
@@ -768,7 +768,7 @@ def skipUnlessDarwin(func):
def skipUnlessTargetAndroid(func):
- return unittest2.skipUnless(
+ return unittest.skipUnless(
lldbplatformutil.target_is_android(), "requires target to be Android"
)(func)
@@ -809,7 +809,7 @@ def skipIfPlatform(oslist):
"""Decorate the item to skip tests if running on one of the listed platforms."""
# This decorator cannot be ported to `skipIf` yet because it is used on entire
# classes, which `skipIf` explicitly forbids.
- return unittest2.skipIf(
+ return unittest.skipIf(
lldbplatformutil.getPlatform() in oslist, "skip on %s" % (", ".join(oslist))
)
@@ -818,7 +818,7 @@ def skipUnlessPlatform(oslist):
"""Decorate the item to skip tests unless running on one of the listed platforms."""
# This decorator cannot be ported to `skipIf` yet because it is used on entire
# classes, which `skipIf` explicitly forbids.
- return unittest2.skipUnless(
+ return unittest.skipUnless(
lldbplatformutil.getPlatform() in oslist,
"requires one of %s" % (", ".join(oslist)),
)
@@ -1078,7 +1078,7 @@ def _get_bool_config(key, fail_value=True):
def _get_bool_config_skip_if_decorator(key):
have = _get_bool_config(key)
- return unittest2.skipIf(not have, "requires " + key)
+ return unittest.skipIf(not have, "requires " + key)
def skipIfCursesSupportMissing(func):
@@ -1110,7 +1110,7 @@ def skipIfLLVMTargetMissing(target):
found = True
break
- return unittest2.skipIf(not found, "requires " + target)
+ return unittest.skipIf(not found, "requires " + target)
# Call sysctl on darwin to see if a specified hardware feature is available on this machine.
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py
index 4393e0caacaab..e6d9a2efb3159 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -33,7 +33,7 @@
import tempfile
# Third-party modules
-import unittest2
+import unittest
# LLDB Modules
import lldbsuite
@@ -658,7 +658,7 @@ def iter_filters():
for filterspec in iter_filters():
filtered = True
print("adding filter spec %s to module %s" % (filterspec, repr(module)))
- tests = unittest2.defaultTestLoader.loadTestsFromName(filterspec, module)
+ tests = unittest.defaultTestLoader.loadTestsFromName(filterspec, module)
configuration.suite.addTests(tests)
# Forgo this module if the (base, filterspec) combo is invalid
@@ -670,7 +670,7 @@ def iter_filters():
# Also the fail-over case when the filterspec branch
# (base, filterspec) combo doesn't make sense.
configuration.suite.addTests(
- unittest2.defaultTestLoader.loadTestsFromName(base)
+ unittest.defaultTestLoader.loadTestsFromName(base)
)
@@ -1032,7 +1032,7 @@ def run_suite():
#
# Install the control-c handler.
- unittest2.signals.installHandler()
+ unittest.signals.installHandler()
#
# Invoke the default TextTestRunner to run the test suite
@@ -1066,7 +1066,7 @@ def run_suite():
# Invoke the test runner.
if configuration.count == 1:
- result = unittest2.TextTestRunner(
+ result = unittest.TextTestRunner(
stream=sys.stderr,
verbosity=configuration.verbose,
resultclass=test_result.LLDBTestResult,
@@ -1077,7 +1077,7 @@ def run_suite():
# not enforced.
test_result.LLDBTestResult.__ignore_singleton__ = True
for i in range(configuration.count):
- result = unittest2.TextTestRunner(
+ result = unittest.TextTestRunner(
stream=sys.stderr,
verbosity=configuration.verbose,
resultclass=test_result.LLDBTestResult,
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index d944b09cbcc47..018f2a06980a8 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -44,7 +44,7 @@
import traceback
# Third-party modules
-import unittest2
+import unittest
# LLDB modules
import lldb
@@ -517,7 +517,7 @@ def builder_module():
return lldbplatformutil.builder_module()
-class Base(unittest2.TestCase):
+class Base(unittest.TestCase):
"""
Abstract base for performing lldb (see TestBase) or other generic tests (see
BenchBase for one example). lldbtest.Base works with the test driver to
@@ -1090,17 +1090,14 @@ def markFailure(self):
# Once by the Python unittest framework, and a second time by us.
print("FAIL", file=sbuf)
- def markExpectedFailure(self, err, bugnumber):
+ def markExpectedFailure(self, err):
"""Callback invoked when an expected failure/error occurred."""
self.__expected__ = True
with recording(self, False) as sbuf:
# False because there's no need to write "expected failure" to the
# stderr twice.
# Once by the Python unittest framework, and a second time by us.
- if bugnumber is None:
- print("expected failure", file=sbuf)
- else:
- print("expected failure (problem id:" + str(bugnumber) + ")", file=sbuf)
+ print("expected failure", file=sbuf)
def markSkippedTest(self):
"""Callback invoked when a test is skipped."""
@@ -1111,19 +1108,14 @@ def markSkippedTest(self):
# Once by the Python unittest framework, and a second time by us.
print("skipped test", file=sbuf)
- def markUnexpectedSuccess(self, bugnumber):
+ def markUnexpectedSuccess(self):
"""Callback invoked when an unexpected success occurred."""
self.__unexpected__ = True
with recording(self, False) as sbuf:
# False because there's no need to write "unexpected success" to the
# stderr twice.
# Once by the Python unittest framework, and a second time by us.
- if bugnumber is None:
- print("unexpected success", file=sbuf)
- else:
- print(
- "unexpected success (problem id:" + str(bugnumber) + ")", file=sbuf
- )
+ print("unexpected success", file=sbuf)
def getRerunArgs(self):
return " -f %s.%s" % (self.__class__.__name__, self._testMethodName)
@@ -1704,13 +1696,11 @@ def test_method(self, attrvalue=attrvalue):
xfail_reason = xfail_for_debug_info_cat_fn(cat)
if xfail_reason:
- test_method = unittest2.expectedFailure(xfail_reason)(
- test_method
- )
+ test_method = unittest.expectedFailure(test_method)
skip_reason = skip_for_debug_info_cat_fn(cat)
if skip_reason:
- test_method = unittest2.skip(skip_reason)(test_method)
+ test_method = unittest.skip(skip_reason)(test_method)
newattrs[method_name] = test_method
@@ -2226,7 +2216,7 @@ def completions_match(self, command, completions):
match_strings = lldb.SBStringList()
interp.HandleCompletion(command, len(command), 0, -1, match_strings)
# match_strings is a 1-indexed list, so we have to slice...
- self.assertItemsEqual(
+ self.assertCountEqual(
completions, list(match_strings)[1:], "List of returned completion is wrong"
)
diff --git a/lldb/packages/Python/lldbsuite/test/test_result.py b/lldb/packages/Python/lldbsuite/test/test_result.py
index cb84c909c4196..20365f53a6754 100644
--- a/lldb/packages/Python/lldbsuite/test/test_result.py
+++ b/lldb/packages/Python/lldbsuite/test/test_result.py
@@ -12,14 +12,14 @@
import traceback
# Third-party modules
-import unittest2
+import unittest
# LLDB Modules
from . import configuration
from lldbsuite.test_event import build_exception
-class LLDBTestResult(unittest2.TextTestResult):
+class LLDBTestResult(unittest.TextTestResult):
"""
Enforce a singleton pattern to allow introspection of test progress.
@@ -243,7 +243,7 @@ def addFailure(self, test, err):
if self.checkExclusion(
configuration.xfail_tests, test.id()
) or self.checkCategoryExclusion(configuration.xfail_categories, test):
- self.addExpectedFailure(test, err, None)
+ self.addExpectedFailure(test, err)
return
configuration.sdir_has_content = True
@@ -264,12 +264,12 @@ def addFailure(self, test, err):
else:
configuration.failures_per_category[category] = 1
- def addExpectedFailure(self, test, err, bugnumber):
+ def addExpectedFailure(self, test, err):
configuration.sdir_has_content = True
- super(LLDBTestResult, self).addExpectedFailure(test, err, bugnumber)
+ super(LLDBTestResult, self).addExpectedFailure(test, err)
method = getattr(test, "markExpectedFailure", None)
if method:
- method(err, bugnumber)
+ method(err)
self.stream.write(
"XFAIL: LLDB (%s) :: %s\n" % (self._config_string(test), str(test))
)
@@ -285,12 +285,12 @@ def addSkip(self, test, reason):
% (self._config_string(test), str(test), reason)
)
- def addUnexpectedSuccess(self, test, bugnumber):
+ def addUnexpectedSuccess(self, test):
configuration.sdir_has_content = True
- super(LLDBTestResult, self).addUnexpectedSuccess(test, bugnumber)
+ super(LLDBTestResult, self).addUnexpectedSuccess(test)
method = getattr(test, "markUnexpectedSuccess", None)
if method:
- method(bugnumber)
+ method()
self.stream.write(
"XPASS: LLDB (%s) :: %s\n" % (self._config_string(test), str(test))
)
diff --git a/lldb/test/API/commands/expression/test/TestExprs.py b/lldb/test/API/commands/expression/test/TestExprs.py
index e95c76b7104c2..0e3d2e6cf41ff 100644
--- a/lldb/test/API/commands/expression/test/TestExprs.py
+++ b/lldb/test/API/commands/expression/test/TestExprs.py
@@ -12,7 +12,7 @@
"""
-import unittest2
+import unittest
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
@@ -46,7 +46,7 @@ def build_and_run(self):
# llvm.org/pr17135 <rdar://problem/14874559>
# APFloat::toString does not identify the correct (i.e. least) precision.
- @unittest2.expectedFailure
+ @unittest.expectedFailure
def test_floating_point_expr_commands(self):
self.build_and_run()
diff --git a/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py b/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
index d9b7426b14844..ee597ad2b148c 100644
--- a/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
+++ b/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
@@ -8,7 +8,7 @@
"""
-import unittest2
+import unittest
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
diff --git a/lldb/test/API/functionalities/jitloader_gdb/TestJITLoaderGDB.py b/lldb/test/API/functionalities/jitloader_gdb/TestJITLoaderGDB.py
index a074215244787..98c0b149003df 100644
--- a/lldb/test/API/functionalities/jitloader_gdb/TestJITLoaderGDB.py
+++ b/lldb/test/API/functionalities/jitloader_gdb/TestJITLoaderGDB.py
@@ -1,7 +1,7 @@
"""Test for the JITLoaderGDB interface"""
-import unittest2
+import unittest
import os
import lldb
from lldbsuite.test import lldbutil
@@ -14,7 +14,7 @@ class JITLoaderGDBTestCase(TestBase):
lambda: "Skipped because the test crashes the test runner",
bugnumber="llvm.org/pr24702",
)
- @unittest2.expectedFailure # llvm.org/pr24702
+ @unittest.expectedFailure # llvm.org/pr24702
def test_bogus_values(self):
"""Test that we handle inferior misusing the GDB JIT interface"""
self.build()
diff --git a/lldb/test/API/functionalities/thread/state/TestThreadStates.py b/lldb/test/API/functionalities/thread/state/TestThreadStates.py
index e128ca84977b4..56954c9f34c7e 100644
--- a/lldb/test/API/functionalities/thread/state/TestThreadStates.py
+++ b/lldb/test/API/functionalities/thread/state/TestThreadStates.py
@@ -3,7 +3,7 @@
"""
-import unittest2
+import unittest
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
@@ -41,14 +41,14 @@ def test_state_after_continue(self):
@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24660")
@expectedFailureNetBSD
# thread states not properly maintained
- @unittest2.expectedFailure # llvm.org/pr16712
+ @unittest.expectedFailure # llvm.org/pr16712
def test_state_after_expression(self):
"""Test thread state after expression."""
self.build()
self.thread_state_after_expression_test()
# thread states not properly maintained
- @unittest2.expectedFailure # llvm.org/pr15824 and <rdar://problem/28557237>
+ @unittest.expectedFailure # llvm.org/pr15824 and <rdar://problem/28557237>
@expectedFailureAll(
oslist=["windows"],
bugnumber="llvm.org/pr24668: Breakpoints not resolved correctly",
diff --git a/lldb/test/API/functionalities/tty/TestTerminal.py b/lldb/test/API/functionalities/tty/TestTerminal.py
index 457abd7b4a89d..750cdb3fc8361 100644
--- a/lldb/test/API/functionalities/tty/TestTerminal.py
+++ b/lldb/test/API/functionalities/tty/TestTerminal.py
@@ -2,7 +2,7 @@
Test lldb command aliases.
"""
-import unittest2
+import unittest
import os
import lldb
from lldbsuite.test.decorators import *
@@ -17,13 +17,13 @@ class LaunchInTerminalTestCase(TestBase):
@skipUnlessDarwin
# If the test is being run under sudo, the spawned terminal won't retain that elevated
# privilege so it can't open the socket to talk back to the test case
- @unittest2.skipIf(
+ @unittest.skipIf(
hasattr(os, "geteuid") and os.geteuid() == 0, "test cannot be run as root"
)
# Do we need to disable this test if the testsuite is being run on a remote system?
# This env var is only defined when the shell is running in a local mac
# terminal window
- @unittest2.skipUnless(
+ @unittest.skipUnless(
"TERM_PROGRAM" in os.environ, "test must be run on local system"
)
@no_debug_info_test
diff --git a/lldb/test/API/lang/c/shared_lib/TestSharedLib.py b/lldb/test/API/lang/c/shared_lib/TestSharedLib.py
index 235b9b4ce3442..e0994aae76169 100644
--- a/lldb/test/API/lang/c/shared_lib/TestSharedLib.py
+++ b/lldb/test/API/lang/c/shared_lib/TestSharedLib.py
@@ -1,7 +1,7 @@
"""Test that types defined in shared libraries work correctly."""
-import unittest2
+import unittest
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
@@ -35,7 +35,7 @@ def t...
[truncated]
|
✅ With the latest revision this PR passed the Python code formatter. |
lldb/test/API/lldbtest.py
Outdated
return lit.Test.UNRESOLVED, output | ||
parsed_details[detail_parts[0]] = int(detail_parts[1]) | ||
|
||
failures = parsed_details.get("failures", 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use defaultdict(int)
(https://docs.python.org/3/library/collections.html#collections.defaultdict) or https://docs.python.org/3/library/stdtypes.html#dict.setdefault here.
I ran this on Arm and AArch64 Linux. One test
After:
To be honest, the way the XFAIL is written is very strange. We wrap a Also, I think the XFAIL was added for Arm (32 bit) (30308d1) and did not intend to include AArch64. Whatever the intent was, this now fails on AArch64 so it should be both. I will see if I can just rewrite the xfail here, because we're definitely doing something weird at the moment. |
raise Exception("Decorator can only be used to decorate a test method") | ||
|
||
@wraps(func) | ||
def wrapper(*args, **kwargs): | ||
xfail_reason = expected_fn(*args, **kwargs) | ||
if xfail_reason is not None: | ||
xfail_func = unittest2.expectedFailure(func) | ||
xfail_func = unittest.expectedFailure(func) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's something wrong here. With this PR, the tests in lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
don't xfail, they just fail.
But I presume that expectedFailureIf
is working, so perhaps it's this extra wrapper layer doing something different and the behaviour of expectedFailure
has changed since.
I've simplified that strange xfail in 7565ae6. Still seeing the tests fail instead of xfail though, see my comment. |
Argh, thanks very much for running those tests -- I had noticed that issue before, but forgot about it when mailing this. The issue is that Versions of both More details w/ commit & bug links in #73067 (comment). Anyway, I'll see about landing that first, and hopefully it doesn't interfere w/ this PR much. |
These tests could change to something like:
Since you could argue it's not a known failure, it's a test that's totally incompatible with the host, which would usually be a skip. |
I ran the test suite locally (macOS on arm64) and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me.
Please run the python formatter before landing though. |
edd3cfa
to
193f8ac
Compare
I got one buildbot email: https://lab.llvm.org/buildbot/#/builders/68/builds/68776 Looks like lldb-dap crashes occasionally, and so the The arm/aarch64 lldb bots have had an |
Yeah now was a bad time to land this unfortunately, because I managed to screw up Linaro's lldb bots. Waiting on a fix for that to go in and we'll get back to you if stuff is broken. |
This removes the dependency LLDB API tests have on lldb/third_party/Python/module/unittest2, and instead uses the standard one provided by Python.
This does not actually remove the vendored dep yet, nor update the docs. I'll do both those once this sticks.
Non-trivial changes to call out:
assertItemsEqual
is now calledassertCountEqual
(5 passes, 0 failures, 1 errors, 0 skipped, ...)
, but standard unittest prints a terser message that omits test result types that didn't happen, e.g.OK (skipped=1)
. Our lit integration parses this stderr and needs to be updated w/ that expectation.I tested this w/
ninja check-lldb-abi
on Linux. There's a good chance non-Linux tests have similar quirks, but I'm not able to uncover those.