Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better shell specific testing #1136

Merged
merged 5 commits into from
Oct 11, 2021
Merged

Better shell specific testing #1136

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f9d077f
fix rez-env output test
davidlatwe Sep 29, 2021
110bc08
able to exclude shell in test
davidlatwe Sep 29, 2021
fc3a228
exclude shells that are not fit
davidlatwe Oct 5, 2021
17aef1f
also exclude pwsh
davidlatwe Oct 5, 2021
2a1dca5
bump ubuntu version for ci action
davidlatwe Oct 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/rez/tests/test_shells.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from rez.tests.util import TestBase, TempdirMixin, per_available_shell, \
install_dependent
from rez.bind import hello_world
from rez.config import config
import unittest
import subprocess
import tempfile
Expand Down Expand Up @@ -183,16 +184,20 @@ def test_rcfile(self):
@per_available_shell()
@install_dependent()
def test_rez_env_output(self):
target_shell = config.default_shell # overridden by test util

def _test(txt):
# Assumes that the shell has an echo command, build-in or alias
binpath = os.path.join(system.rez_bin_path, "rez-env")
args = [binpath, "--", "echo", txt]
args = [binpath, "--shell", target_shell, "--", "echo", txt]

process = subprocess.Popen(
args, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
stderr=subprocess.PIPE, universal_newlines=True
)
sh_out = process.communicate()
if sh_out[1]:
raise Exception("Command failed:\n%s" % sh_out[1])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't use process.returncode is because, it seems that powershell always return 0 even the command actually failed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this a case of powershell -File vs. -Command?
Only the latter passes the error code.
https://stackoverflow.com/questions/50200325/returning-an-exit-code-from-a-powershell-script

Or maybe $ErrorActionPreference = "Stop":
https://stackoverflow.com/questions/9948517/how-to-stop-a-powershell-script-on-the-first-error

The checks also need to happen form $LastExitCode I believe:
https://stackoverflow.com/questions/10666035/difference-between-and-lastexitcode-in-powershell

What an over-engineered little shell :)

self.assertEqual(sh_out[0].strip(), txt)

# please note - it's no coincidence that there are no substrings like
Expand Down
8 changes: 7 additions & 1 deletion src/rez/tests/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,10 @@ def wrapper(self, *args, **kwargs):
return decorator


def per_available_shell():
def per_available_shell(exclude=None):
"""Function decorator that runs the function over all available shell types."""
exclude = exclude or []

def decorator(func):
@functools.wraps(func)
def wrapper(self, *args, **kwargs):
Expand All @@ -216,6 +218,10 @@ def wrapper(self, *args, **kwargs):
]

for shell in shells:
if not only_shell and shell in exclude:
print("\nshell excluded from this test: %s..." % shell)
continue

print("\ntesting in shell: %s..." % shell)
config.override("default_shell", shell)

Expand Down