Variables testing: confirm space-containing values

The previous commit introduced a change to how the framework handled
arguments, which necessitated some changes in the variables code.
It got too complicated, too many places would need too much logic.
Just accept that the test.run(arguments="...") will never be quite
like the same arguments on the CLI, and just use lists to avoid
things being broken on embedded spaces - those won't be split.
Many tests arleady do this, so it's nothing new. Added a comment
in TestCmd to make it more clear.

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

Author: mwichmann

CHANGES.txt

@@ -21,9 +21,9 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
   From Mats Wichmann:
     - 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
+      result that looks like a dict. Previously, only a single "key" was
+      accepted, and unlike the zero-args case, it was serialized to a
+      string containing just 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

SCons/Variables/EnumVariableTests.py

@@ -159,7 +159,7 @@ def valid(o, v) -> None:
         def invalid(o, v) -> None:
             with self.assertRaises(
                 SCons.Errors.UserError,
-                msg=f"did not catch expected UserError for o = {o.key}, v = {v}",
+                msg=f"did not catch expected UserError for o = {o.key!r}, v = {v!r}",
             ):
                 o.validator('X', v, {})
         table = {
@@ -186,6 +186,22 @@ def invalid(o, v) -> None:
             expected[1](opt1, v)
             expected[2](opt2, v)
 
+        # make sure there are no problems with space-containing entries
+        opts = SCons.Variables.Variables()
+        opts.Add(
+            SCons.Variables.EnumVariable(
+                'test0',
+                help='test option help',
+                default='nospace',
+                allowed_values=['nospace', 'with space'],
+                map={},
+                ignorecase=0,
+            )
+        )
+        opt = opts.options[0]
+        valid(opt, 'nospace')
+        valid(opt, 'with space')
+
 
 if __name__ == "__main__":
     unittest.main()

SCons/Variables/ListVariable.py

@@ -123,8 +123,6 @@ def _converter(val, allowedElems, mapdict) -> _ListVariable:
     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':

test/Variables/EnumVariable.py

@@ -52,8 +52,8 @@ def check(expect):
                allowed_values=('motif', 'gtk', 'kde'),
                map={}, ignorecase=1), # case insensitive
     EV('some', 'some option', 'xaver',
-       allowed_values=('xaver', 'eins'),
-       map={}, ignorecase=2), # make lowercase
+       allowed_values=('xaver', 'eins', 'zwei wörter'),
+       map={}, ignorecase=2), # case lowering
     )
 
 _ = DefaultEnvironment(tools=[])
@@ -89,11 +89,14 @@ def check(expect):
 test.run(arguments='guilib=IrGeNdwas', stderr=expect_stderr, status=2)
 
 expect_stderr = """
-scons: *** Invalid value for enum variable 'some': 'irgendwas'. Valid values are: ('xaver', 'eins')
+scons: *** Invalid value for enum variable 'some': 'irgendwas'. Valid values are: ('xaver', 'eins', 'zwei wörter')
 """ + test.python_file_line(SConstruct_path, 20)
 
 test.run(arguments='some=IrGeNdwas', stderr=expect_stderr, status=2)
 
+test.run(arguments=['some=zwei Wörter'])
+check(['no', 'gtk', 'zwei wörter'])  # case-lowering converter
+
 test.pass_test()
 
 # Local Variables:

test/Variables/PackageVariable.py

@@ -70,6 +70,11 @@ def check(expect):
 test.run(arguments=['x11=%s' % test.workpath()])
 check([test.workpath()])
 
+space_subdir = test.workpath('space subdir')
+test.subdir(space_subdir)
+test.run(arguments=[f'x11={space_subdir}'])
+check([space_subdir])
+
 expect_stderr = """
 scons: *** Path does not exist for variable 'x11': '/non/existing/path/'
 """ + test.python_file_line(SConstruct_path, 13)

test/Variables/PathVariable.py

@@ -106,12 +106,19 @@ def check(expect):
 
 default_file = test.workpath('default_file')
 default_subdir = test.workpath('default_subdir')
+
 existing_subdir = test.workpath('existing_subdir')
 test.subdir(existing_subdir)
 
 existing_file = test.workpath('existing_file')
 test.write(existing_file, "existing_file\n")
 
+space_subdir = test.workpath('space subdir')
+test.subdir(space_subdir)
+
+space_file = test.workpath('space file')
+test.write(space_file, "space_file\n")
+
 non_existing_subdir = test.workpath('non_existing_subdir')
 non_existing_file = test.workpath('non_existing_file')
 
@@ -135,17 +142,22 @@ def check(expect):
 test.run(arguments=['X=%s' % existing_file])
 check([existing_file])
 
-test.run(arguments=['X=%s' % non_existing_file])
-check([non_existing_file])
-
 test.run(arguments=['X=%s' % existing_subdir])
 check([existing_subdir])
 
+test.run(arguments=['X=%s' % space_file])
+check([space_file])
+
+test.run(arguments=['X=%s' % space_subdir])
+check([space_subdir])
+
 test.run(arguments=['X=%s' % non_existing_subdir])
 check([non_existing_subdir])
+test.must_not_exist(non_existing_subdir)
 
+test.run(arguments=['X=%s' % non_existing_file])
+check([non_existing_file])
 test.must_not_exist(non_existing_file)
-test.must_not_exist(non_existing_subdir)
 
 test.write(SConstruct_path, """\
 opts = Variables(args=ARGUMENTS)

testing/framework/TestCmd.py

@@ -1023,7 +1023,7 @@ def __init__(
         interpreter=None,
         workdir=None,
         subdir=None,
-        verbose=None,
+        verbose: int = -1,
         match=None,
         match_stdout=None,
         match_stderr=None,
@@ -1039,7 +1039,7 @@ def __init__(
         self.description_set(description)
         self.program_set(program)
         self.interpreter_set(interpreter)
-        if verbose is None:
+        if verbose == -1:
             try:
                 verbose = max(0, int(os.environ.get('TESTCMD_VERBOSE', 0)))
             except ValueError:
@@ -1178,10 +1178,10 @@ def command_args(self, program=None, interpreter=None, arguments=None):
                 cmd.extend([f"{k}={v}" for k, v in arguments.items()])
                 return cmd
             if isinstance(arguments, str):
-                # 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)
+                # Split into a list for passing to SCons. This *will*
+                # break if the string has embedded spaces as part of a substing -
+                # use a # list to pass those to avoid the problem.
+                arguments = arguments.split()
             cmd.extend(arguments)
         return cmd
 

testing/framework/TestCmdTests.py

@@ -2239,13 +2239,13 @@ def test_command_args(self) -> None:
         expect = ['python', run_env.workpath('prog'), 'arg1', 'arg2']
         self.assertEqual(expect, r)
 
-        r = test.command_args('prog', 'python', 'arg1 arg2="quoted"')
-        expect = ['python', run_env.workpath('prog'), 'arg1', 'arg2="quoted"']
+        r = test.command_args('prog', 'python', 'arg1 arg2=value')
+        expect = ['python', run_env.workpath('prog'), 'arg1', 'arg2=value']
         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"']
+        r = test.command_args('prog', 'python', ['arg1', 'arg2=with space'])
+        expect = ['python', run_env.workpath('prog'), 'arg1', 'arg2=with space']
         with self.subTest():
             self.assertEqual(expect, r)
 

testing/framework/test-framework.rst

@@ -621,6 +621,18 @@ or::
 
    test.must_match(..., match=TestSCons.match_re, ...)
 
+Often you want to supply arguments to SCons when it is invoked
+to run a test, which you can do using an *arguments* parameter::
+
+   test.run(arguments="-O -v FOO=BAR")
+
+One caveat here: the way the parameter is processed is unavoidably
+different from typing on the command line - if you need it not to
+be split on spaces, pre-split it yourself, and pass the list, like::
+
+   test.run(arguments=["-f", "SConstruct2", "FOO=Two Words"])
+
+
 Avoiding tests based on tool existence
 ======================================