Repository: AcademySoftwareFoundation/rez (language: python)

Issue:
improve shell parameterization in tests
Iterating over shell in tests currently is not done very well. It is difficult to determine which shells ran and where an error occurs.

Pytest has good support for parameterization, and there's a project that will enable it both for pytest _and_ unittest, which suits rez perfectly (since porting all its tests to pytest will be a decent chunk of work): https://pypi.org/project/parameterized/

**Motivation**
We would like tests that suck less. This will also help with efforts on #1256

**Related Issues/PRs**
* #1256



Discussion / hints:

Fix patch:
diff --git a/.github/docker/rez-win-py/entrypoint.ps1 b/.github/docker/rez-win-py/entrypoint.ps1
index 982141c4e..10734ac1d 100644
--- a/.github/docker/rez-win-py/entrypoint.ps1
+++ b/.github/docker/rez-win-py/entrypoint.ps1
@@ -39,10 +39,11 @@ if (-not $?) {exit 1}
 
 # Install pytest for better rez-selftest output
 .\build\Scripts\rez\rez-python -m pip install pytest-cov
+.\build\Scripts\rez\rez-python -m pip install parameterized
 
 # Run Rez Tests
 #
-.\build\Scripts\rez\rez-selftest.exe
+.\build\Scripts\rez\rez-selftest.exe -v
 
 # Pass on exit code to runner
 exit $LASTEXITCODE
diff --git a/.github/workflows/core.yaml b/.github/workflows/core.yaml
index 0dbf3007c..1fa7bf4df 100644
--- a/.github/workflows/core.yaml
+++ b/.github/workflows/core.yaml
@@ -54,4 +54,16 @@ jobs:
       # test class. This will be easier once ported to pytest.
       - name: Run Rez Tests
         run: |
-          ./build/bin/rez/rez-selftest --config --copy_package --formatter --imports --packages --resources_ --rex --schema --solver --version
+          ./build/bin/rez/rez-selftest -v \
+            --config \
+            --copy_package \
+            --formatter \
+            --imports \
+            --packages \
+            --package_filter \
+            --packages_order \
+            --resources_ \
+            --rex \
+            --schema \
+            --solver \
+            --version
diff --git a/.github/workflows/mac.yaml b/.github/workflows/mac.yaml
index b1e0e2fcb..d77fde0ae 100644
--- a/.github/workflows/mac.yaml
+++ b/.github/workflows/mac.yaml
@@ -52,7 +52,9 @@ jobs:
       - name: Install Rez test dependencies
         run: |
           ./build/bin/rez/rez-python -m pip install pytest-cov
+          ./build/bin/rez/rez-python -m pip install parameterized
 
       - name: Run Rez Tests
         run: |
-          ./build/bin/rez/rez-selftest
+          ./build/bin/rez/rez-selftest -v
+
diff --git a/.github/workflows/ubuntu.yaml b/.github/workflows/ubuntu.yaml
index 71216da3c..e549252ff 100644
--- a/.github/workflows/ubuntu.yaml
+++ b/.github/workflows/ubuntu.yaml
@@ -69,7 +69,9 @@ jobs:
       - name: Install Rez test dependencies
         run: |
           ./build/bin/rez/rez-python -m pip install pytest-cov
+          ./build/bin/rez/rez-python -m pip install parameterized
 
       - name: Run Rez Tests
         run: |
-          ./build/bin/rez/rez-selftest
+          ./build/bin/rez/rez-selftest -v
+


Test patch:
diff --git a/src/rez/cli/selftest.py b/src/rez/cli/selftest.py
index 82583f7a9..ef9e8a5de 100644
--- a/src/rez/cli/selftest.py
+++ b/src/rez/cli/selftest.py
@@ -14,10 +14,10 @@
 
 try:
     import pytest  # noqa
+    use_pytest = True
 except ImportError:
     use_pytest = False
-else:
-    use_pytest = True
+
 
 cli_dir = os.path.dirname(inspect.getfile(inspect.currentframe()))
 src_rez_dir = os.path.dirname(cli_dir)
diff --git a/src/rez/tests/test_build.py b/src/rez/tests/test_build.py
index 4f74de8cd..fb9da2a93 100644
--- a/src/rez/tests/test_build.py
+++ b/src/rez/tests/test_build.py
@@ -5,6 +5,7 @@
 """
 test the build system
 """
+from rez.config import config
 from rez.build_process import create_build_process
 from rez.build_system import create_build_system
 from rez.resolved_context import ResolvedContext
@@ -136,16 +137,22 @@ def _test_build_sup_world(self):
 
     @per_available_shell()
     @install_dependent()
-    def test_build_whack(self):
-        """Test that a broken build fails correctly."""
+    def test_build_whack(self, shell):
+        """Test that a broken build fails correctly.
+        """
+        config.override("default_shell", shell)
+
         working_dir = os.path.join(self.src_root, "whack")
         builder = self._create_builder(working_dir)
         self.assertRaises(BuildError, builder.build, clean=True)
 
     @per_available_shell()
     @install_dependent()
