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 <[email protected]>

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

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
 -----

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

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__":

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()