[Lldb-commits] [lldb] [lldb][test] Remove `self` references from decorators (PR #72416)
Jordan Rupprecht via lldb-commits
lldb-commits at lists.llvm.org
Wed Nov 15 09:36:09 PST 2023
https://github.com/rupprecht created https://github.com/llvm/llvm-project/pull/72416
None
>From b3178bb93c0d414857732b08228987894df762d4 Mon Sep 17 00:00:00 2001
From: Jordan Rupprecht <rupprecht at google.com>
Date: Wed, 15 Nov 2023 08:58:17 -0800
Subject: [PATCH] [lldb][test] Move most `self` references out of decorator
skip checks.
This is partial step toward removing the vendored `unittest2` dep in favor of the `unittest` library in standard python. One of the large differences is when xfail decorators are evaluated. With the `unittest2` vendored dep, this can happen at the moment of calling the test case, and with LLDB's decorator wrappers, we are passed the test class in the decorator arg. With the `unittest` framework, this is determined much earlier; we cannot decide when the test is about to start that we need to xfail.
Fortunately, almost none of these checks require any state that can't be determined statically. For this patch, I moved the impl for all the checks to `lldbplatformutil` and pointed the decorators to that, removing as many `self` (i.e. test class object) references as possible. I left wrappers within `TestBase` that forward to `lldbplatformutil` for convenience, but we should probably remove those later.
The remaining check that can't be moved statically is the check for the debug info type (e.g. to xfail only for dwarf). Fixing that requires a different approach, so I will postpone that to the next patch.
---
.../Python/lldbsuite/test/decorators.py | 87 +++++++----
.../Python/lldbsuite/test/lldbplatformutil.py | 146 +++++++++++++++++-
.../Python/lldbsuite/test/lldbtest.py | 103 ++----------
3 files changed, 207 insertions(+), 129 deletions(-)
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py
index b8fea1e02e864de..14328f3c54a8d1f 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -197,15 +197,17 @@ def _decorateTest(
):
def fn(self):
skip_for_os = _match_decorator_property(
- lldbplatform.translate(oslist), self.getPlatform()
+ lldbplatform.translate(oslist), lldbplatformutil.getPlatform()
)
skip_for_hostos = _match_decorator_property(
lldbplatform.translate(hostoslist), lldbplatformutil.getHostPlatform()
)
skip_for_compiler = _match_decorator_property(
- compiler, self.getCompiler()
- ) and self.expectedCompilerVersion(compiler_version)
- skip_for_arch = _match_decorator_property(archs, self.getArchitecture())
+ compiler, lldbplatformutil.getCompiler()
+ ) and lldbplatformutil.expectedCompilerVersion(compiler_version)
+ skip_for_arch = _match_decorator_property(
+ archs, lldbplatformutil.getArchitecture()
+ )
skip_for_debug_info = _match_decorator_property(debug_info, self.getDebugInfo())
skip_for_triple = _match_decorator_property(
triple, lldb.selected_platform.GetTriple()
@@ -236,7 +238,7 @@ def fn(self):
)
skip_for_dwarf_version = (dwarf_version is None) or (
_check_expected_version(
- dwarf_version[0], dwarf_version[1], self.getDwarfVersion()
+ dwarf_version[0], dwarf_version[1], lldbplatformutil.getDwarfVersion()
)
)
skip_for_setting = (setting is None) or (setting in configuration.settings)
@@ -375,7 +377,9 @@ def skipIf(
def _skip_for_android(reason, api_levels, archs):
def impl(obj):
result = lldbplatformutil.match_android_device(
- obj.getArchitecture(), valid_archs=archs, valid_api_levels=api_levels
+ lldbplatformutil.getArchitecture(),
+ valid_archs=archs,
+ valid_api_levels=api_levels,
)
return reason if result else None
@@ -537,7 +541,10 @@ def wrapper(*args, **kwargs):
def expectedFlakeyOS(oslist, bugnumber=None, compilers=None):
def fn(self):
- return self.getPlatform() in oslist and self.expectedCompiler(compilers)
+ return (
+ lldbplatformutil.getPlatform() in oslist
+ and lldbplatformutil.expectedCompiler(compilers)
+ )
return expectedFlakey(fn, bugnumber)
@@ -618,9 +625,11 @@ def are_sb_headers_missing():
def skipIfRosetta(bugnumber):
"""Skip a test when running the testsuite on macOS under the Rosetta translation layer."""
- def is_running_rosetta(self):
+ def is_running_rosetta():
if lldbplatformutil.getPlatform() in ["darwin", "macosx"]:
- if (platform.uname()[5] == "arm") and (self.getArchitecture() == "x86_64"):
+ if (platform.uname()[5] == "arm") and (
+ lldbplatformutil.getArchitecture() == "x86_64"
+ ):
return "skipped under Rosetta"
return None
@@ -694,7 +703,7 @@ def skipIfWindows(func):
def skipIfWindowsAndNonEnglish(func):
"""Decorate the item to skip tests that should be skipped on non-English locales on Windows."""
- def is_Windows_NonEnglish(self):
+ def is_Windows_NonEnglish():
if sys.platform != "win32":
return None
kernel = ctypes.windll.kernel32
@@ -724,11 +733,15 @@ def skipUnlessTargetAndroid(func):
def skipIfHostIncompatibleWithRemote(func):
"""Decorate the item to skip tests if binaries built on this host are incompatible."""
- def is_host_incompatible_with_remote(self):
- host_arch = self.getLldbArchitecture()
+ def is_host_incompatible_with_remote():
+ host_arch = lldbplatformutil.getLldbArchitecture()
host_platform = lldbplatformutil.getHostPlatform()
- target_arch = self.getArchitecture()
- target_platform = "darwin" if self.platformIsDarwin() else self.getPlatform()
+ target_arch = lldbplatformutil.getArchitecture()
+ target_platform = (
+ "darwin"
+ if lldbplatformutil.platformIsDarwin()
+ else lldbplatformutil.getPlatform()
+ )
if (
not (target_arch == "x86_64" and host_arch == "i386")
and host_arch != target_arch
@@ -771,8 +784,8 @@ def skipUnlessPlatform(oslist):
def skipUnlessArch(arch):
"""Decorate the item to skip tests unless running on the specified architecture."""
- def arch_doesnt_match(self):
- target_arch = self.getArchitecture()
+ def arch_doesnt_match():
+ target_arch = lldbplatformutil.getArchitecture()
if arch != target_arch:
return "Test only runs on " + arch + ", but target arch is " + target_arch
return None
@@ -797,8 +810,8 @@ def skipIfTargetAndroid(bugnumber=None, api_levels=None, archs=None):
def skipUnlessAppleSilicon(func):
"""Decorate the item to skip tests unless running on Apple Silicon."""
- def not_apple_silicon(test):
- if platform.system() != "Darwin" or test.getArchitecture() not in [
+ def not_apple_silicon():
+ if platform.system() != "Darwin" or lldbplatformutil.getArchitecture() not in [
"arm64",
"arm64e",
]:
@@ -811,10 +824,10 @@ def not_apple_silicon(test):
def skipUnlessSupportedTypeAttribute(attr):
"""Decorate the item to skip test unless Clang supports type __attribute__(attr)."""
- def compiler_doesnt_support_struct_attribute(self):
- compiler_path = self.getCompiler()
+ def compiler_doesnt_support_struct_attribute():
+ compiler_path = lldbplatformutil.getCompiler()
f = tempfile.NamedTemporaryFile()
- cmd = [self.getCompiler(), "-x", "c++", "-c", "-o", f.name, "-"]
+ cmd = [lldbplatformutil.getCompiler(), "-x", "c++", "-c", "-o", f.name, "-"]
p = subprocess.Popen(
cmd,
stdin=subprocess.PIPE,
@@ -833,8 +846,8 @@ def compiler_doesnt_support_struct_attribute(self):
def skipUnlessHasCallSiteInfo(func):
"""Decorate the function to skip testing unless call site info from clang is available."""
- def is_compiler_clang_with_call_site_info(self):
- compiler_path = self.getCompiler()
+ def is_compiler_clang_with_call_site_info():
+ compiler_path = lldbplatformutil.getCompiler()
compiler = os.path.basename(compiler_path)
if not compiler.startswith("clang"):
return "Test requires clang as compiler"
@@ -861,18 +874,21 @@ def is_compiler_clang_with_call_site_info(self):
def skipUnlessThreadSanitizer(func):
"""Decorate the item to skip test unless Clang -fsanitize=thread is supported."""
- def is_compiler_clang_with_thread_sanitizer(self):
+ def is_compiler_clang_with_thread_sanitizer():
if is_running_under_asan():
return "Thread sanitizer tests are disabled when runing under ASAN"
- compiler_path = self.getCompiler()
+ compiler_path = lldbplatformutil.getCompiler()
compiler = os.path.basename(compiler_path)
if not compiler.startswith("clang"):
return "Test requires clang as compiler"
if lldbplatformutil.getPlatform() == "windows":
return "TSAN tests not compatible with 'windows'"
# rdar://28659145 - TSAN tests don't look like they're supported on i386
- if self.getArchitecture() == "i386" and platform.system() == "Darwin":
+ if (
+ lldbplatformutil.getArchitecture() == "i386"
+ and platform.system() == "Darwin"
+ ):
return "TSAN tests not compatible with i386 targets"
if not _compiler_supports(compiler_path, "-fsanitize=thread"):
return "Compiler cannot compile with -fsanitize=thread"
@@ -884,7 +900,7 @@ def is_compiler_clang_with_thread_sanitizer(self):
def skipUnlessUndefinedBehaviorSanitizer(func):
"""Decorate the item to skip test unless -fsanitize=undefined is supported."""
- def is_compiler_clang_with_ubsan(self):
+ def is_compiler_clang_with_ubsan():
if is_running_under_asan():
return (
"Undefined behavior sanitizer tests are disabled when runing under ASAN"
@@ -895,7 +911,7 @@ def is_compiler_clang_with_ubsan(self):
# Try to compile with ubsan turned on.
if not _compiler_supports(
- self.getCompiler(),
+ lldbplatformutil.getCompiler(),
"-fsanitize=undefined",
"int main() { int x = 0; return x / x; }",
outputf,
@@ -910,7 +926,10 @@ def is_compiler_clang_with_ubsan(self):
# Find the ubsan dylib.
# FIXME: This check should go away once compiler-rt gains support for __ubsan_on_report.
- cmd = "%s -fsanitize=undefined -x c - -o - -### 2>&1" % self.getCompiler()
+ cmd = (
+ "%s -fsanitize=undefined -x c - -o - -### 2>&1"
+ % lldbplatformutil.getCompiler()
+ )
with os.popen(cmd) as cc_output:
driver_jobs = cc_output.read()
m = re.search(r'"([^"]+libclang_rt.ubsan_osx_dynamic.dylib)"', driver_jobs)
@@ -942,7 +961,7 @@ def is_running_under_asan():
def skipUnlessAddressSanitizer(func):
"""Decorate the item to skip test unless Clang -fsanitize=thread is supported."""
- def is_compiler_with_address_sanitizer(self):
+ def is_compiler_with_address_sanitizer():
# Also don't run tests that use address sanitizer inside an
# address-sanitized LLDB. The tests don't support that
# configuration.
@@ -951,7 +970,7 @@ def is_compiler_with_address_sanitizer(self):
if lldbplatformutil.getPlatform() == "windows":
return "ASAN tests not compatible with 'windows'"
- if not _compiler_supports(self.getCompiler(), "-fsanitize=address"):
+ if not _compiler_supports(lldbplatformutil.getCompiler(), "-fsanitize=address"):
return "Compiler cannot compile with -fsanitize=address"
return None
@@ -966,8 +985,8 @@ def skipIfAsan(func):
def skipUnlessAArch64MTELinuxCompiler(func):
"""Decorate the item to skip test unless MTE is supported by the test compiler."""
- def is_toolchain_with_mte(self):
- compiler_path = self.getCompiler()
+ def is_toolchain_with_mte():
+ compiler_path = lldbplatformutil.getCompiler()
compiler = os.path.basename(compiler_path)
f = tempfile.NamedTemporaryFile()
if lldbplatformutil.getPlatform() == "windows":
@@ -1053,7 +1072,7 @@ def skipIfLLVMTargetMissing(target):
# Call sysctl on darwin to see if a specified hardware feature is available on this machine.
def skipUnlessFeature(feature):
- def is_feature_enabled(self):
+ def is_feature_enabled():
if platform.system() == "Darwin":
try:
DEVNULL = open(os.devnull, "w")
diff --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
index 2ecfe527e0f251e..4b51366cf284ec3 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -7,12 +7,15 @@
import subprocess
import sys
import os
+from distutils.version import LooseVersion
from urllib.parse import urlparse
# LLDB modules
-from . import configuration
import lldb
+from . import configuration
+from . import lldbtest_config
import lldbsuite.test.lldbplatform as lldbplatform
+from lldbsuite.test.builders import get_builder
def check_first_register_readable(test_case):
@@ -184,3 +187,144 @@ def hasChattyStderr(test_case):
):
return True # The dynamic linker on the device will complain about unknown DT entries
return False
+
+
+def builder_module():
+ return get_builder(sys.platform)
+
+
+def getArchitecture():
+ """Returns the architecture in effect the test suite is running with."""
+ module = builder_module()
+ arch = module.getArchitecture()
+ if arch == "amd64":
+ arch = "x86_64"
+ if arch in ["armv7l", "armv8l"]:
+ arch = "arm"
+ return arch
+
+
+lldbArchitecture = None
+
+
+def getLldbArchitecture():
+ """Returns the architecture of the lldb binary."""
+ global lldbArchitecture
+ if not lldbArchitecture:
+ # These two target settings prevent lldb from doing setup that does
+ # nothing but slow down the end goal of printing the architecture.
+ command = [
+ lldbtest_config.lldbExec,
+ "-x",
+ "-b",
+ "-o",
+ "settings set target.preload-symbols false",
+ "-o",
+ "settings set target.load-script-from-symbol-file false",
+ "-o",
+ "file " + lldbtest_config.lldbExec,
+ ]
+
+ output = subprocess.check_output(command)
+ str = output.decode()
+
+ for line in str.splitlines():
+ m = re.search(r"Current executable set to '.*' \((.*)\)\.", line)
+ if m:
+ lldbArchitecture = m.group(1)
+ break
+
+ return lldbArchitecture
+
+
+def getCompiler():
+ """Returns the compiler in effect the test suite is running with."""
+ module = builder_module()
+ return module.getCompiler()
+
+
+def getCompilerBinary():
+ """Returns the compiler binary the test suite is running with."""
+ return getCompiler().split()[0]
+
+
+def getCompilerVersion():
+ """Returns a string that represents the compiler version.
+ Supports: llvm, clang.
+ """
+ compiler = getCompilerBinary()
+ version_output = subprocess.check_output([compiler, "--version"], errors="replace")
+ m = re.search("version ([0-9.]+)", version_output)
+ if m:
+ return m.group(1)
+ return "unknown"
+
+
+def getDwarfVersion():
+ """Returns the dwarf version generated by clang or '0'."""
+ if configuration.dwarf_version:
+ return str(configuration.dwarf_version)
+ if "clang" in getCompiler():
+ try:
+ triple = builder_module().getTriple(getArchitecture())
+ target = ["-target", triple] if triple else []
+ driver_output = subprocess.check_output(
+ [getCompiler()] + target + "-g -c -x c - -o - -###".split(),
+ stderr=subprocess.STDOUT,
+ )
+ driver_output = driver_output.decode("utf-8")
+ for line in driver_output.split(os.linesep):
+ m = re.search("dwarf-version=([0-9])", line)
+ if m:
+ return m.group(1)
+ except subprocess.CalledProcessError:
+ pass
+ return "0"
+
+
+def expectedCompilerVersion(compiler_version):
+ """Returns True iff compiler_version[1] matches the current compiler version.
+ Use compiler_version[0] to specify the operator used to determine if a match has occurred.
+ Any operator other than the following defaults to an equality test:
+ '>', '>=', "=>", '<', '<=', '=<', '!=', "!" or 'not'
+
+ If the current compiler version cannot be determined, we assume it is close to the top
+ of trunk, so any less-than or equal-to comparisons will return False, and any
+ greater-than or not-equal-to comparisons will return True.
+ """
+ if compiler_version is None:
+ return True
+ operator = str(compiler_version[0])
+ version = compiler_version[1]
+
+ if version is None:
+ return True
+
+ test_compiler_version = getCompilerVersion()
+ if test_compiler_version == "unknown":
+ # Assume the compiler version is at or near the top of trunk.
+ return operator in [">", ">=", "!", "!=", "not"]
+
+ if operator == ">":
+ return LooseVersion(test_compiler_version) > LooseVersion(version)
+ if operator == ">=" or operator == "=>":
+ return LooseVersion(test_compiler_version) >= LooseVersion(version)
+ if operator == "<":
+ return LooseVersion(test_compiler_version) < LooseVersion(version)
+ if operator == "<=" or operator == "=<":
+ return LooseVersion(test_compiler_version) <= LooseVersion(version)
+ if operator == "!=" or operator == "!" or operator == "not":
+ return str(version) not in str(test_compiler_version)
+ return str(version) in str(test_compiler_version)
+
+
+def expectedCompiler(compilers):
+ """Returns True iff any element of compilers is a sub-string of the current compiler."""
+ if compilers is None:
+ return True
+
+ for compiler in compilers:
+ if compiler in getCompiler():
+ return True
+
+ return False
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 19ba0e8c133edcf..90941a7cc3270ce 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -29,7 +29,6 @@
# System modules
import abc
-from distutils.version import LooseVersion
from functools import wraps
import gc
import glob
@@ -58,7 +57,6 @@
from lldbsuite.support import encoded_file
from lldbsuite.support import funcutils
from lldbsuite.support import seven
-from lldbsuite.test.builders import get_builder
from lldbsuite.test_event import build_exception
# See also dotest.parseOptionsAndInitTestdirs(), where the environment variables
@@ -516,7 +514,7 @@ def getsource_if_available(obj):
def builder_module():
- return get_builder(sys.platform)
+ return lldbplatformutil.builder_module()
class Base(unittest2.TestCase):
@@ -1310,82 +1308,29 @@ def isAArch64Windows(self):
def getArchitecture(self):
"""Returns the architecture in effect the test suite is running with."""
- module = builder_module()
- arch = module.getArchitecture()
- if arch == "amd64":
- arch = "x86_64"
- if arch in ["armv7l", "armv8l"]:
- arch = "arm"
- return arch
+ return lldbplatformutil.getArchitecture()
def getLldbArchitecture(self):
"""Returns the architecture of the lldb binary."""
- if not hasattr(self, "lldbArchitecture"):
- # These two target settings prevent lldb from doing setup that does
- # nothing but slow down the end goal of printing the architecture.
- command = [
- lldbtest_config.lldbExec,
- "-x",
- "-b",
- "-o",
- "settings set target.preload-symbols false",
- "-o",
- "settings set target.load-script-from-symbol-file false",
- "-o",
- "file " + lldbtest_config.lldbExec,
- ]
-
- output = check_output(command)
- str = output.decode()
-
- for line in str.splitlines():
- m = re.search(r"Current executable set to '.*' \((.*)\)\.", line)
- if m:
- self.lldbArchitecture = m.group(1)
- break
-
- return self.lldbArchitecture
+ return lldbplatformutil.getLldbArchitecture()
def getCompiler(self):
"""Returns the compiler in effect the test suite is running with."""
- module = builder_module()
- return module.getCompiler()
+ return lldbplatformutil.getCompiler()
def getCompilerBinary(self):
"""Returns the compiler binary the test suite is running with."""
- return self.getCompiler().split()[0]
+ return lldbplatformutil.getCompilerBinary()
def getCompilerVersion(self):
"""Returns a string that represents the compiler version.
Supports: llvm, clang.
"""
- compiler = self.getCompilerBinary()
- version_output = check_output([compiler, "--version"], errors="replace")
- m = re.search("version ([0-9.]+)", version_output)
- if m:
- return m.group(1)
- return "unknown"
+ return lldbplatformutil.getCompilerVersion()
def getDwarfVersion(self):
"""Returns the dwarf version generated by clang or '0'."""
- if configuration.dwarf_version:
- return str(configuration.dwarf_version)
- if "clang" in self.getCompiler():
- try:
- triple = builder_module().getTriple(self.getArchitecture())
- target = ["-target", triple] if triple else []
- driver_output = check_output(
- [self.getCompiler()] + target + "-g -c -x c - -o - -###".split(),
- stderr=STDOUT,
- )
- driver_output = driver_output.decode("utf-8")
- for line in driver_output.split(os.linesep):
- m = re.search("dwarf-version=([0-9])", line)
- if m:
- return m.group(1)
- except CalledProcessError:
- pass
- return "0"
+ return lldbplatformutil.getDwarfVersion()
def platformIsDarwin(self):
"""Returns true if the OS triple for the selected platform is any valid apple OS"""
@@ -1412,41 +1357,11 @@ def expectedCompilerVersion(self, compiler_version):
of trunk, so any less-than or equal-to comparisons will return False, and any
greater-than or not-equal-to comparisons will return True.
"""
- if compiler_version is None:
- return True
- operator = str(compiler_version[0])
- version = compiler_version[1]
-
- if version is None:
- return True
-
- test_compiler_version = self.getCompilerVersion()
- if test_compiler_version == "unknown":
- # Assume the compiler version is at or near the top of trunk.
- return operator in [">", ">=", "!", "!=", "not"]
-
- if operator == ">":
- return LooseVersion(test_compiler_version) > LooseVersion(version)
- if operator == ">=" or operator == "=>":
- return LooseVersion(test_compiler_version) >= LooseVersion(version)
- if operator == "<":
- return LooseVersion(test_compiler_version) < LooseVersion(version)
- if operator == "<=" or operator == "=<":
- return LooseVersion(test_compiler_version) <= LooseVersion(version)
- if operator == "!=" or operator == "!" or operator == "not":
- return str(version) not in str(test_compiler_version)
- return str(version) in str(test_compiler_version)
+ return lldbplatformutil.expectedCompilerVersion(compiler_version)
def expectedCompiler(self, compilers):
"""Returns True iff any element of compilers is a sub-string of the current compiler."""
- if compilers is None:
- return True
-
- for compiler in compilers:
- if compiler in self.getCompiler():
- return True
-
- return False
+ return lldbplatformutil.expectedCompiler(compilers)
def expectedArch(self, archs):
"""Returns True iff any element of archs is a sub-string of the current architecture."""
More information about the lldb-commits
mailing list