-    def test_builds(self):
-        """Test an interdependent set of builds."""
+    def test_builds(self, shell):
+        """Test an interdependent set of builds.
+        """
+        config.override("default_shell", shell)
+
         self._test_build_build_util()
         self._test_build_floob()
         self._test_build_foo()
@@ -154,8 +161,11 @@ def test_builds(self):
 
     @per_available_shell()
     @install_dependent()
-    def test_builds_anti(self):
-        """Test we can build packages that contain anti packages"""
+    def test_builds_anti(self, shell):
+        """Test we can build packages that contain anti packages
+        """
+        config.override("default_shell", shell)
+
         self._test_build_build_util()
         self._test_build_floob()
         self._test_build_anti()
diff --git a/src/rez/tests/test_release.py b/src/rez/tests/test_release.py
index 8d2a2adf4..953ab44c8 100644
--- a/src/rez/tests/test_release.py
+++ b/src/rez/tests/test_release.py
@@ -5,6 +5,7 @@
 """
 test the release system
 """
+from rez.config import config
 from rez.build_process import create_build_process
 from rez.build_system import create_build_system
 from rez.resolved_context import ResolvedContext
@@ -104,8 +105,10 @@ def _standardize_variants(variants):
 
     @per_available_shell()
     @install_dependent()
-    def test_1(self):
+    def test_1(self, shell):
         """Basic release."""
+        config.override("default_shell", shell)
+
         # release should fail because release path does not exist
         self._setup_release()
         builder = self._create_builder()
@@ -165,9 +168,11 @@ def test_1(self):
 
     @per_available_shell()
     @install_dependent()
-    def test_2_variant_add(self):
+    def test_2_variant_add(self, shell):
         """Test variant installation on release
         """
+        config.override("default_shell", shell)
+
         orig_src_path = self.src_path
         self.src_path = os.path.join(self.src_path, "variants")
         try:
diff --git a/src/rez/tests/test_shells.py b/src/rez/tests/test_shells.py
index fb67337f5..718db3307 100644
--- a/src/rez/tests/test_shells.py
+++ b/src/rez/tests/test_shells.py
@@ -53,8 +53,8 @@ def _create_context(cls, pkgs):
         return ResolvedContext(pkgs, caching=False)
 
     @per_available_shell()
-    def test_no_output(self):
-        sh = create_shell()
+    def test_no_output(self, shell):
+        sh = create_shell(shell)
         _, _, _, command = sh.startup_capabilities(command=True)
         if command:
             r = self._create_context(["hello_world"])
@@ -117,8 +117,8 @@ def test_create_executable_script(self):
             self.assertListEqual(files, [py_script_file])
 
     @per_available_shell()
-    def test_command(self):
-        sh = create_shell()
+    def test_command(self, shell):
+        sh = create_shell(shell)
         _, _, _, command = sh.startup_capabilities(command=True)
 
         if command:
@@ -128,8 +128,8 @@ def test_command(self):
             self.assertEqual(_stdout(p), "Hello Rez World!")
 
     @per_available_shell()
-    def test_command_returncode(self):
-        sh = create_shell()
+    def test_command_returncode(self, shell):
+        sh = create_shell(shell)
         _, _, _, command = sh.startup_capabilities(command=True)
 
         if command:
@@ -142,8 +142,8 @@ def test_command_returncode(self):
                 self.assertEqual(p.returncode, 66)
 
     @per_available_shell()
-    def test_norc(self):
-        sh = create_shell()
+    def test_norc(self, shell):
+        sh = create_shell(shell)
         _, norc, _, command = sh.startup_capabilities(norc=True, command=True)
 
         if norc and command:
@@ -154,8 +154,8 @@ def test_norc(self):
             self.assertEqual(_stdout(p), "Hello Rez World!")
 
     @per_available_shell()
-    def test_stdin(self):
-        sh = create_shell()
+    def test_stdin(self, shell):
+        sh = create_shell(shell)
         _, _, stdin, _ = sh.startup_capabilities(stdin=True)
 
         if stdin:
@@ -168,8 +168,8 @@ def test_stdin(self):
             self.assertEqual(stdout, "Hello Rez World!")
 
     @per_available_shell()
-    def test_rcfile(self):
-        sh = create_shell()
+    def test_rcfile(self, shell):
+        sh = create_shell(shell)
         rcfile, _, _, command = sh.startup_capabilities(rcfile=True, command=True)
 
         if rcfile and command:
@@ -191,13 +191,12 @@ def test_rcfile(self):
     #
     @per_available_shell(exclude=["cmd"])
     @install_dependent()
