Skip to content

Commit

Permalink
Keep unittests DRY, override default_shell in test decorator
Browse files Browse the repository at this point in the history
- Tests that use the the `per_available_shell` decorator were reporting
  false positives for most cases b/c it appears to have been assumed that
  execute_shell would execute the intended shell when it was actually
  picking up the system shell. If shell had been passed to `execute_shell`
  in most places then this change would not have been needed. After
  observing that most cases appear to depend on overriding the config I
  chose to move the override into the decorator to improve DRY-ness.

Signed-off-by: javrin <jawabiscuit@users.noreply.github.com>
  • Loading branch information
Jawabiscuit committed Sep 15, 2023
1 parent 1c54516 commit cfd8846
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 19 deletions.
5 changes: 0 additions & 5 deletions src/rez/tests/test_e2e_shells.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
"""
from __future__ import print_function

from rez.config import config
from rez.shells import create_shell
from rez.resolved_context import ResolvedContext
from rez.tests.util import TestBase, TempdirMixin, per_available_shell
Expand Down Expand Up @@ -39,10 +38,6 @@ def _create_context(cls, pkgs):

@per_available_shell()
def test_shell_execution(self, shell):
config.override("default_shell", shell)
if shell == "gitbash":
config.override("enable_path_normalization", True)

sh = create_shell(shell)
_, _, _, command = sh.startup_capabilities(command=True)
if command:
Expand Down
12 changes: 0 additions & 12 deletions src/rez/tests/test_shells.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,6 @@ def test_aaa_shell_presence(self):

@per_available_shell()
def test_no_output(self, shell):
config.override("default_shell", shell)
if shell == "gitbash":
config.override("enable_path_normalization", True)

sh = create_shell(shell)
_, _, _, command = sh.startup_capabilities(command=True)
if command:
Expand Down Expand Up @@ -310,8 +306,6 @@ def test_rex_code(self, shell):
"""Test that Rex code run in the shell creates the environment variable
values that we expect.
"""
config.override("default_shell", shell)

def _execute_code(func, expected_output):
loc = inspect.getsourcelines(func)[0][1:]
code = textwrap.dedent('\n'.join(loc))
Expand Down Expand Up @@ -481,8 +475,6 @@ def test_rex_code_alias(self, shell):
the absolute path to doskey.exe before we modify PATH and continue to
use the absolute path after the modifications.
"""
config.override("default_shell", shell)

def _execute_code(func):
loc = inspect.getsourcelines(func)[0][1:]
code = textwrap.dedent('\n'.join(loc))
Expand All @@ -509,8 +501,6 @@ def test_alias_command(self, shell):
This is important for Windows CMD shell because the doskey.exe isn't
executed yet when the alias is being passed.
"""
config.override("default_shell", shell)

def _make_alias(ex):
ex.alias('hi', 'echo "hi"')

Expand All @@ -529,8 +519,6 @@ def test_alias_command_with_args(self, shell):
This is important for Windows CMD shell because the doskey.exe isn't
executed yet when the alias is being passed.
"""
config.override("default_shell", shell)

def _make_alias(ex):
ex.alias('tell', 'echo')

Expand Down
2 changes: 0 additions & 2 deletions src/rez/tests/test_suites.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,6 @@ def test_executable(self, shell):
```
"""
config.override("default_shell", shell)

c_pooh = ResolvedContext(["pooh"])
s = Suite()
s.add_context("pooh", c_pooh)
Expand Down
15 changes: 15 additions & 0 deletions src/rez/tests/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,21 @@ def decorator(func):
def wrapper(self, shell=None):
for shell in shells:
print("\ntesting in shell: %s..." % shell)
# The default shell if none is configured is the system shell
# (bash or cmd ususally), so in order to test the other shells
# we need to override the default shell to the one we are
# testing to get a accurate results with more complete coverage.
#
# E.g. create_shell() will use the shell provided by this decorator
# but resoved_context.execute_shell() will use the default shell to
# execute a command if no shell is passed in.
config.override("default_shell", shell)

# TODO: If & when path normalization is set to True by default,
# this should be removed. For now, we need to enable it for
# gitbash, because it requires path normalization.
if shell == "gitbash":
config.override("enable_path_normalization", True)

try:
func(self, shell=shell)
Expand Down

0 comments on commit cfd8846

Please sign in to comment.