Revert back to having a default shell #68448
Conversation
| from salt.utils.win_functions import shlex_split | ||
| from salt.utils.win_runas import runas as win_runas | ||
|
|
||
| HAS_WIN_RUNAS = True | ||
| else: | ||
| import shlex | ||
|
|
||
| _cmd_quote = shlex.quote |
There was a problem hiding this comment.
Maybe do the same thing for split as was done for _cmd_quote to avoid having to check salt.utils.platform.is_windows on every usage of shlex.split.
| @pytest.mark.skip_on_freebsd | ||
| def test_shell_enabled(self): | ||
| def test_shell_disabled(self): | ||
| """ | ||
| test shell enabled output for cmd.run | ||
| test shell disabled output for cmd.run | ||
| """ | ||
| enabled_ret = "3\nsaltines" # the result of running self.cmd in a shell | ||
| ret = self.run_function("cmd.run", [self.cmd], python_shell=True) | ||
| self.assertEqual(ret.strip(), enabled_ret) |
There was a problem hiding this comment.
Seems like it would be good to also have test_shell_enabled test in addition to test_shell_disabled instead of replacing it with test_shell_disabled
There was a problem hiding this comment.
The original test was for disabled... I can add a test for enabled as well
| @pytest.mark.skip_on_windows(reason="Skip on Windows OS") | ||
| @pytest.mark.skip_on_freebsd | ||
| def test_shell_enabled(self): | ||
| def test_shell_disabled(self): |
There was a problem hiding this comment.
Would good be good to have a test for windows to ensure the expected behavior occurs when python_shell=False is passed on windows.
|
Is this a fix to #68298 ? If yes, thanks, as this regression has broken some code of mine. |
| elif __opts__.get("cmd_safe", True) is False and python_shell is None: | ||
| # Override-switch for python_shell | ||
| return True |
There was a problem hiding this comment.
Are there any tests for cmd_safe? I did a quick search and did not notice any.
There was a problem hiding this comment.
There were none before it was removed in #68156
twangboy
force-pushed
the
default_shell
branch
from
140dcca to
7c54899
Compare
3 participants
What does this PR do?
Reverts part of #68156 that changed the default behavior for
python_shellandshell. Also fixes a bug in the PR mentioned above, where you couldn't pass non-list parameters to a script.What issues does this PR fix or reference?
Fixes #68156
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes