Remove subprocess usage from tests

The previous test setup used subprocess calls, which did not allow to
debug the tests. It also caused a mix of Python modules from the
installed West package and the local source tree when running tests
directly with `pytest`.

The tests have been updated so that `pytest` can now be run directly and
fully debugged. `pytest` can be run as follows:
- `pytest` — runs tests against the installed package
- `pytest -o pythonpath=src` — runs tests against the local source tree

Within the tests following methods can be used to run west commands:
- cmd(...): call main() with given west command and capture std (and
optionally stderr)
- cmd_raises(...): call main() with given west command and catch
expected exception. The exception and stderr are returned by default.
Optionally stdout can be captured.
- cmd_subprocess(...): Run west command in a subprocess and capture
stdout.

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

@@ -76,8 +76,8 @@ omit = [
 [tool.coverage.report]
 omit = [
   "*/tmp/*",
-  "net-tools/scripts/test.py",
-  "subdir/Kconfiglib/scripts/test.py",
+  "*/net-tools/scripts/test.py",
+  "*/subdir/Kconfiglib/scripts/test.py",
 ]
 
 [tool.coverage.paths]

tests/conftest.py

@@ -3,6 +3,7 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import contextlib
+import io
 import os
 import platform
 import shutil
@@ -14,6 +15,8 @@
 
 import pytest
 
+from west.app import main
+
 GIT = shutil.which('git')
 
 # Git capabilities are discovered at runtime in
@@ -366,48 +369,69 @@ def check_output(*args, **kwargs):
     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 relies on the test environment to ensure that the
-    # 'west' executable is a bootstrapper installed from the current
-    # west source code.
-    #
-    # 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.
-    cmd = ['west'] + (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:', shutil.which('west'), file=sys.stderr)
-        raise
+def _cmd(cmd, cwd=None, env=None):
+    # Executes a west command by invoking the `main()` function with the
+    # provided command arguments.
+    # Parameters:
+    #   cwd: The working directory in which to execute the command.
+    #   env: A dictionary of extra environment variables to apply temporarily
+    #        during execution.
+
+    # ensure that cmd is a list of strings
+    cmd = cmd.split() if isinstance(cmd, str) else cmd
+    cmd = [str(c) for c in cmd]
+
+    # run main()
+    with (
+        chdir(cwd or Path.cwd()),
+        update_env(env or {}),
+    ):
+        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
 
 
-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
-    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")
+def cmd(cmd, cwd=None, stderr=None, env=None):
+    # Same as _cmd(), but it captures and returns stdout.
+    # Optionally stderr is captured into given stderr (io.StringIO)
+    stdout_buf = io.StringIO()
+    stderr_buf = stderr or sys.stderr
+    with contextlib.redirect_stdout(stdout_buf), contextlib.redirect_stderr(stderr_buf):
+        _cmd(cmd, cwd, env)
+    return stdout_buf.getvalue()
+
+
+def cmd_raises(cmd_str_or_list, expected_exception_type, stdout=None, cwd=None, env=None):
+    # Similar to '_cmd' but an expected exception is caught.
+    # The exception is returned together with stderr.
+    # Optionally stdout is captured into given stdout (io.StringIO)
+    stdout_buf = stdout or sys.stdout
+    stderr_buf = io.StringIO()
+    with (
+        contextlib.redirect_stdout(stdout_buf),
+        contextlib.redirect_stderr(stderr_buf),
+        pytest.raises(expected_exception_type) as exc_info,
+    ):
+        _cmd(cmd_str_or_list, cwd=cwd, env=env)
+    return exc_info, stderr_buf.getvalue()
+
+
+def cmd_subprocess(cmd, *args, **kwargs):
+    # 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 and also, that program-level setup and teardown in `main` happen
+    # fresh for each call.
+    cmd = cmd.split() if isinstance(cmd, str) else cmd
+    cmd = [sys.executable, main.__file__] + cmd
+    print('running (subprocess):', cmd)
+    ret = check_output(cmd, *args, **kwargs)
+    return ret
 
 
 def create_workspace(workspace_dir, and_git=True):

tests/test_alias.py

@@ -2,10 +2,8 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
-import subprocess
-
 import pytest
-from conftest import cmd
+from conftest import cmd, cmd_raises
 
 
 @pytest.fixture(autouse=True)
@@ -50,10 +48,8 @@ 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)
+    exc, _ = cmd_raises('test1', SystemExit)
+    assert 'unknown command "test1";' in str(exc.value)
 
 
 def test_alias_empty():
@@ -62,10 +58,8 @@ 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)
+    exc, _ = cmd_raises('empty', SystemExit)
+    assert 'empty alias "empty"' in str(exc.value)
 
 
 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
@@ -279,13 +278,13 @@ def test_append():
 
 
 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
 
 
@@ -425,7 +424,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')
 
 
@@ -439,7 +438,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')
 
 
@@ -449,7 +448,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'
@@ -462,7 +461,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'
 
@@ -475,17 +474,17 @@ 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
 
 
@@ -519,27 +518,27 @@ def test_config_precedence():
 
 
 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
 
@@ -33,13 +31,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,5 +1,4 @@
-import subprocess
-import sys
+from conftest import cmd, cmd_subprocess
 
 import west.version
 
@@ -11,7 +10,14 @@ def test_main():
     # sane (i.e. the actual version number is printed instead of
     # simply an error message to stderr).
 
-    output_as_module = subprocess.check_output([sys.executable, '-m', 'west', '--version']).decode()
-    output_directly = subprocess.check_output(['west', '--version']).decode()
-    assert west.version.__version__ in output_as_module
-    assert output_as_module == output_directly
+    expected_version = west.version.__version__
+
+    # call west executable directly
+    output_directly = cmd(['--version'])
+    assert expected_version in output_directly
+
+    output_subprocess = cmd_subprocess('--version')
+    assert expected_version in output_subprocess
+
+    # output must be same in both cases
+    assert output_subprocess.rstrip() == output_directly.rstrip()