From 20514636511ce67c8a5384bdf5767831dc6916f9 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Fri, 10 May 2024 16:36:17 -0600 Subject: [PATCH] Variables cleanup: PackageVariable Part 3 of a series, updating the EnumVariable implementation, tests and docstrings. Signed-off-by: Mats Wichmann --- SCons/Variables/PackageVariable.py | 58 ++++++++++++++++--------- SCons/Variables/PackageVariableTests.py | 18 +++----- test/Variables/PackageVariable.py | 13 ++---- 3 files changed, 48 insertions(+), 41 deletions(-) diff --git a/SCons/Variables/PackageVariable.py b/SCons/Variables/PackageVariable.py index 58c8441bd9..fc281250c7 100644 --- a/SCons/Variables/PackageVariable.py +++ b/SCons/Variables/PackageVariable.py @@ -42,15 +42,16 @@ default='yes' ) ) - ... - if env['x11'] == True: + env = Environment(variables=opts) + if env['x11'] is True: dir = ... # search X11 in some standard places ... env['x11'] = dir if env['x11']: ... # build with x11 ... """ -from typing import Tuple, Callable +import os +from typing import Callable, Optional, Tuple import SCons.Errors @@ -60,7 +61,12 @@ DISABLE_STRINGS = ('0', 'no', 'false', 'off', 'disable') def _converter(val): - """ """ + """Convert package variables. + + Returns True or False if one of the recognized truthy or falsy + values is seen, else return the value unchanged (expected to + be a path string). + """ lval = val.lower() if lval in ENABLE_STRINGS: return True @@ -70,35 +76,45 @@ def _converter(val): def _validator(key, val, env, searchfunc) -> None: - """ """ - # NB: searchfunc is currently undocumented and unsupported - # TODO write validator, check for path - import os + """Validate package variable for valid path. + Checks that if a path is given as the value, that pathname actually exists. + """ + # NOTE: searchfunc is currently undocumented and unsupported if env[key] is True: if searchfunc: env[key] = searchfunc(key, val) + # TODO: need to check path, or be sure searchfunc raises. elif env[key] and not os.path.exists(val): - raise SCons.Errors.UserError( - 'Path does not exist for option %s: %s' % (key, val)) + msg = f'Path does not exist for variable {key!r}: {val!r}' + raise SCons.Errors.UserError(msg) from None -def PackageVariable(key, help, default, searchfunc=None) -> Tuple[str, str, str, Callable, Callable]: +# lint: W0622: Redefining built-in 'help' (redefined-builtin) +def PackageVariable( + key: str, help: str, default, searchfunc: Optional[Callable] = None +) -> Tuple[str, str, str, Callable, Callable]: """Return a tuple describing a package list SCons Variable. - The input parameters describe a 'package list' option. Returns - a tuple including the correct converter and validator appended. - The result is usable as input to :meth:`Add` . + The input parameters describe a 'package list' variable. Returns + a tuple with the correct converter and validator appended. + The result is usable as input to :meth:`~SCons.Variables.Variables.Add`. - A 'package list' option may either be 'all', 'none' or a pathname - string. This information is appended to *help*. + A 'package list' variable may either be a truthy string from + :const:`ENABLE_STRINGS`, a falsy string from + :const:`DISABLE_STRINGS`, or a pathname string. + This information is appended to *help* using only one string + each for truthy/falsy. """ # NB: searchfunc is currently undocumented and unsupported - help = '\n '.join( - (help, '( yes | no | /path/to/%s )' % key)) - return (key, help, default, - lambda k, v, e: _validator(k, v, e, searchfunc), - _converter) + help = '\n '.join((help, f'( yes | no | /path/to/{key} )')) + return ( + key, + help, + default, + lambda k, v, e: _validator(k, v, e, searchfunc), + _converter, + ) # Local Variables: # tab-width:4 diff --git a/SCons/Variables/PackageVariableTests.py b/SCons/Variables/PackageVariableTests.py index ad5ba0612a..ed8ec30db4 100644 --- a/SCons/Variables/PackageVariableTests.py +++ b/SCons/Variables/PackageVariableTests.py @@ -32,11 +32,11 @@ class PackageVariableTestCase(unittest.TestCase): def test_PackageVariable(self) -> None: """Test PackageVariable creation""" opts = SCons.Variables.Variables() - opts.Add(SCons.Variables.PackageVariable('test', 'test option help', '/default/path')) + opts.Add(SCons.Variables.PackageVariable('test', 'test build variable help', '/default/path')) o = opts.options[0] assert o.key == 'test', o.key - assert o.help == 'test option help\n ( yes | no | /path/to/test )', repr(o.help) + assert o.help == 'test build variable help\n ( yes | no | /path/to/test )', repr(o.help) assert o.default == '/default/path', o.default assert o.validator is not None, o.validator assert o.converter is not None, o.converter @@ -44,7 +44,7 @@ def test_PackageVariable(self) -> None: def test_converter(self) -> None: """Test the PackageVariable converter""" opts = SCons.Variables.Variables() - opts.Add(SCons.Variables.PackageVariable('test', 'test option help', '/default/path')) + opts.Add(SCons.Variables.PackageVariable('test', 'test build variable help', '/default/path')) o = opts.options[0] @@ -64,11 +64,11 @@ def test_converter(self) -> None: for t in true_values: x = o.converter(t) - assert x, "converter returned false for '%s'" % t + assert x, f"converter returned False for {t!r}" for f in false_values: x = o.converter(f) - assert not x, "converter returned true for '%s'" % f + assert not x, f"converter returned True for {f!r}" x = o.converter('/explicit/path') assert x == '/explicit/path', x @@ -85,7 +85,7 @@ def test_converter(self) -> None: def test_validator(self) -> None: """Test the PackageVariable validator""" opts = SCons.Variables.Variables() - opts.Add(SCons.Variables.PackageVariable('test', 'test option help', '/default/path')) + opts.Add(SCons.Variables.PackageVariable('test', 'test build variable help', '/default/path')) test = TestCmd.TestCmd(workdir='') test.write('exists', 'exists\n') @@ -101,12 +101,8 @@ def test_validator(self) -> None: o.validator('T', '/path', env) o.validator('X', exists, env) - caught = None - try: + with self.assertRaises(SCons.Errors.UserError) as cm: o.validator('X', does_not_exist, env) - except SCons.Errors.UserError: - caught = 1 - assert caught, "did not catch expected UserError" if __name__ == "__main__": diff --git a/test/Variables/PackageVariable.py b/test/Variables/PackageVariable.py index eb143834af..47ee01bcc9 100644 --- a/test/Variables/PackageVariable.py +++ b/test/Variables/PackageVariable.py @@ -1,6 +1,8 @@ #!/usr/bin/env python # -# __COPYRIGHT__ +# MIT License +# +# Copyright The SCons Foundation # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the @@ -20,9 +22,6 @@ # LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. -# - -__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__" """ Test the PackageVariable canned Variable type. @@ -39,8 +38,6 @@ def check(expect): result = test.stdout().split('\n') assert result[1:len(expect)+1] == expect, (result[1:len(expect)+1], expect) - - test.write(SConstruct_path, """\ from SCons.Variables.PackageVariable import PackageVariable PV = PackageVariable @@ -75,13 +72,11 @@ def check(expect): check([test.workpath()]) expect_stderr = """ -scons: *** Path does not exist for option x11: /non/existing/path/ +scons: *** Path does not exist for variable 'x11': '/non/existing/path/' """ + test.python_file_line(SConstruct_path, 14) test.run(arguments='x11=/non/existing/path/', stderr=expect_stderr, status=2) - - test.pass_test() # Local Variables: