Unflushed `stderr` doesn't get captured via `CliRunner`

[ericfrederich]

CliRunner flushes stdout but not stderr.

To reproduce:
Invoke a command that uses print with file=sys.stderr which doesn't flush by default (click.secho flushes by default).

Expected results:
Anything written to stderr should be captured in the result of a CliRunner.invoke()

Environment:

• Python version: 3.11
• Click version: 8.1.7 as well as main branch here on GitHub

[ericfrederich]

Fyi... this was the code I used to discover the bug. On the pull request I ended up just modifying an existing test.

import click
import pytest
from click.testing import CliRunner


@click.command()
@click.option("--out/--no-out")
@click.option("--err/--no-err")
def dummy_cmd(out: bool, err: bool):
    import sys

    print("this is stdout", file=sys.stdout)
    print("this is stderr", file=sys.stderr)
    if out:
        print("Extra out!", file=sys.stdout)
    if err:
        print("Extra err!", file=sys.stderr)


@pytest.mark.parametrize("out_option", [None, True, False])
@pytest.mark.parametrize("err_option", [None, True, False])
def test_dummy_cli(out_option: str | None, err_option: str | None):
    runner = CliRunner(mix_stderr=False)
    args = []
    expected_out_lines = 1
    expected_err_lines = 1
    if out_option is not None:
        if out_option:
            args.append("--out")
            expected_out_lines += 1
        else:
            args.append("--no-out")
    if err_option is not None:
        if err_option:
            args.append("--err")
            expected_err_lines += 1
        else:
            args.append("--no-err")
    result = runner.invoke(dummy_cmd, [*args])

    if result.exception:
        raise result.exception

    if result.exit_code != 0:
        raise AssertionError(f"Non-zero return code: {result.exit_code!r}\noutput:\n{result.output}")

    assert len(result.stdout.splitlines()) == expected_out_lines
    assert len(result.stderr.splitlines()) == expected_err_lines

[davidism]

Duplicate of #2636. If you're using things that don't flush, like print, then they don't flush. Hiding that by doing it in the test runner just hides the potentially incorrect output behavior you're using. This is one reason why click.echo exists.

[ericfrederich]

This argument doesn't pass the smell test.
If that's really the case why is there a sys.stdout.flush() call here prior to capturing stdout? Interestingly 2 different test fail when I comment out the stdout flush.

All I'm asking is that the code follow the same style when capturing stderr as it does when capturing stdout.
That's all it takes to fix the bug. Just call sys.stderr.flush() prior to stderr = outstreams[1].getvalue() the same way sys.stdout.flush() is called prior to stdout = outstreams[0].getvalue(). Let's be consistent.

Another thought... it may not be your code which is writing to stderr or stdout, but some dependency which you have no control over. Alternatively it may be a dependency you explicitly choose to use which likes to do its own stdout/stderr writing (something like rich)

Click has a lot of things... cli, prompts, progress bars, styled printing, etc. Please don't make this an all-or-nothing library. It's nice to pick and choose. I shouldn't be forced to use click.echo just because I want to use the CLI and testing's CliRunner.

[davidism]

You're welcome to use print to stderr, you just have to remember to flush in that case. Stdout is flushed, stderr isn't. That's true regardless of if you're using click.

If we flush for you during tests, that's hiding the bug, not fixing it.

[davidism]

Another way to put it is: you need to explain why your suggested behavior is correct. It's certainly different than current, but that doesn't mean it's more correct.

What I know is that stdout is flushed, and stderr isn't. I don't know why that's the case, but I do know that this is the behavior I've encountered all over the place, and if I want stderr to show up at a specific time I need to flush manually. This is what click.echo does automatically. If what I know and click's test runner's current behavior is incorrect, then you need to show me sources that explain why. Otherwise, it would just mean we're hiding the actual behavior of the calls you're making.

[ericfrederich]

Thanks, I think understand what you're saying. Are you saying the CliRunner flushed stdout to mimic the behavior you've encountered all over the place?

If click.echo forces a flush every time though, then why does the CliRunner need to flush as well? Why do the tests fail when that flush in the CliRunner is removed?

[kdeldycke]

This has been fixed in v8.2.1 by #2933.