-
Notifications
You must be signed in to change notification settings - Fork 153
support running and debugging pytest for local tree
#859
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,74 @@ 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_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, 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(cmd: list | str, cwd=None, stderr: io.StringIO | None = None, env=None): | ||
| # Same as _cmd(), but it captures and returns combined stdout and stderr. | ||
| # Optionally stderr can be captured separately into given stderr. | ||
| # Note that this function does not capture any stdout or stderr from an | ||
| # internally invoked subprocess. | ||
| stdout_buf = io.StringIO() | ||
| stderr_buf = stderr or stdout_buf | ||
| with contextlib.redirect_stdout(stdout_buf), contextlib.redirect_stderr(stderr_buf): | ||
| _cmd(cmd, cwd, env) | ||
| return stdout_buf.getvalue() | ||
|
|
||
|
|
||
| def cmd_raises(cmd: list | str, 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, cwd=cwd, env=env) | ||
| return exc_info, stderr_buf.getvalue() | ||
|
|
||
|
|
||
| def cmd_subprocess(cmd: list | str, *args, **kwargs): | ||
| # This function behaves similarly to `cmd()`, but executes the command in a | ||
| # separate Python subprocess, capturing all stdout output. | ||
| # The captured stdout includes both Python-level output and the output of | ||
| # any subprocesses spawned internally. This makes the function particularly | ||
| # useful in test cases where the code under test launches subprocesses and | ||
| # the combined stdout needs to be verified. | ||
| # The main drawback is that it cannot be debugged within Python, so it | ||
| # should only be used sparingly in tests. | ||
| cmd = cmd if isinstance(cmd, list) else cmd.split() | ||
| cmd = [sys.executable, main.__file__] + cmd | ||
| print('running (subprocess):', cmd) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Naive question sorry: isn't there some logging function around here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so. There are print statements at other places as well in |
||
| ret = check_output(cmd, *args, **kwargs) | ||
| return ret | ||
|
|
||
|
|
||
| def create_workspace(workspace_dir, and_git=True): | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please correct me: this 3rd commit and test addition is only to test the previous, Please correct me.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is just a test for the second commit. Without the second commit, the I am not sure if I see this as a test code. With this change also users can invoke west by running the I will add this to the PR description. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment must explain what is the effect of this when people run
westfrom apip3pipx/uv/uvxinstall or from their distro.sys.pathis security-sensitive, all use cases must be carefully considered.You made this a separate commit. I love smaller commits and smaller PRs [*] but I wonder what happens when I try to use the previous, big refactoring without this commit?
[*] https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#defining-smaller-prs
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no effect when people run
west. As the comment says it has only an effect whenrunning this script. I will adapt the comment little bit to point out that it only affects ifrunning this script directlyThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is apparently not enough, see fixup: