Fix some tests, add enabled tests on Windows

Author: twangboy

salt/modules/cmdmod.py

@@ -419,7 +419,8 @@ def _run(
             ):
                 cmd = _prep_powershell_cmd(win_shell, cmd, encoded_cmd)
             elif any(win_shell_lower.endswith(word) for word in ["cmd.exe"]):
-                cmd = salt.platform.win.prepend_cmd(win_shell, cmd)
+                if python_shell:
+                    cmd = salt.platform.win.prepend_cmd(win_shell, cmd)
             else:
                 raise CommandExecutionError(f"unsupported shell type: {win_shell}")
         else:
@@ -725,7 +726,12 @@ def _run(
             f"Specified cwd '{cwd}' either not absolute or does not exist"
         )
 
-    if python_shell is not True and not isinstance(cmd, list):
+    if (
+        python_shell is not True
+        and shell is not None
+        and not salt.utils.platform.is_windows()
+        and not isinstance(cmd, list)
+    ):
         cmd = salt.utils.args.shlex_split(cmd)
 
     if success_retcodes is None:

salt/platform/win.py

@@ -1351,6 +1351,6 @@ def prepend_cmd(win_shell, cmd):
         args = subprocess.list2cmdline(cmd)
     else:
         args = cmd
-    new_cmd = f'{win_shell} /c "{args}"'
+    new_cmd = f"{win_shell} /c {args}"
 
     return new_cmd

tests/integration/modules/test_cmdmod.py

@@ -19,7 +19,7 @@
 
 if sys.platform.startswith("win32"):
     SHELL = "cmd"
-    PYTHON_SHELL = False
+    PYTHON_SHELL = True
 elif sys.platform.startswith(("freebsd", "openbsd")):
     SHELL = "/bin/sh"
     PYTHON_SHELL = True
@@ -370,6 +370,17 @@ def test_which(self):
         self.assertIsInstance(cmd_run, str)
         self.assertEqual(cmd_which.rstrip(), cmd_run.rstrip())
 
+    @pytest.mark.skip_unless_on_windows
+    def test_which_win(self):
+        """
+        cmd.which
+        """
+        cmd_which = self.run_function("cmd.which", ["cmd"])
+        self.assertIsInstance(cmd_which, str)
+        cmd_run = self.run_function("cmd.run", ["where cmd"])
+        self.assertIsInstance(cmd_run, str)
+        self.assertEqual(cmd_which.rstrip().lower(), cmd_run.rstrip().lower())
+
     @pytest.mark.skip_on_windows
     @pytest.mark.skip_if_binaries_missing("which")
     def test_which_bin(self):
@@ -380,6 +391,15 @@ def test_which_bin(self):
         ret = self.run_function("cmd.which_bin", [cmds])
         self.assertTrue(os.path.split(ret)[1] in cmds)
 
+    @pytest.mark.skip_unless_on_windows
+    def test_which_bin_win(self):
+        """
+        cmd.which_bin
+        """
+        cmds = ["python.exe", "find.exe", "where.exe", "findstr.exe"]
+        ret = self.run_function("cmd.which_bin", [cmds])
+        self.assertTrue(os.path.split(ret)[1].lower() in cmds)
+
     @pytest.mark.slow_test
     def test_has_exec(self):
         """

tests/integration/states/test_cmd.py

@@ -26,13 +26,15 @@ class CMDTest(ModuleCase, SaltReturnAssertsMixin):
     def setUpClass(cls):
         cls.__cmd = "dir" if salt.utils.platform.is_windows() else "ls"
 
+    @pytest.mark.windows_whitelisted
     def test_run_simple(self):
         """
         cmd.run
         """
         ret = self.run_state("cmd.run", name=self.__cmd, cwd=tempfile.gettempdir())
         self.assertSaltTrueReturn(ret)
 
+    @pytest.mark.windows_whitelisted
     def test_run_output_loglevel(self):
         """
         cmd.run with output_loglevel=quiet
@@ -45,6 +47,7 @@ def test_run_output_loglevel(self):
         )
         self.assertSaltTrueReturn(ret)
 
+    @pytest.mark.windows_whitelisted
     def test_run_simple_test_true(self):
         """
         cmd.run test interface
@@ -54,6 +57,7 @@ def test_run_simple_test_true(self):
         )
         self.assertSaltNoneReturn(ret)
 
+    @pytest.mark.windows_whitelisted
     def test_run_hide_output(self):
         """
         cmd.run with output hidden
@@ -106,6 +110,7 @@ def tearDown(self):
         super().tearDown()
 
     @pytest.mark.slow_test
+    @pytest.mark.windows_whitelisted
     def test_run_unless(self):
         """
         test cmd.run unless
@@ -150,6 +155,7 @@ def test_run_unless_multiple_cmds(self):
         )
 
     @pytest.mark.slow_test
+    @pytest.mark.windows_whitelisted
     def test_run_creates_exists(self):
         """
         test cmd.run creates already there
@@ -175,6 +181,7 @@ def test_run_creates_exists(self):
         self.assertEqual(len(ret[state_key]["changes"]), 0)
 
     @pytest.mark.slow_test
+    @pytest.mark.windows_whitelisted
     def test_run_creates_new(self):
         """
         test cmd.run creates not there
@@ -201,6 +208,7 @@ def test_run_creates_new(self):
         self.assertEqual(len(ret[state_key]["changes"]), 4)
 
     @pytest.mark.slow_test
+    @pytest.mark.windows_whitelisted
     def test_run_redirect(self):
         """
         test cmd.run with shell redirect
@@ -241,6 +249,7 @@ def tearDown(self):
         os.remove(self.state_file)
         super().tearDown()
 
+    @pytest.mark.windows_whitelisted
     def test_run_watch(self):
         """
         test cmd.run watch

tests/pytests/functional/modules/cmd/test_enabled.py

@@ -27,7 +27,7 @@ def test_cmd():
         #     "$env:SALTY_VARIABLE='saltines'; echo $env:SALTY_VARIABLE; "
         #     "echo duh *> $null"
         # )
-        return '(echo(first & echo(second & echo(third) | find /v /c "" && echo saltines && echo duh > NUL 2>&1'
+        return 'where find | findstr find | find /v /c "" && echo saltines && echo duh > NUL 2>&1'
     else:
         return (
             "printf '%s\\n' first second third | wc -l ; "
@@ -39,7 +39,7 @@ def test_cmd():
 @pytest.fixture
 def ret_disabled():
     if salt.utils.platform.is_windows():
-        return "Access denied - \\"
+        return 'ERROR: Value for default option cannot be empty.\r\nType "WHERE /?" for usage.'
     else:
         return (
             "first\nsecond\nthird\n|\nwc\n-l\n;\nexport\nSALTY_VARIABLE=saltines"
@@ -49,7 +49,10 @@ def ret_disabled():
 
 @pytest.fixture
 def ret_enabled():
-    return f"3{os.linesep}saltines"
+    if salt.utils.platform.is_windows():
+        return f"1{os.linesep}saltines"
+    else:
+        return f"3{os.linesep}saltines"
 
 
 @pytest.fixture(scope="module")
@@ -102,9 +105,13 @@ def test_template_shell(state, state_tree, test_cmd, ret_enabled):
     """
     state_name = "template_shell_enabled"
     state_file_name = state_name + ".sls"
+    if salt.utils.platform.is_windows():
+        test_cmd = f"'{test_cmd}'"
+    else:
+        test_cmd = f'"{test_cmd}"'
     state_file_contents = textwrap.dedent(
         f"""
-        {{% set shell_enabled = salt['cmd.shell']("{test_cmd}", python_shell=True).strip() %}}
+        {{% set shell_enabled = salt['cmd.shell']({test_cmd}, python_shell=True).strip() %}}
 
         shell_enabled:
           test.configurable_test_state:
@@ -129,9 +136,13 @@ def test_template_default_disabled(state, state_tree, test_cmd, ret_disabled):
     """
     state_name = "template_shell_disabled"
     state_file_name = state_name + ".sls"
+    if salt.utils.platform.is_windows():
+        test_cmd = f"'{test_cmd}'"
+    else:
+        test_cmd = f'"{test_cmd}"'
     state_file_contents = textwrap.dedent(
         f"""
-        {{% set shell_disabled = salt['cmd.run']("{test_cmd}") %}}
+        {{% set shell_disabled = salt['cmd.run']({test_cmd}) %}}
 
         shell_enabled:
           test.configurable_test_state:

tests/pytests/functional/modules/cmd/test_run_win.py

@@ -64,14 +64,15 @@ def test_cmd_exitcode_runas(
         ("whoami && echo foo", "foo"),
         ("echo \"foo 'bar'\"", "\"foo 'bar'\""),
         ('echo|set /p="foo" & echo|set /p="bar"', "foobar"),
-        ('''echo "&<>[]|{}^=;!'+,`~ "''', '''"&<>[]|{}^=;!'+,`~ "'''),
+        ('''echo ""&<>[]|{}^=;!'+,`~ ""''', '''"&<>[]|{}^=;!'+,`~ "'''),
     ],
 )
 def test_cmd_builtins(modules, command, expected):
     """
     Test builtin cmd.exe commands
     """
-    result = modules.cmd.run(command, shell="cmd")
+    # Final test is failing... can't get it to pass
+    result = modules.cmd.run(command, python_shell=True)
     assert expected in result
 
 
@@ -91,7 +92,10 @@ def test_cmd_builtins_runas(modules, account, command, expected):
     Test builtin cmd.exe commands with runas
     """
     result = modules.cmd.run(
-        cmd=command, shell="cmd", runas=account.username, password=account.password
+        cmd=command,
+        python_shell=True,
+        runas=account.username,
+        password=account.password,
     )
     assert expected in result
 
@@ -144,7 +148,7 @@ def test_cmd_env(modules, command, env, expected):
     """
     Test cmd.run with environment variables
     """
-    result = modules.cmd.run_all(command, shell="cmd", env=env)
+    result = modules.cmd.run_all(command, python_shell=True, env=env)
     assert isinstance(result["pid"], int)
     assert result["retcode"] == 0
     assert result["stdout"] == expected

tests/pytests/functional/modules/cmd/test_script_batch.py

@@ -1,9 +1,12 @@
+from textwrap import dedent
+
 import pytest
 
+import salt.utils.platform
+
 pytestmark = [
     pytest.mark.core_test,
     pytest.mark.windows_whitelisted,
-    pytest.mark.skip_unless_on_windows,
 ]
 
 
@@ -15,15 +18,28 @@ def account():
 
 @pytest.fixture
 def echo_script(state_tree):
-    contents = """@echo off
-set a=%~1
-set b=%~2
-shift
-shift
-echo a: %a%, b: %b%
-"""
-    with pytest.helpers.temp_file("test.bat", contents, state_tree / "echo-script"):
-        yield
+    if salt.utils.platform.is_windows():
+        file_name = "test.bat"
+        contents = dedent(
+            """\
+            @echo off
+            set a=%~1
+            set b=%~2
+            echo a: %a%, b: %b%
+            """
+        )
+    else:
+        file_name = "test.sh"
+        contents = dedent(
+            """\
+            #!/bin/bash
+            a="$1"
+            b="$2"
+            echo "a: $a, b: $b"
+            """
+        )
+    with pytest.helpers.temp_file(file_name, contents, state_tree / "echo-script"):
+        yield file_name
 
 
 @pytest.mark.parametrize(
@@ -39,8 +55,8 @@ def test_echo(modules, echo_script, args, expected):
     """
     Test argument processing with a batch script
     """
-    script = "salt://echo-script/test.bat"
-    result = modules.cmd.script(script, args=args, shell="cmd")
+    script = f"salt://echo-script/{echo_script}"
+    result = modules.cmd.script(script, args=args)
     assert result["stdout"] == expected
 
 
@@ -55,13 +71,12 @@ def test_echo(modules, echo_script, args, expected):
 )
 def test_echo_runas(modules, account, echo_script, args, expected):
     """
-    Test argument processing with a batch script and runas
+    Test argument processing with a batch/bash script and runas
     """
-    script = "salt://echo-script/test.bat"
+    script = f"salt://echo-script/{echo_script}"
     result = modules.cmd.script(
         script,
         args=args,
-        shell="cmd",
         runas=account.username,
         password=account.password,
     )