Fix ListVariable with a space-containing value

Fix ListVariable handling of a quoted variable value containing spaces.
As a side effect of splitting the former monolithic converter/validator
for ListVariable into separate callbacks, it became possible for
subst to be called twice. The ListVariable converter produces an
instance of a _ListVariable container, and running subst on that
result ends up losing information, so avoid doing so.

While developing a test for this, it turned out the test framework
also didn't handle a quoted argument containing a space, so that
a test case passing arguments to scons via "run(arguments='...')"
could end up with scons seeing a different (broken) command line
than scons invoked with the same arguments typing to a shell prompt.
A regex is now used to more accurately split the "arguments" parameter,
and a unit test was added to the framework tests to validate.

The framework fix had a side effect - it was possible that when run
as part of the test suite, the Variables package could receive a value
still wrapped in quotes, leading to string mismatches ('"with space"'
is not equal to 'with space'), so ListVariable now strips wrapping
quote marks.

Also during testing it turned out that the earlier fix for #4241,
allowing a Variable to declare the value should not be subst'd,
introduced problems for two types which assumed they would always
be passed a string. With subst=False, they could be passed a default
value that had been specified as a bool. Fixed to not fail on that.

Fixes #4585

Signed-off-by: Mats Wichmann <mats@linux.com>

13 files changed, 207 insertions, 81 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index ffebcd0..d4df05c 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -22,8 +22,8 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - env.Dump() now considers the "key" positional argument to be a varargs
       type (zero, one or many). However called, it returns a serialized
       result that looks like a dict. Previously, only one "key" was
-      accepted. and unlike the zero-args case, it was be serialized
-      to a string containing the value without the key. For example, if
+      accepted, and unlike the zero-args case, it was be serialized
+      to a string containing the value (without the key). For example, if
       "print(repr(env.Dump('CC'))" previously returned "'gcc'", it will now
       return "{'CC': 'gcc'}".
     - Add a timeout to test/ninja/default_targets.py - it's gotten stuck on
@@ -40,6 +40,8 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Improve wording of manpage "Functions and Environment Methods" section.
       Make doc function signature style more consistent - tweaks to AddOption,
       DefaultEnvironment and Tool,.
+    - Fix handling of ListVariable when supplying a quoted choice containing
+      a space character (issue #4585).
 
 
 RELEASE 4.8.0 -  Sun, 07 Jul 2024 17:22:20 -0700
diff --git a/RELEASE.txt b/RELEASE.txt
index 2efc001..5b7f9f1 100644
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -46,6 +46,8 @@ FIXES
   to be avaiable. `BoolVariable`, `EnumVariable`, `ListVariable`,
   `PackageVariable` and `PathVariable` are added to `__all__`,
   so this form of import should now work again.
+- Fix handling of ListVariable when supplying a quoted choice containing
+  a space character (issue #4585).
 
 IMPROVEMENTS
 ------------
diff --git a/SCons/Variables/BoolVariable.py b/SCons/Variables/BoolVariable.py
index e1fe62b..815a4b7 100644
--- a/SCons/Variables/BoolVariable.py
+++ b/SCons/Variables/BoolVariable.py
@@ -32,7 +32,7 @@ Usage example::
         ...
 """
 
-from typing import Tuple, Callable
+from typing import Callable, Tuple, Union
 
 import SCons.Errors
 
@@ -42,7 +42,7 @@ TRUE_STRINGS = ('y', 'yes', 'true', 't', '1', 'on', 'all')
 FALSE_STRINGS = ('n', 'no', 'false', 'f', '0', 'off', 'none')
 
 
-def _text2bool(val: str) -> bool:
+def _text2bool(val: Union[str, bool]) -> bool:
     """Convert boolean-like string to boolean.
 
     If *val* looks like it expresses a bool-like value, based on
@@ -54,6 +54,9 @@ def _text2bool(val: str) -> bool:
     Raises:
         ValueError: if *val* cannot be converted to boolean.
     """
+    if isinstance(val, bool):
+        # mainly for the subst=False case: default might be a bool
+        return val
     lval = val.lower()
     if lval in TRUE_STRINGS:
         return True
@@ -63,7 +66,7 @@ def _text2bool(val: str) -> bool:
     raise ValueError(f"Invalid value for boolean variable: {val!r}")
 
 
-def _validator(key, val, env) -> None:
+def _validator(key: str, val, env) -> None:
     """Validate that the value of *key* in *env* is a boolean.
 
     Parameter *val* is not used in the check.
diff --git a/SCons/Variables/BoolVariableTests.py b/SCons/Variables/BoolVariableTests.py
index 4c9b23e..6d950fe 100644
--- a/SCons/Variables/BoolVariableTests.py
+++ b/SCons/Variables/BoolVariableTests.py
@@ -43,7 +43,6 @@ class BoolVariableTestCase(unittest.TestCase):
         """Test the BoolVariable converter"""
         opts = SCons.Variables.Variables()
         opts.Add(SCons.Variables.BoolVariable('test', 'test option help', False))
-
         o = opts.options[0]
 
         true_values = [
@@ -76,6 +75,17 @@ class BoolVariableTestCase(unittest.TestCase):
         with self.assertRaises(ValueError):
             o.converter('x')
 
+        # Synthesize the case where the variable is created with subst=False:
+        # Variables code won't subst before calling the converter,
+        # and we might have pulled a bool from the option default.
+        with self.subTest():
+            x = o.converter(True)
+            assert x, f"converter returned False for {t!r}"
+        with self.subTest():
+            x = o.converter(False)
+            assert not x, f"converter returned False for {t!r}"
+
+
     def test_validator(self) -> None:
         """Test the BoolVariable validator"""
         opts = SCons.Variables.Variables()
diff --git a/SCons/Variables/ListVariable.py b/SCons/Variables/ListVariable.py
index f795307..0042fa8 100644
--- a/SCons/Variables/ListVariable.py
+++ b/SCons/Variables/ListVariable.py
@@ -81,6 +81,7 @@ class _ListVariable(collections.UserList):
         if allowedElems is None:
             allowedElems = []
         super().__init__([_f for _f in initlist if _f])
+        # TODO: why sorted? don't we want to display in the order user gave?
         self.allowedElems = sorted(allowedElems)
 
     def __cmp__(self, other):
@@ -118,7 +119,12 @@ def _converter(val, allowedElems, mapdict) -> _ListVariable:
     The arguments *allowedElems* and *mapdict* are non-standard
     for a :class:`Variables` converter: the lambda in the
     :func:`ListVariable` function arranges for us to be called correctly.
+
+    Incoming values ``all`` and ``none`` are recognized and converted
+    into their expanded form.
     """
+    val = val.replace('"', '')  # trim any wrapping quotes
+    val = val.replace("'", '')
     if val == 'none':
         val = []
     elif val == 'all':
@@ -155,7 +161,7 @@ def _validator(key, val, env) -> None:
     allowedElems = env[key].allowedElems
     if isinstance(val, _ListVariable):  # not substituted, use .data
         notAllowed = [v for v in val.data if v not in allowedElems]
-    else:  # val will be a string
+    else:  # presumably a string
         notAllowed = [v for v in val.split() if v not in allowedElems]
     if notAllowed:
         # Converter only synthesized 'all' and 'none', they are never
diff --git a/SCons/Variables/ListVariableTests.py b/SCons/Variables/ListVariableTests.py
index 9424509..ca5a793 100644
--- a/SCons/Variables/ListVariableTests.py
+++ b/SCons/Variables/ListVariableTests.py
@@ -21,6 +21,8 @@
 # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
+"""Test List Variables elements."""
+
 import copy
 import unittest
 
@@ -50,18 +52,22 @@ class ListVariableTestCase(unittest.TestCase):
         assert o.default == 'one,three'
 
     def test_converter(self) -> None:
-        """Test the ListVariable converter"""
+        """Test the ListVariable converter.
+
+        There is now a separate validator (for a long time validation was
+        in the converter), but it depends on the _ListVariable instance the
+        converter creates, so it's easier to test them in the same function.
+        """
         opts = SCons.Variables.Variables()
         opts.Add(
             SCons.Variables.ListVariable(
                 'test',
                 'test option help',
-                'all',
-                ['one', 'two', 'three'],
-                {'ONE': 'one', 'TWO': 'two'},
+                default='all',
+                names=['one', 'two', 'three'],
+                map={'ONE': 'one', 'TWO': 'two'},
             )
         )
-
         o = opts.options[0]
 
         x = o.converter('all')
@@ -110,10 +116,48 @@ class ListVariableTestCase(unittest.TestCase):
         # invalid value should convert (no change) without error
         x = o.converter('no_match')
         assert str(x) == 'no_match', x
-        # ... and fail to validate
+
+        # validator checks
+
+        # first, the one we just set up
         with self.assertRaises(SCons.Errors.UserError):
             o.validator('test', 'no_match', {"test": x})
 
+        # now a new option, this time with a name w/ space in it (issue #4585)
+        opts.Add(
+            SCons.Variables.ListVariable(
+                'test2',
+                help='test2 option help',
+                default='two',
+                names=['one', 'two', 'three', 'four space'],
+            )
+        )
+        o = opts.options[1]
+
+        def test_valid(opt, seq):
+            """Validate a ListVariable value.
+
+            Call the converter manually, since we need the _ListVariable
+            object to pass to the validator - this would normally be done
+            by the Variables.Update method.
+            """
+            x = opt.converter(seq)
+            self.assertIsNone(opt.validator(opt.key, x, {opt.key: x}))
+
+        with self.subTest():
+            test_valid(o, 'one')
+        with self.subTest():
+            test_valid(o, 'one,two,three')
+        with self.subTest():
+            test_valid(o, 'all')
+        with self.subTest():
+            test_valid(o, 'none')
+        with self.subTest():
+            test_valid(o, 'four space')
+        with self.subTest():
+            test_valid(o, 'one,four space')
+
+
     def test_copy(self) -> None:
         """Test copying a ListVariable like an Environment would"""
         opts = SCons.Variables.Variables()
diff --git a/SCons/Variables/PackageVariable.py b/SCons/Variables/PackageVariable.py
index fc28125..9fa5140 100644
--- a/SCons/Variables/PackageVariable.py
+++ b/SCons/Variables/PackageVariable.py
@@ -51,7 +51,7 @@ Can be used as a replacement for autoconf's ``--with-xxx=yyy`` ::
 """
 
 import os
-from typing import Callable, Optional, Tuple
+from typing import Callable, Optional, Tuple, Union
 
 import SCons.Errors
 
@@ -60,13 +60,16 @@ __all__ = ['PackageVariable',]
 ENABLE_STRINGS = ('1', 'yes', 'true',  'on', 'enable', 'search')
 DISABLE_STRINGS = ('0', 'no',  'false', 'off', 'disable')
 
-def _converter(val):
+def _converter(val: Union[str, bool]) -> Union[str, bool]:
     """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).
     """
+    if isinstance(val, bool):
+        # mainly for the subst=False case: default might be a bool
+        return val
     lval = val.lower()
     if lval in ENABLE_STRINGS:
         return True
@@ -75,7 +78,7 @@ def _converter(val):
     return val
 
 
-def _validator(key, val, env, searchfunc) -> None:
+def _validator(key: str, val, env, searchfunc) -> None:
     """Validate package variable for valid path.
 
     Checks that if a path is given as the value, that pathname actually exists.
diff --git a/SCons/Variables/PackageVariableTests.py b/SCons/Variables/PackageVariableTests.py
index 0d8aa6b..00cf1e3 100644
--- a/SCons/Variables/PackageVariableTests.py
+++ b/SCons/Variables/PackageVariableTests.py
@@ -82,6 +82,16 @@ class PackageVariableTestCase(unittest.TestCase):
         x = o.converter(str(False))
         assert not x, "converter returned a string when given str(False)"
 
+        # Synthesize the case where the variable is created with subst=False:
+        # Variables code won't subst before calling the converter,
+        # and we might have pulled a bool from the option default.
+        with self.subTest():
+            x = o.converter(True)
+            assert x, f"converter returned False for {t!r}"
+        with self.subTest():
+            x = o.converter(False)
+            assert not x, f"converter returned False for {t!r}"
+
     def test_validator(self) -> None:
         """Test the PackageVariable validator"""
         opts = SCons.Variables.Variables()
diff --git a/SCons/Variables/PathVariable.py b/SCons/Variables/PathVariable.py
index d5988ac..43904e6 100644
--- a/SCons/Variables/PathVariable.py
+++ b/SCons/Variables/PathVariable.py
@@ -93,12 +93,12 @@ class _PathVariableClass:
     """
 
     @staticmethod
-    def PathAccept(key, val, env) -> None:
+    def PathAccept(key: str, val, env) -> None:
         """Validate path with no checking."""
         return
 
     @staticmethod
-    def PathIsDir(key, val, env) -> None:
+    def PathIsDir(key: str, val, env) -> None:
         """Validate path is a directory."""
         if os.path.isdir(val):
             return
@@ -109,7 +109,7 @@ class _PathVariableClass:
         raise SCons.Errors.UserError(msg)
 
     @staticmethod
-    def PathIsDirCreate(key, val, env) -> None:
+    def PathIsDirCreate(key: str, val, env) -> None:
         """Validate path is a directory, creating if needed."""
         if os.path.isdir(val):
             return
@@ -123,7 +123,7 @@ class _PathVariableClass:
             raise SCons.Errors.UserError(msg) from exc
 
     @staticmethod
-    def PathIsFile(key, val, env) -> None:
+    def PathIsFile(key: str, val, env) -> None:
         """Validate path is a file."""
         if not os.path.isfile(val):
             if os.path.isdir(val):
@@ -133,7 +133,7 @@ class _PathVariableClass:
             raise SCons.Errors.UserError(msg)
 
     @staticmethod
-    def PathExists(key, val, env) -> None:
+    def PathExists(key: str, val, env) -> None:
         """Validate path exists."""
         if not os.path.exists(val):
             msg = f'Path for variable {key!r} does not exist: {val}'
diff --git a/SCons/Variables/__init__.py b/SCons/Variables/__init__.py
index 501505f..2832526 100644
--- a/SCons/Variables/__init__.py
+++ b/SCons/Variables/__init__.py
@@ -61,7 +61,10 @@ class Variable:
 
     def __str__(self) -> str:
         """Provide a way to "print" a Variable object."""
-        return f"({self.key!r}, {self.aliases}, {self.help!r}, {self.default!r}, {self.validator}, {self.converter})"
+        return (
+            f"({self.key!r}, {self.aliases}, {self.help!r}, {self.default!r}, "
+            f"validator={self.validator}, converter={self.converter})"
+        )
 
 
 class Variables:
@@ -287,7 +290,13 @@ class Variables:
         for option in self.options:
             if option.validator and option.key in values:
                 if option.do_subst:
-                    value = env.subst('${%s}' % option.key)
+                    val = env[option.key]
+                    if not SCons.Util.is_String(val):
+                        # issue #4585: a _ListVariable should not be further
+                        #    substituted, breaks on values with spaces.
+                        value = val
+                    else:
+                        value = env.subst('${%s}' % option.key)
                 else:
                     value = env[option.key]
                 option.validator(option.key, value, env)
diff --git a/test/Variables/ListVariable.py b/test/Variables/ListVariable.py
index a322f9b..d6356e8 100644
--- a/test/Variables/ListVariable.py
+++ b/test/Variables/ListVariable.py
@@ -37,7 +37,7 @@ SConstruct_path = test.workpath('SConstruct')
 
 def check(expected):
     result = test.stdout().split('\n')
-    r = result[1:len(expected)+1]
+    r = result[1 : len(expected) + 1]
     assert r == expected, (r, expected)
 
 
@@ -45,17 +45,24 @@ test.write(SConstruct_path, """\
 from SCons.Variables.ListVariable import ListVariable as LV
 from SCons.Variables import ListVariable
 
-list_of_libs = Split('x11 gl qt ical')
+list_of_libs = Split('x11 gl qt ical') + ["with space"]
 
 optsfile = 'scons.variables'
 opts = Variables(optsfile, args=ARGUMENTS)
 opts.AddVariables(
-    ListVariable('shared',
-               'libraries to build as shared libraries',
-               'all',
-               names = list_of_libs,
-               map = {'GL':'gl', 'QT':'qt'}),
-    LV('listvariable', 'listvariable help', 'all', names=['l1', 'l2', 'l3'])
+    ListVariable(
+        'shared',
+        'libraries to build as shared libraries',
+        default='all',
+        names=list_of_libs,
+        map={'GL': 'gl', 'QT': 'qt'},
+    ),
+    LV(
+        'listvariable',
+        'listvariable help',
+        default='all',
+        names=['l1', 'l2', 'l3'],
+    ),
 )
 
 _ = DefaultEnvironment(tools=[])  # test speedup
@@ -70,7 +77,7 @@ if 'ical' in env['shared']:
 else:
     print('0')
 
-print(" ".join(env['shared']))
+print(",".join(env['shared']))
 
 print(env.subst('$shared'))
 # Test subst_path() because it's used in $CPPDEFINES expansions.
@@ -79,14 +86,27 @@ Default(env.Alias('dummy', None))
 """)
 
 test.run()
-check(['all', '1', 'gl ical qt x11', 'gl ical qt x11',
-       "['gl ical qt x11']"])
+check(
+    [
+        'all',
+        '1',
+        'gl,ical,qt,with space,x11',
+        'gl ical qt with space x11',
+        "['gl ical qt with space x11']",
+    ]
+)
 
-expect = "shared = 'all'"+os.linesep+"listvariable = 'all'"+os.linesep
+expect = "shared = 'all'" + os.linesep + "listvariable = 'all'" + os.linesep
 test.must_match(test.workpath('scons.variables'), expect)
-
-check(['all', '1', 'gl ical qt x11', 'gl ical qt x11',
-       "['gl ical qt x11']"])
+check(
+    [
+        'all',
+        '1',
+        'gl,ical,qt,with space,x11',
+        'gl ical qt with space x11',
+        "['gl ical qt with space x11']",
+    ]
+)
 
 test.run(arguments='shared=none')
 check(['none', '0', '', '', "['']"])
@@ -95,74 +115,80 @@ test.run(arguments='shared=')
 check(['none', '0', '', '', "['']"])
 
 test.run(arguments='shared=x11,ical')
-check(['ical,x11', '1', 'ical x11', 'ical x11',
-       "['ical x11']"])
+check(['ical,x11', '1', 'ical,x11', 'ical x11', "['ical x11']"])
 
 test.run(arguments='shared=x11,,ical,,')
-check(['ical,x11', '1', 'ical x11', 'ical x11',
-       "['ical x11']"])
+check(['ical,x11', '1', 'ical,x11', 'ical x11', "['ical x11']"])
 
 test.run(arguments='shared=GL')
 check(['gl', '0', 'gl', 'gl'])
 
 test.run(arguments='shared=QT,GL')
-check(['gl,qt', '0', 'gl qt', 'gl qt', "['gl qt']"])
+check(['gl,qt', '0', 'gl,qt', 'gl qt', "['gl qt']"])
 
+#test.run(arguments='shared="with space"')
+#check(['with space', '0', 'with space', 'with space', "['with space']"])
 
 expect_stderr = """
-scons: *** Invalid value(s) for variable 'shared': 'foo'. Valid values are: gl,ical,qt,x11,all,none
-""" + test.python_file_line(SConstruct_path, 18)
+scons: *** Invalid value(s) for variable 'shared': 'foo'. Valid values are: gl,ical,qt,with space,x11,all,none
+""" + test.python_file_line(SConstruct_path, 25)
 
 test.run(arguments='shared=foo', stderr=expect_stderr, status=2)
 
 # be paranoid in testing some more combinations
 
 expect_stderr = """
-scons: *** Invalid value(s) for variable 'shared': 'foo'. Valid values are: gl,ical,qt,x11,all,none
-""" + test.python_file_line(SConstruct_path, 18)
+scons: *** Invalid value(s) for variable 'shared': 'foo'. Valid values are: gl,ical,qt,with space,x11,all,none
+""" + test.python_file_line(SConstruct_path, 25)
 
 test.run(arguments='shared=foo,ical', stderr=expect_stderr, status=2)
 
 expect_stderr = """
-scons: *** Invalid value(s) for variable 'shared': 'foo'. Valid values are: gl,ical,qt,x11,all,none
-""" + test.python_file_line(SConstruct_path, 18)
+scons: *** Invalid value(s) for variable 'shared': 'foo'. Valid values are: gl,ical,qt,with space,x11,all,none
+""" + test.python_file_line(SConstruct_path, 25)
 
 test.run(arguments='shared=ical,foo', stderr=expect_stderr, status=2)
 
 expect_stderr = """
-scons: *** Invalid value(s) for variable 'shared': 'foo'. Valid values are: gl,ical,qt,x11,all,none
-""" + test.python_file_line(SConstruct_path, 18)
+scons: *** Invalid value(s) for variable 'shared': 'foo'. Valid values are: gl,ical,qt,with space,x11,all,none
+""" + test.python_file_line(SConstruct_path, 25)
 
 test.run(arguments='shared=ical,foo,x11', stderr=expect_stderr, status=2)
 
 expect_stderr = """
-scons: *** Invalid value(s) for variable 'shared': 'foo,bar'. Valid values are: gl,ical,qt,x11,all,none
-""" + test.python_file_line(SConstruct_path, 18)
+scons: *** Invalid value(s) for variable 'shared': 'foo,bar'. Valid values are: gl,ical,qt,with space,x11,all,none
+""" + test.python_file_line(SConstruct_path, 25)
 
 test.run(arguments='shared=foo,x11,,,bar', stderr=expect_stderr, status=2)
 
-test.write('SConstruct', """
+test.write('SConstruct2', """\
 from SCons.Variables import ListVariable
 
 opts = Variables(args=ARGUMENTS)
 opts.AddVariables(
-    ListVariable('gpib',
-               'comment',
-               ['ENET', 'GPIB'],
-               names = ['ENET', 'GPIB', 'LINUX_GPIB', 'NO_GPIB']),
-    )
+    ListVariable(
+        'gpib',
+        'comment',
+        default=['ENET', 'GPIB'],
+        names=['ENET', 'GPIB', 'LINUX_GPIB', 'NO_GPIB'],
+    ),
+)
 
 DefaultEnvironment(tools=[])  # test speedup
-env = Environment(variables=opts)
+env = Environment(tools=[], variables=opts)
 Help(opts.GenerateHelpText(env))
 
 print(env['gpib'])
 Default(env.Alias('dummy', None))
 """)
 
-test.run(stdout=test.wrap_stdout(read_str="ENET,GPIB\n", build_str="""\
+test.run(
+    arguments="-f SConstruct2",
+    stdout=test.wrap_stdout(read_str="ENET,GPIB\n",
+    build_str="""\
 scons: Nothing to be done for `dummy'.
-"""))
+""")
+)
 
 test.pass_test()
 
diff --git a/testing/framework/TestCmd.py b/testing/framework/TestCmd.py
index fba0b75..5d9aed9 100644
--- a/testing/framework/TestCmd.py
+++ b/testing/framework/TestCmd.py
@@ -1178,7 +1178,10 @@ class TestCmd:
                 cmd.extend([f"{k}={v}" for k, v in arguments.items()])
                 return cmd
             if isinstance(arguments, str):
-                arguments = arguments.split()
+                # Split into a list for passing to SCons - don't lose
+                # quotes, and don't break apart quoted substring with space.
+                # str split() fails on the spaces, shlex.split() on the quotes.
+                arguments = re.findall(r"(?:\".*?\"|\S)+", arguments)
             cmd.extend(arguments)
         return cmd
 
diff --git a/testing/framework/TestCmdTests.py b/testing/framework/TestCmdTests.py
index 4340d90..2a6cceb 100644
--- a/testing/framework/TestCmdTests.py
+++ b/testing/framework/TestCmdTests.py
@@ -1,8 +1,5 @@
 #!/usr/bin/env python
-"""
-Unit tests for the TestCmd.py module.
-"""
-
+#
 # Copyright 2000-2010 Steven Knight
 # This module is free software, and you may redistribute it and/or modify
 # it under the same terms as Python itself, so long as this copyright message
@@ -19,6 +16,9 @@ Unit tests for the TestCmd.py module.
 # AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
 
+"""
+Unit tests for the TestCmd.py module.
+"""
 
 import os
 import shutil
@@ -2225,59 +2225,67 @@ class command_args_TestCase(TestCmdTestCase):
 
         r = test.command_args('prog')
         expect = [run_env.workpath('prog')]
-        assert r == expect, (expect, r)
+        self.assertEqual(expect, r)
 
         r = test.command_args(test.workpath('new_prog'))
         expect = [test.workpath('new_prog')]
-        assert r == expect, (expect, r)
+        self.assertEqual(expect, r)
 
         r = test.command_args('prog', 'python')
         expect = ['python', run_env.workpath('prog')]
-        assert r == expect, (expect, r)
+        self.assertEqual(expect, r)
 
         r = test.command_args('prog', 'python', 'arg1 arg2')
         expect = ['python', run_env.workpath('prog'), 'arg1', 'arg2']
-        assert r == expect, (expect, r)
+        self.assertEqual(expect, r)
+
+        r = test.command_args('prog', 'python', 'arg1 arg2="quoted"')
+        expect = ['python', run_env.workpath('prog'), 'arg1', 'arg2="quoted"']
+        with self.subTest():
+            self.assertEqual(expect, r)
+
+        r = test.command_args('prog', 'python', 'arg1 arg2="quoted with space"')
+        expect = ['python', run_env.workpath('prog'), 'arg1', 'arg2="quoted with space"']
+        with self.subTest():
+            self.assertEqual(expect, r)
 
         test.program_set('default_prog')
         default_prog = run_env.workpath('default_prog')
 
         r = test.command_args()
         expect = [default_prog]
-        assert r == expect, (expect, r)
+        self.assertEqual(expect, r)
 
         r = test.command_args(interpreter='PYTHON')
         expect = ['PYTHON', default_prog]
-        assert r == expect, (expect, r)
+        self.assertEqual(expect, r)
 
         r = test.command_args(interpreter='PYTHON', arguments='arg3 arg4')
         expect = ['PYTHON', default_prog, 'arg3', 'arg4']
-        assert r == expect, (expect, r)
+        self.assertEqual(expect, r)
 
         # Test arguments = dict
         r = test.command_args(interpreter='PYTHON', arguments={'VAR1':'1'})
         expect = ['PYTHON', default_prog, 'VAR1=1']
-        assert r == expect, (expect, r)
-
+        self.assertEqual(expect, r)
 
         test.interpreter_set('default_python')
 
         r = test.command_args()
         expect = ['default_python', default_prog]
-        assert r == expect, (expect, r)
+        self.assertEqual(expect, r)
 
         r = test.command_args(arguments='arg5 arg6')
         expect = ['default_python', default_prog, 'arg5', 'arg6']
-        assert r == expect, (expect, r)
+        self.assertEqual(expect, r)
 
         r = test.command_args('new_prog_1')
         expect = [run_env.workpath('new_prog_1')]
-        assert r == expect, (expect, r)
+        self.assertEqual(expect, r)
 
         r = test.command_args(program='new_prog_2')
         expect = [run_env.workpath('new_prog_2')]
-        assert r == expect, (expect, r)
-
+        self.assertEqual(expect, r)
 
 
 class start_TestCase(TestCmdTestCase):