llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Charles Zablit (charles-zablit) <details> <summary>Changes</summary> This patch refactors the way we check for the windows version in the `@<!-- -->skipIfWindows` decorator. The new logic reuses the `expectedCompilerVersion` method logic for the parsing and comparison of the version. --- Full diff: https://github.com/llvm/llvm-project/pull/172838.diff 3 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/decorators.py (+10-12) - (modified) lldb/packages/Python/lldbsuite/test/lldbplatformutil.py (+52-16) - (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+8-8) ``````````diff diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py index 6f388cb090f41..8126b39348090 100644 --- a/lldb/packages/Python/lldbsuite/test/decorators.py +++ b/lldb/packages/Python/lldbsuite/test/decorators.py @@ -781,30 +781,28 @@ def skipIfLinux(func): return skipIfPlatform(["linux"])(func) -def skipIfWindows(func=None, major=None, build=None): +def skipIfWindows(func=None, windows_version=None): """Decorate the item to skip tests that should be skipped on Windows.""" def decorator(func): - if major is None and build is None: + if windows_version is None: return skipIfPlatform(["windows"])(func) else: - import platform - import sys + actual_win_version = lldbplatformutil.getWindowsVersion() def version_check(): - check_major = 0 if major is None else major - check_build = 0 if build is None else build - if platform.system() != "Windows": + if actual_win_version == "unknown": return False - win_version = sys.getwindowsversion() - return ( - win_version.major >= check_major - and win_version.build >= check_build + operator, required_windows_version = windows_version + return lldbplatformutil.isExpectedVersion( + actual_version=actual_win_version, + required_version=required_windows_version, + operator=operator, ) return unittest.skipIf( version_check(), - f"Test is skipped on Windows major={major} build={build}", + f"Test is skipped on Windows '{actual_win_version}'", )(func) if func is not None: diff --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py index cea6270695dc0..1e08f0715eefd 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py @@ -8,6 +8,7 @@ import subprocess import sys import os +from typing import Optional from packaging import version from urllib.parse import urlparse @@ -300,6 +301,24 @@ def getCompilerVersion(): return "unknown" +def getWindowsVersion(): + """Returns a string that represents the Windows version. + + The string is a concatenation of the following, eparated by a dot: + - The major version number. + - The build number. + + Example: + - Windows 11 version 24H2 -> "10.26100" + - Windows 10 version 1809 -> "10.17763" + """ + import sys + if sys.platform != "win32": + return "unknown" + windows_version = sys.getwindowsversion() + return f"{windows_version.major}.{windows_version.build}" + + def getDwarfVersion(): """Returns the dwarf version generated by clang or '0'.""" if configuration.dwarf_version: @@ -322,8 +341,34 @@ def getDwarfVersion(): return "0" +def isExpectedVersion( + actual_version: str, required_version: str, operator: str +) -> bool: + """Returns True if actual_version matches the required_version given the operator. + Any operator other than the following defaults to an equality test: + '>', '>=', "=>", '<', '<=', '=<', '!=', "!" or 'not' + + Example: + - actual_version='1.2.0', required_version='1.0.0', operator='>=' returns True + """ + actual_version_ = version.parse(actual_version) + required_version_ = version.parse(required_version) + + if operator == ">": + return actual_version_ > required_version_ + if operator == ">=" or operator == "=>": + return actual_version_ >= required_version_ + if operator == "<": + return actual_version_ < required_version_ + if operator == "<=" or operator == "=<": + return actual_version_ <= required_version_ + if operator == "!=" or operator == "!" or operator == "not": + return actual_version not in required_version + return actual_version in required_version + + def expectedCompilerVersion(compiler_version): - """Returns True iff compiler_version[1] matches the current compiler version. + """Returns True if 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' @@ -342,23 +387,14 @@ def expectedCompilerVersion(compiler_version): test_compiler_version_str = getCompilerVersion() if test_compiler_version_str == "unknown": - # Assume the compiler version is at or near the top of trunk. + # Assume the version is at or near the top of trunk. return operator in [">", ">=", "!", "!=", "not"] - actual_version = version.parse(version_str) - test_compiler_version = version.parse(test_compiler_version_str) - - if operator == ">": - return test_compiler_version > actual_version - if operator == ">=" or operator == "=>": - return test_compiler_version >= actual_version - if operator == "<": - return test_compiler_version < actual_version - if operator == "<=" or operator == "=<": - return test_compiler_version <= actual_version - if operator == "!=" or operator == "!" or operator == "not": - return version_str not in test_compiler_version_str - return version_str in test_compiler_version_str + return isExpectedVersion( + actual_version=test_compiler_version_str, + required_version=version_str, + operator=operator, + ) def expectedCompiler(compilers): diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py index 2fdf1bb42ca09..422d589a11bc5 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -16,7 +16,7 @@ class TestDAP_launch(lldbdap_testcase.DAPTestCaseBase): - @skipIfWindows(major=10, build=1809) + @skipIfWindows(windows_version=['>', '10.17763']) def test_default(self): """ Tests the default launch of a simple program. No arguments, @@ -76,7 +76,7 @@ def test_failing_console(self): r"unexpected value, expected 'internalConsole\', 'integratedTerminal\' or 'externalTerminal\' at arguments.console", ) - @skipIfWindows(major=10, build=1809) + @skipIfWindows(windows_version=['>', '10.17763']) def test_termination(self): """ Tests the correct termination of lldb-dap upon a 'disconnect' @@ -210,7 +210,7 @@ def test_disableSTDIO(self): self.assertEqual(output, "", "expect no program output") @skipIfLinux # shell argument expansion doesn't seem to work on Linux - @skipIfWindows(major=10, build=1809) + @skipIfWindows(windows_version=['>', '10.17763']) @expectedFailureAll(oslist=["freebsd", "netbsd"], bugnumber="llvm.org/pr48349") def test_shellExpandArguments_enabled(self): """ @@ -233,7 +233,7 @@ def test_shellExpandArguments_enabled(self): quote_path, line, 'verify "%s" expanded to "%s"' % (glob, program) ) - @skipIfWindows(major=10, build=1809) + @skipIfWindows(windows_version=['>', '10.17763']) def test_shellExpandArguments_disabled(self): """ Tests the default launch of a simple program with shell expansion @@ -255,7 +255,7 @@ def test_shellExpandArguments_disabled(self): quote_path, line, 'verify "%s" stayed to "%s"' % (glob, glob) ) - @skipIfWindows(major=10, build=1809) + @skipIfWindows(windows_version=['>', '10.17763']) def test_args(self): """ Tests launch of a simple program with arguments @@ -280,7 +280,7 @@ def test_args(self): 'arg[%i] "%s" not in "%s"' % (i + 1, quoted_arg, lines[i]), ) - @skipIfWindows(major=10, build=1809) + @skipIfWindows(windows_version=['>', '10.17763']) def test_environment_with_object(self): """ Tests launch of a simple program with environment variables @@ -557,7 +557,7 @@ def test_terminate_commands(self): output = self.collect_console(pattern=terminateCommands[0]) self.verify_commands("terminateCommands", output, terminateCommands) - @skipIfWindows(major=10, build=1809) + @skipIfWindows(windows_version=['>', '10.17763']) def test_version(self): """ Tests that "initialize" response contains the "version" string the same @@ -640,7 +640,7 @@ def test_stdio_redirection(self): ) @skipIfAsan - @skipIfWindows(major=10, build=1809) + @skipIfWindows(windows_version=['>', '10.17763']) @skipIf(oslist=["linux"], archs=no_match(["x86_64"])) @skipIfBuildType(["debug"]) def test_stdio_redirection_and_console(self): `````````` </details> https://github.com/llvm/llvm-project/pull/172838 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
