8000 `CliRunner`: restrict `mix_stderr` influence to `<output>`; keep `<stderr>` and `<stdout>` stable · Issue #2522 · pallets/click · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

CliRunner: restrict mix_stderr influence to <output>; keep <stderr> and <stdout> stable #2522

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

Closed
kdeldycke opened this issue May 30, 2023 · 1 comment · Fixed by #2523
Closed
Milestone

Comments

@kdeldycke
Copy link
Collaborator
kdeldycke commented May 30, 2023

Let's start with the current behaviour of CliRunner regarding its mix_stderr option.

Here is a simple CLI:

from click import command, echo
from click.testing import CliRunner


@command()
def hello():
    echo("1 - Hello world")
    echo("2 - Hello error", err=True)
    echo("3 - Good bye world")
    echo("4 - Good bye error", err=True)


if __name__ == "__main__":

    print("\n*** mix_stderr=True (default) ***\n")
    runner = CliRunner(mix_stderr=True)
    result = runner.invoke(hello)
    print(f"--- <stdout> ---\n{result.stdout}")
    print(f"--- <stderr> ---\n(raises error)\n")
    print(f"--- <output> ---\n{result.output}")

    print("\n*** mix_stderr=False ***\n")
    runner = CliRunner(mix_stderr=False)
    result = runner.invoke(hello)
    print(f"--- <stdout> ---\n{result.stdout}")
    print(f"--- <stderr> ---\n{result.stderr}")
    print(f"--- <output> ---\n{result.output}")

When called, the CLI above produces:

$ python ./cli_runner_stderr_test.py

*** mix_stderr=True (default) ***

--- <stdout> ---
1 - Hello world
2 - Hello error
3 - Good bye world
4 - Good bye error

--- <stderr> ---
(raises error)

--- <output> ---
1 - Hello world
2 - Hello error
3 - Good bye world
4 - Good bye error


*** mix_stderr=False ***

--- <stdout> ---
1 - Hello world
3 - Good bye world

--- <stderr> ---
2 - Hello error
4 - Good bye error

--- <output> ---
1 - Hello world
3 - Good bye world

What I'd like to propose is to instead have CliRunner produce this result:

$ python ./cli_runner_stderr_test.py

*** mix_stderr=True (default) ***

--- <stdout> ---
1 - Hello world
3 - Good bye world

--- <stderr> ---
2 - Hello error
4 - Good bye error

--- <output> ---
1 - Hello world
2 - Hello error
3 - Good bye world
4 - Good bye error


*** mix_stderr=False ***

--- <stdout> ---
1 - Hello world
3 - Good bye world

--- <stderr> ---
2 - Hello error
4 - Good bye error

--- <output> ---
1 - Hello world
3 - Good bye world

So what I propose is to streamline the semantics of result.stdout, result.stderr and result.output:

  • Let result.stdout always contain the pure output to <stdout>. Never mangle <stderr> in it.
  • Let result.stderr always contain the pure output to <stderr>. Never raise an error.
  • Use result.output as the content the user is expected to see:
    • let it be a proxy of <stdout> if mix_stderr=False
    • have it produce a mix of <stdout> and <stderr> if mix_stderr=True

The advantage of this is we could properly check the individual <stdout> and <stderr> streams, and check for the user output by inspecting <output>.

The raised exception in <stderr> has been introduced in 7.0 for backward compatibility, because the code prior to 7.0 wasn't returning the stderr/stderr split output (see #868). It's from 2017 so we can safely change that behaviour by now.

Note that I explicitely numbered each echo() statement to highlight the print order. If a refactor is attempted, I am anticipating some edge-cases around the preservation of order, so it's good to have simple code to test that.

@kdeldycke kdeldycke changed the title CliRunner: restrict mix_stderr influence to output only; keep <stderr> and <stdout> constant CliRunner: restrict mix_stderr influence to <output> only; keep <stderr> and <stdout> constant May 30, 2023
@kdeldycke kdeldycke changed the title CliRunner: restrict mix_stderr influence to <output> only; keep <stderr> and <stdout> constant CliRunner: restrict mix_stderr influence to <output>; keep <stderr> and <stdout> stable May 30, 2023
@kdeldycke
Copy link
Collaborator Author

I just finished a PR at: #2523

It is ready to be reviewed and/or merged upstream.

@davidism davidism added this to the 8.2.0 milestone Jul 4, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants
0