avoid using subprocess in tests

Use main.main() instead, which makes the tests fully debuggable.

Author: thorsten-klein

README.rst

@@ -156,9 +156,12 @@ Then, run the test suite locally from the top level directory::
   # Recommended in an active virtual environment
   poe all
 
-  # Manually
+  # Manually (test the installed west version)
   pytest
 
+  # Manually (test the local copy)
+  pytest -o pythonpath=src
+
 The ``all`` target from ``poe`` runs multiple tasks sequentially. Run ``poe -h``
 to get the list of configured tasks.
 You can pass arguments to the task running ``poe``. This is especially useful

pyproject.toml

@@ -114,7 +114,4 @@ env = {PYTHONIOENCODING = "utf-8"}
 [tool.poe.tasks]
 lint = "ruff check ."
 types = "mypy --package west"
-all = ["test", "lint", "types"]
-
-[tool.pytest.ini_options]
-pythonpath = ["src"]
+all = ["test", "lint", "types"]

tests/conftest.py

@@ -2,6 +2,8 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
+import contextlib
+import io
 import os
 import platform
 import shutil
@@ -13,25 +15,9 @@
 
 import pytest
 
-import west.version
 from west import configuration as config
 from west.app import main
 
-# Expected PYTHONPATH pointing to the current source directory
-PYTHONPATH = os.fspath(Path(west.version.__file__).resolve().parents[1])
-
-# West's main Python script, runnable as a command-line executable
-WEST_MAIN = main.__file__
-
-# installed west executable (only set if it is correct version)
-WEST_EXE = shutil.which('west')
-if WEST_EXE:
-    # ensure that installed west exe has expected version
-    expected = west.version.__version__
-    actual = subprocess.check_output([WEST_EXE,'--version'], text=True)
-    if expected not in actual:
-        WEST_EXE = None
-
 GIT = shutil.which('git')
 
 # Git capabilities are discovered at runtime in
@@ -78,6 +64,47 @@
 
 WINDOWS = (platform.system() == 'Windows')
 
+#
+# Contextmanager
+#
+
+@contextlib.contextmanager
+def update_env(env: dict):
+    """
+    Temporarily update the process environment variables.
+    This context manager updates `os.environ` with the key-value pairs
+    provided in the `env` dictionary for the duration of the `with` block.
+    Any existing environment variables are preserved and restored when
+    the block exits.
+    """
+    env_bak = dict(os.environ)
+    env_vars = {}
+    for k,v in env.items():
+        env_vars[k] = str(v) if v else ""
+    os.environ.update(env_vars)
+    try:
+        yield
+    finally:
+        os.environ.clear()
+        os.environ.update(env_bak)
+
+
+@contextlib.contextmanager
+def chdir(path):
+    """
+    Temporarily change the current working directory.
+    This context manager changes the current working directory to `path`
+    for the duration of the `with` block. After the block exits, the
+    working directory is restored to its original value.
+    """
+    oldpwd = os.getcwd()
+    os.chdir(path)
+    try:
+        yield
+    finally:
+        os.chdir(oldpwd)
+
+
 #
 # Test fixtures
 #
@@ -334,52 +361,53 @@ def check_output(*args, **kwargs):
         raise
     return out_bytes.decode(sys.getdefaultencoding())
 
+
 def cmd(cmd, cwd=None, stderr=None, env=None):
     # Run a west command in a directory (cwd defaults to os.getcwd()).
     #
-    # This helper takes the command as a string.
+    # This helper takes the command as a string or list.
     #
