[Resolve #1520] Fix test for cmd hook shell option#1552
[Resolve #1520] Fix test for cmd hook shell option#1552iainelder wants to merge 1 commit intoSceptre:masterfrom
Conversation
The previous version of this test meant to use bash, but really used sh. Improves first PR Sceptre#1523.
|
@dboitnot , does this still work on Nix? |
| def test_shell_parameter_sets_the_shell(stack, capfd): | ||
| # Determine the local path to bash (it's not always /bin/bash) | ||
| Cmd("echo $0", stack).run() | ||
| Cmd("which bash", stack).run() |
There was a problem hiding this comment.
i think this is too specific and would make this unit test fail on developer systems without bash. Can you think of a more generic way to validate the correctly loaded shell?
There was a problem hiding this comment.
How about using Python as the test shell?
Use sys.executable to get the absolute path to the interpreter.
Then pass "print(sys.executable)" as the command and check that it's still Python in the stdout.
Python supports the -c option, which makes it a shell in this context.
There was a problem hiding this comment.
good idea but how about just using python's subprocess.run directly?
➜ python
Python 3.12.11 (main, Nov 3 2025, 12:02:03) [Clang 15.0.0 (clang-1500.3.9.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
>>> subprocess.run(["which","bash"], capture_output=True, text=True)
CompletedProcess(args=['which', 'bash'], returncode=0, stdout='/bin/bash\n', stderr='')
There was a problem hiding this comment.
Outside the box idea here, since this is really just testing that the parameter is being respected, try testing with a shell parameter for something that will never exist, and capture the exception to assert. something like
with pytest.raises(OSError):
Cmd({"run": "echo $0", "shell": "nonexistantshellprovidedbyuser"}, stack).run()As an aside, if the test environment were to be dockerized, then one could guarantee that bash exists in the env and this would no be an issue
4 participants
The previous version of this test meant to use bash, but really used sh.
Improves first PR #1523.
PR Checklist
[Resolve #issue-number].poetry run tox) are passing.poetry run pre-commit run --all-files).and description in grammatically correct, complete sentences.
Approver/Reviewer Checklist
Other Information
Guide to writing a good commit