From 784b526a1b79923639cda2a707e8c0019c8330e6 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Thu, 9 May 2024 09:47:25 -0600 Subject: [PATCH] Variables cleanup: BoolVariable Part 1 of a series, updating the BoolVariable implementation, tests and docstrings. Note some of the test strings will change again when the change to the "main" module (Variables/__init__.py) change lands. Signed-off-by: Mats Wichmann --- CHANGES.txt | 4 +++- RELEASE.txt | 2 ++ SCons/Variables/BoolVariable.py | 32 +++++++++++++++------------- SCons/Variables/BoolVariableTests.py | 22 +++++-------------- test/Variables/BoolVariable.py | 5 ++--- 5 files changed, 29 insertions(+), 36 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 45ae35ad4c..224f114658 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -49,7 +49,9 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER build pdf versions which are then ignored. - Add the ability to print a Variables object for debugging purposes (provides a __str__ method in the class). - - Update manpage and user guide for Variables usage. + - Clean up Variables: more consistently call them variables (finish the + old change from Options) in docstrings, etc.; some typing and other + tweaks. Update manpage and user guide for Variables usage. RELEASE 4.7.0 - Sun, 17 Mar 2024 17:22:20 -0700 diff --git a/RELEASE.txt b/RELEASE.txt index b582f9a352..f42b979cbe 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -39,6 +39,8 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY NOTE: With this change, user created Actions should now catch and handle expected exceptions (whereas previously many of these were silently caught and suppressed by the SCons Action exection code). +- The implementation of Variables was slightly refactored, there should + not be user-visible changes. FIXES ----- diff --git a/SCons/Variables/BoolVariable.py b/SCons/Variables/BoolVariable.py index 71e44c9cd2..4d998e4d9e 100644 --- a/SCons/Variables/BoolVariable.py +++ b/SCons/Variables/BoolVariable.py @@ -27,7 +27,7 @@ opts = Variables() opts.Add(BoolVariable('embedded', 'build for an embedded system', False)) - ... + env = Environment(variables=opts) if env['embedded']: ... """ @@ -54,13 +54,13 @@ def _text2bool(val: str) -> bool: Raises: ValueError: if *val* cannot be converted to boolean. """ - lval = val.lower() if lval in TRUE_STRINGS: return True if lval in FALSE_STRINGS: return False - raise ValueError("Invalid value for boolean option: %s" % val) + # TODO: leave this check to validator? + raise ValueError(f"Invalid value for boolean variable: {val!r}") def _validator(key, val, env) -> None: @@ -73,23 +73,25 @@ def _validator(key, val, env) -> None: Raises: KeyError: if *key* is not set in *env* UserError: if the value of *key* is not ``True`` or ``False``. - """ - if not env[key] in (True, False): - raise SCons.Errors.UserError( - 'Invalid value for boolean option %s: %s' % (key, env[key]) - ) + """ + if env[key] not in (True, False): + msg = f'Invalid value for boolean variable {key!r}: {env[key]}' + raise SCons.Errors.UserError(msg) from None -def BoolVariable(key, help, default) -> Tuple[str, str, str, Callable, Callable]: +# lint: W0622: Redefining built-in 'help' (redefined-builtin) +def BoolVariable(key, help: str, default) -> Tuple[str, str, str, Callable, Callable]: """Return a tuple describing a boolean SCons Variable. - The input parameters describe a boolean option. Returns a tuple - including the correct converter and validator. - The *help* text will have ``(yes|no)`` automatically appended to show the - valid values. The result is usable as input to :meth:`Add`. + The input parameters describe a boolean variable, using a string + value as described by :const:`TRUE_STRINGS` and :const:`FALSE_STRINGS`. + Returns a tuple including the correct converter and validator. + The *help* text will have ``(yes|no)`` automatically appended to + show the valid values. The result is usable as input to + :meth:`~SCons.Variables.Variables.Add`. """ - help = '%s (yes|no)' % help - return (key, help, default, _validator, _text2bool) + help = f'{help} (yes|no)' + return key, help, default, _validator, _text2bool # Local Variables: # tab-width:4 diff --git a/SCons/Variables/BoolVariableTests.py b/SCons/Variables/BoolVariableTests.py index 9cf3ae1375..4c9b23e805 100644 --- a/SCons/Variables/BoolVariableTests.py +++ b/SCons/Variables/BoolVariableTests.py @@ -67,18 +67,14 @@ 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}" - caught = False - try: + with self.assertRaises(ValueError): o.converter('x') - except ValueError: - caught = True - assert caught, "did not catch expected ValueError for 'x'" def test_validator(self) -> None: """Test the BoolVariable validator""" @@ -98,19 +94,11 @@ def test_validator(self) -> None: o.validator('F', 0, env) # negative checks - caught = False - try: + with self.assertRaises(SCons.Errors.UserError): o.validator('N', 0, env) - except SCons.Errors.UserError: - caught = True - assert caught, "did not catch expected UserError for value %s" % env['N'] - caught = False - try: + with self.assertRaises(KeyError): o.validator('NOSUCHKEY', 0, env) - except KeyError: - caught = True - assert caught, "did not catch expected KeyError for 'NOSUCHKEY'" if __name__ == "__main__": diff --git a/test/Variables/BoolVariable.py b/test/Variables/BoolVariable.py index 9a95d859cf..e29dc7c2df 100644 --- a/test/Variables/BoolVariable.py +++ b/test/Variables/BoolVariable.py @@ -27,7 +27,6 @@ Test the BoolVariable canned Variable type. """ - import TestSCons test = TestSCons.TestSCons() @@ -69,10 +68,10 @@ def check(expect): expect_stderr = """ scons: *** Error converting option: warnings -Invalid value for boolean option: irgendwas +Invalid value for boolean variable: 'irgendwas' """ + test.python_file_line(SConstruct_path, 13) -test.run(arguments='warnings=irgendwas', stderr = expect_stderr, status=2) +test.run(arguments='warnings=irgendwas', stderr=expect_stderr, status=2) test.pass_test()