-    # This helper relies on the test environment to ensure that the
-    # 'west' executable is a bootstrapper installed from the current
-    # west source code. If installed 'west' executable has correct version,
-    # it is preferred. Otherwise, fall back to running the local Python module.
-    #
-    # stdout from cmd is captured and returned. The command is run in
-    # a python subprocess so that program-level setup and teardown
-    # happen fresh.
-
-    # If you have quoting issues: do NOT quote. It's not portable.
-    # Instead, pass `cmd` as a list.
-
-    west_exe = WEST_EXE or WEST_MAIN
-
-    cmd = [west_exe] + (cmd.split() if isinstance(cmd, str) else cmd)
-    print('running:', cmd)
-
-    if env:
-        print('with non-default environment:')
-        for k in env:
-            if k not in os.environ or env[k] != os.environ[k]:
-                print(f'\t{k}={env[k]}')
-        for k in os.environ:
-            if k not in env:
-                print(f'\t{k}: deleted, was: {os.environ[k]}')
-    if cwd is not None:
-        cwd = os.fspath(cwd)
-        print(f'in {cwd}')
-    try:
-        return check_output(cmd, cwd=cwd, stderr=stderr, env=env)
-    except subprocess.CalledProcessError:
-        print('cmd: west:', west_exe, file=sys.stderr)
-        raise
+    # This function executes the `main()` function with the given command as
+    # arguments, capturing its stdout output while optionally modifying the working
+    # directory and environment.
+    cmd = (cmd.split() if isinstance(cmd, str) else cmd)
+    # every member of cmd must be string
+    cmd = [str(c) for c in cmd]
+
+    stdout_buf = io.StringIO()
+    with chdir(cwd or Path.cwd()):
+        with update_env(env or {}):
+            with contextlib.redirect_stdout(stdout_buf), \
+                 contextlib.redirect_stderr(stderr or sys.stderr):
+                try:
+                    main.main(cmd)
+                except SystemExit as e:
+                    if e.code:
+                        raise e
+                except Exception as e:
+                    print(f'Uncaught exception type {e}', file=sys.stderr)
+                    raise e
+    return stdout_buf.getvalue()
 
 
 def cmd_raises(cmd_str_or_list, expected_exception_type, cwd=None, env=None):
     # Similar to 'cmd' but an expected exception is caught.
-    # Returns the output together with stderr data
+    # Returns the stderr data.
+    stderr_buf = io.StringIO()
     with pytest.raises(expected_exception_type) as exc_info:
-        cmd(cmd_str_or_list, stderr=subprocess.STDOUT, cwd=cwd, env=env)
-    return exc_info.value.output.decode("utf-8")
+        cmd(cmd_str_or_list, stderr=stderr_buf, cwd=cwd, env=env)
+    return exc_info, stderr_buf.getvalue()
+
+
+def cmd_all_stdout(cmd, cwd=None, stderr=None, env=None):
+    # This function is similar to `cmd()`, but runs the command in a separate
+    # Python subprocess. This ensures that both Python-level stdout and any
+    # subprocess output invoked by `main` are captured together in a single
+    # string. It also ensures that program-level setup and teardown in `main`
+    # happen fresh for each call.
+    WEST_MAIN = main.__file__
+    cmd = (cmd.split() if isinstance(cmd, str) else cmd)
+    return check_output([sys.executable, WEST_MAIN] + cmd, cwd=cwd)
 
 
 def create_workspace(workspace_dir, and_git=True):

tests/test_alias.py

@@ -2,8 +2,6 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
-import subprocess
-
 import pytest
 from conftest import cmd
 
@@ -50,10 +48,9 @@ def test_alias_infinite_recursion():
     cmd('config alias.test2 test3')
     cmd('config alias.test3 test1')
 
-    with pytest.raises(subprocess.CalledProcessError) as excinfo:
-        cmd('test1', stderr=subprocess.STDOUT)
-
-    assert 'unknown command "test1";' in str(excinfo.value.stdout)
+    with pytest.raises(SystemExit) as excinfo:
+        cmd('test1')
+    assert 'unknown command "test1";' in str(excinfo.value.code)
 
 
 def test_alias_empty():
@@ -62,10 +59,9 @@ def test_alias_empty():
     # help command shouldn't fail
     cmd('help')
 
