Skip to content
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

`CliRunner.invoke` takes `mix_stderr` which does nothing #1435

Closed
adamtheturtle opened this issue Dec 3, 2019 · 10 comments
Closed

`CliRunner.invoke` takes `mix_stderr` which does nothing #1435

adamtheturtle opened this issue Dec 3, 2019 · 10 comments
Milestone

Comments

@adamtheturtle
Copy link
Contributor

@adamtheturtle adamtheturtle commented Dec 3, 2019

mix_stderr is a parameter of CliRunner.invoke.
This is not documented in the docstring as a :param.
Simply reading the code (which I did after trying this and seeing no output) shows that this parameter is not used.

mix_stderr works on CliRunner.__init__.

The mix_stderr parameter should be removed, or changed to work and get documented.

@davidism

This comment has been minimized.

Copy link
Member

@davidism davidism commented Dec 4, 2019

Fairly sure this is fixed in master

@adamtheturtle

This comment has been minimized.

Copy link
Contributor Author

@adamtheturtle adamtheturtle commented Dec 4, 2019

@davidism I believe that it is not.

See on master the parameter list includes mix_stderr:

click/click/testing.py

Lines 280 to 281 in 6944450

def invoke(self, cli, args=None, input=None, env=None,
catch_exceptions=True, color=False, mix_stderr=False, **extra):

The parameter documentation does not include mix_stderr:

click/click/testing.py

Lines 296 to 311 in 6944450

.. versionadded:: 4.0
The ``color`` parameter was added.
:param cli: the command to invoke
:param args: the arguments to invoke. It may be given as an iterable
or a string. When given as string it will be interpreted
as a Unix shell command. More details at
:func:`shlex.split`.
:param input: the input data for `sys.stdin`.
:param env: the environment overrides.
:param catch_exceptions: Whether to catch any other exceptions than
``SystemExit``.
:param extra: the keyword arguments to pass to :meth:`main`.
:param color: whether the output should contain color codes. The
application can still override this explicitly.
"""

And the function code does not include mix_stderr:

click/click/testing.py

Lines 312 to 357 in 6944450

exc_info = None
with self.isolation(input=input, env=env, color=color) as outstreams:
exception = None
exit_code = 0
if isinstance(args, string_types):
args = shlex.split(args)
try:
prog_name = extra.pop("prog_name")
except KeyError:
prog_name = self.get_default_prog_name(cli)
try:
cli.main(args=args or (), prog_name=prog_name, **extra)
except SystemExit as e:
exc_info = sys.exc_info()
exit_code = e.code
if exit_code is None:
exit_code = 0
if exit_code != 0:
exception = e
if not isinstance(exit_code, int):
sys.stdout.write(str(exit_code))
sys.stdout.write('\n')
exit_code = 1
except Exception as e:
if not catch_exceptions:
raise
exception = e
exit_code = 1
exc_info = sys.exc_info()
finally:
sys.stdout.flush()
stdout = outstreams[0].getvalue()
stderr = outstreams[1] and outstreams[1].getvalue()
return Result(runner=self,
stdout_bytes=stdout,
stderr_bytes=stderr,
exit_code=exit_code,
exception=exception,
exc_info=exc_info)

@jcrotts

This comment has been minimized.

Copy link
Member

@jcrotts jcrotts commented Dec 10, 2019

I think this is at least partially fixed here:
#1194

@adamtheturtle

This comment has been minimized.

Copy link
Contributor Author

@adamtheturtle adamtheturtle commented Dec 11, 2019

I think this is at least partially fixed here:
#1194

@jcrotts I don't think that #1194 relates to this issue - this issue concerns CliRunner.invoke and that PR concerns CliRunner.__init__.

@jcrotts

This comment has been minimized.

Copy link
Member

@jcrotts jcrotts commented Dec 11, 2019

Some behavior is changed in CliRunner.invoke, the docstring isn't changed though.
https://github.com/pallets/click/pull/1194/files#diff-637edab3f8e0763a9874d17879193dc9R350

@adamtheturtle

This comment has been minimized.

Copy link
Contributor Author

@adamtheturtle adamtheturtle commented Dec 11, 2019

I still don't understand how that in any way addresses this issue, which is that the mix_stderr parameter to CliRunner.invoke does nothing.

@davidism

This comment has been minimized.

Copy link
Member

@davidism davidism commented Dec 11, 2019

Hi Adam, you seem to have a specific idea in mind that we're having trouble understanding. If this is important to you and you have the time, it would probably be easier to create a PR fixing the issue rather than going back and forth more.

@adamtheturtle

This comment has been minimized.

Copy link
Contributor Author

@adamtheturtle adamtheturtle commented Dec 11, 2019

Good idea @davidism . I have created #1442 . I'm happy to back port / change this if you are happy with the general idea.

@adamtheturtle

This comment has been minimized.

Copy link
Contributor Author

@adamtheturtle adamtheturtle commented Dec 13, 2019

@jcrotts @davidism - I hope that the linked PR makes it clear what this issue is.

@miker985

This comment has been minimized.

Copy link

@miker985 miker985 commented Jan 16, 2020

I also just ran into this issue and was confused why stderr and stdout were mixed.

Since it's easy to set this on the CliRunner object I endorse the #1442

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.