-    def test_rez_env_output(self):
-        target_shell = config.default_shell  # overridden by test util
+    def test_rez_env_output(self, shell):
 
         def _test(txt):
             # Assumes that the shell has an echo command, built-in or alias
             binpath = os.path.join(system.rez_bin_path, "rez-env")
-            args = [binpath, "--shell", target_shell, "--", "echo", txt]
+            args = [binpath, "--shell", shell, "--", "echo", txt]
 
             process = subprocess.Popen(
                 args, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
@@ -230,8 +229,8 @@ def _test(txt):
 
     @per_available_shell()
     @install_dependent()
-    def test_rez_command(self):
-        sh = create_shell()
+    def test_rez_command(self, shell):
+        sh = create_shell(shell)
         _, _, _, command = sh.startup_capabilities(command=True)
 
         if command:
@@ -245,9 +244,12 @@ def test_rez_command(self):
             self.assertEqual(p.returncode, 0)
 
     @per_available_shell()
-    def test_rex_code(self):
+    def test_rex_code(self, shell):
         """Test that Rex code run in the shell creates the environment variable
-        values that we expect."""
+        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))
@@ -387,15 +389,16 @@ def _rex_appending():
         _execute_code(_rex_appending, expected_output)
 
     @per_available_shell()
-    def test_rex_code_alias(self):
+    def test_rex_code_alias(self, shell):
         """Ensure PATH changes do not influence the alias command.
 
         This is important for Windows because the doskey.exe might not be on
         the PATH anymore at the time it's executed. That's why we figure out
         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))
@@ -416,13 +419,14 @@ def _alias_after_path_manipulation():
         _execute_code(_alias_after_path_manipulation)
 
     @per_available_shell()
-    def test_alias_command(self):
+    def test_alias_command(self, shell):
         """Testing alias can be passed in as command
 
         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"')
 
@@ -435,12 +439,14 @@ def _make_alias(ex):
         self.assertEqual(0, p.returncode)
 
     @per_available_shell()
-    def test_alias_command_with_args(self):
+    def test_alias_command_with_args(self, shell):
         """Testing alias can be passed in as command with args
 
         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')
 
diff --git a/src/rez/tests/test_suites.py b/src/rez/tests/test_suites.py
index 743a75a0d..e2ada77a5 100644
--- a/src/rez/tests/test_suites.py
+++ b/src/rez/tests/test_suites.py
@@ -143,7 +143,7 @@ def test_3(self):
 
     @per_available_shell()
     @install_dependent()
-    def test_executable(self):
+    def test_executable(self, shell):
         """Test suite tool can be executed
 
         Testing suite tool can be found and executed in multiple platforms.
@@ -158,6 +158,8 @@ def test_executable(self):
         ```
 
         """
+        config.override("default_shell", shell)
+
         c_pooh = ResolvedContext(["pooh"])
         s = Suite()
         s.add_context("pooh", c_pooh)
diff --git a/src/rez/tests/util.py b/src/rez/tests/util.py
index b092e8027..e6e614a50 100644
--- a/src/rez/tests/util.py
+++ b/src/rez/tests/util.py
@@ -20,6 +20,13 @@
 import json
 from contextlib import contextmanager
 
+# https://pypi.org/project/parameterized
+try:
+    from parameterized import parameterized
+    use_parameterized = True
+except ImportError:
+    use_parameterized = False
+
 
 class TestBase(unittest.TestCase):
     """Unit test base class."""
@@ -203,34 +210,35 @@ def wrapper(self, *args, **kwargs):
 
 
 def per_available_shell(exclude=None):
-    """Function decorator that runs the function over all available shell types."""
+    """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):
-            shells = get_shell_types()
+    shells = get_shell_types()
 
-            only_shell = os.getenv("__REZ_SELFTEST_SHELL")
-            if only_shell:
-                shells = [only_shell]
+    only_shell = os.getenv("__REZ_SELFTEST_SHELL")
+    if only_shell:
+        shells = [only_shell]
 
-            # filter to only those shells available
-            shells = [
-                x for x in shells
-                if get_shell_class(x).is_available()
-            ]
+    # filter to only those shells available
+    shells = [
+        x for x in shells
+        if get_shell_class(x).is_available()
+        and x not in (exclude or [])
+    ]
 
-            for shell in shells:
-                if not only_shell and shell in exclude:
-                    print("\nshell excluded from this test: %s..." % shell)
-                    continue
+    # https://pypi.org/project/parameterized
+    if use_parameterized:
+        return parameterized.expand(shells)
 
+    def decorator(func):
+        @functools.wraps(func)
+        def wrapper(self, shell=None):
+            for shell in shells:
                 print("\ntesting in shell: %s..." % shell)
-                config.override("default_shell", shell)
 
                 try:
-                    func(self, *args, **kwargs)
+                    func(self, shell=shell)
                 except Exception as e:
                     # Add the shell to the exception message, if possible.
                     # In some IDEs the args do not exist at all.