-    with pytest.raises(subprocess.CalledProcessError) as excinfo:
-        cmd('empty', stderr=subprocess.STDOUT)
-
-    assert 'empty alias "empty"' in str(excinfo.value.stdout)
+    with pytest.raises(SystemExit) as excinfo:
+        cmd('empty')
+    assert 'empty alias "empty"' in str(excinfo.value.code)
 
 
 def test_alias_early_args():

tests/test_config.py

@@ -5,7 +5,6 @@
 import configparser
 import os
 import pathlib
-import subprocess
 from typing import Any
 
 import pytest
@@ -251,12 +250,12 @@ def test_append():
         '-DCONF_FILE=foo.conf -DEXTRA_CFLAGS=\'-Wextra -g0\' -DFOO=BAR'
 
 def test_append_novalue():
-    err_msg = cmd_raises('config -a pytest.foo', subprocess.CalledProcessError)
+    _, err_msg = cmd_raises('config -a pytest.foo', SystemExit)
     assert '-a requires both name and value' in err_msg
 
 def test_append_notfound():
     update_testcfg('pytest', 'key', 'val', configfile=LOCAL)
-    err_msg = cmd_raises('config -a pytest.foo bar', subprocess.CalledProcessError)
+    _, err_msg = cmd_raises('config -a pytest.foo bar', SystemExit)
     assert 'option pytest.foo not found in the local configuration file' in err_msg
 
 
@@ -387,7 +386,7 @@ def test_delete_cmd_all():
     assert cfg(f=ALL)['pytest']['key'] == 'local'
     cmd('config -D pytest.key')
     assert 'pytest' not in cfg(f=ALL)
-    with pytest.raises(subprocess.CalledProcessError):
+    with pytest.raises(SystemExit):
         cmd('config -D pytest.key')
 
 def test_delete_cmd_none():
@@ -400,7 +399,7 @@ def test_delete_cmd_none():
     assert cmd('config pytest.key').rstrip() == 'global'
     cmd('config -d pytest.key')
     assert cmd('config pytest.key').rstrip() == 'system'
-    with pytest.raises(subprocess.CalledProcessError):
+    with pytest.raises(SystemExit):
         cmd('config -d pytest.key')
 
 def test_delete_cmd_system():
@@ -409,7 +408,7 @@ def test_delete_cmd_system():
     cmd('config --global pytest.key global')
     cmd('config --local pytest.key local')
     cmd('config -d --system pytest.key')
-    with pytest.raises(subprocess.CalledProcessError):
+    with pytest.raises(SystemExit):
         cmd('config --system pytest.key')
     assert cmd('config --global pytest.key').rstrip() == 'global'
     assert cmd('config --local pytest.key').rstrip() == 'local'
@@ -421,7 +420,7 @@ def test_delete_cmd_global():
     cmd('config --local pytest.key local')
     cmd('config -d --global pytest.key')
     assert cmd('config --system pytest.key').rstrip() == 'system'
-    with pytest.raises(subprocess.CalledProcessError):
+    with pytest.raises(SystemExit):
         cmd('config --global pytest.key')
     assert cmd('config --local pytest.key').rstrip() == 'local'
 
@@ -433,16 +432,16 @@ def test_delete_cmd_local():
     cmd('config -d --local pytest.key')
     assert cmd('config --system pytest.key').rstrip() == 'system'
     assert cmd('config --global pytest.key').rstrip() == 'global'
-    with pytest.raises(subprocess.CalledProcessError):
+    with pytest.raises(SystemExit):
         cmd('config --local pytest.key')
 
 def test_delete_cmd_error():
     # Verify illegal combinations of flags error out.
-    err_msg = cmd_raises('config -l -d pytest.key', subprocess.CalledProcessError)
+    _, err_msg = cmd_raises('config -l -d pytest.key', SystemExit)
     assert 'argument -d/--delete: not allowed with argument -l/--list' in err_msg
-    err_msg = cmd_raises('config -l -D pytest.key', subprocess.CalledProcessError)
+    _, err_msg = cmd_raises('config -l -D pytest.key', SystemExit)
     assert 'argument -D/--delete-all: not allowed with argument -l/--list' in err_msg
-    err_msg = cmd_raises('config -d -D pytest.key', subprocess.CalledProcessError)
+    _, err_msg = cmd_raises('config -d -D pytest.key', SystemExit)
     assert 'argument -D/--delete-all: not allowed with argument -d/--delete' in err_msg
 
 def test_default_config():
@@ -473,25 +472,25 @@ def test_config_precedence():
     assert cfg(f=LOCAL)['pytest']['precedence'] == 'local'
 
 def test_config_missing_key():
-    err_msg = cmd_raises('config pytest', subprocess.CalledProcessError)
+    _, err_msg = cmd_raises('config pytest', SystemExit)
     assert 'invalid configuration option "pytest"; expected "section.key" format' in err_msg
 
 
 def test_unset_config():
     # Getting unset configuration options should raise an error.
     # With verbose output, the exact missing option should be printed.
-    err_msg = cmd_raises('-v config pytest.missing', subprocess.CalledProcessError)
+    _, err_msg = cmd_raises('-v config pytest.missing', SystemExit)
     assert 'pytest.missing is unset' in err_msg
 
 def test_no_args():
-    err_msg = cmd_raises('config', subprocess.CalledProcessError)
+    _, err_msg = cmd_raises('config', SystemExit)
     assert 'missing argument name' in err_msg
 
 def test_list():
     def sorted_list(other_args=''):
         return list(sorted(cmd('config -l ' + other_args).splitlines()))
 
-    err_msg = cmd_raises('config -l pytest.foo', subprocess.CalledProcessError)
+    _, err_msg = cmd_raises('config -l pytest.foo', SystemExit)
     assert '-l cannot be combined with name argument' in err_msg
 
     assert cmd('config -l').strip() == ''

tests/test_help.py

@@ -1,8 +1,6 @@
 # Copyright (c) 2020, Nordic Semiconductor ASA
 
 import itertools
-import os
-import sys
 
 from conftest import cmd
 
@@ -32,14 +30,6 @@ def test_extension_help_and_dash_h(west_init_tmpdir):
     ext2out = cmd('test-extension -h')
 
     expected = EXTENSION_EXPECTED
-    if sys.platform == 'win32':
-        # Manage gratuitous incompatibilities:
-        #
-        # - multiline python strings are \n separated even on windows
-        # - the windows command help output gets an extra newline
-        expected = [os.linesep.join(case.splitlines()) + os.linesep
-                    for case in EXTENSION_EXPECTED]
-
     assert ext1out == ext2out
     assert ext1out in expected
 

tests/test_main.py

@@ -1,8 +1,9 @@
 import os
 import subprocess
 import sys
+from pathlib import Path
 
-from conftest import PYTHONPATH, WEST_EXE, cmd
+from conftest import cmd
 
 import west.version
 
@@ -23,9 +24,9 @@ def test_main():
     # Note: if a wrong west version is installed, local PYTHONPATH needs to be
     # prepended so that the called process uses correct west python modules
     env = os.environ.copy()
-    if not WEST_EXE:
-        val = os.getenv('PYTHONPATH')
-        env['PYTHONPATH'] = f'{PYTHONPATH}{os.pathsep}{val}' if val else PYTHONPATH
+    val = os.getenv('PYTHONPATH')
+    PYTHONPATH = os.fspath(Path(west.version.__file__).resolve().parents[1])
+    env['PYTHONPATH'] = f'{PYTHONPATH}{os.pathsep}{val}' if val else PYTHONPATH
 
     # execute west as a python module
     output_as_module = subprocess.check_output(