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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Capturable* argparse structures to mypy.main #7804

Merged
merged 1 commit into from Oct 29, 2019

Conversation

@dmtucker
Copy link
Contributor

dmtucker commented Oct 28, 2019

Fix #7800

Most of this code is yanked from the standard library since the same behaviors are desired, just with plumbing for stdout/stderr. One difference of note is that version is mandatory in CapturableVersionAction.__init__ to get around this error:

mypy/main.py:339: error: "ArgumentParser" has no attribute "version"  [attr-defined]
                version = parser.version
                          ^

I'm unaware of a situation where that line wouldn't raise an AttributeError 馃
Anyways, I didn't want to patch sys.stdout/sys.stderr in order to avoid reintroducing #6125.

Copy link
Collaborator

JukkaL left a comment

Thanks! It looks like this is the most reasonable way of doing this, even though there's quite a bit of boilerplate involved. Generally looks good, just some minor nits.

def test_capture_bad_opt(self) -> None:
"""stderr should be captured when a bad option is passed."""
_, stderr, _ = mypy.api.run(['--some-bad-option'])
assert stderr != ''

This comment has been minimized.

Copy link
@JukkaL

JukkaL Oct 28, 2019

Collaborator

Also test that the return value is a string, as otherwise the inequality will be trivially true. This might happen if it's None, at least. Maybe like this: assert isinstance(stderr, str) and stderr != ''

def test_capture_empty(self) -> None:
"""stderr should be captured when a bad option is passed."""
_, stderr, _ = mypy.api.run([])
assert stderr != ''

This comment has been minimized.

Copy link
@JukkaL

JukkaL Oct 28, 2019

Collaborator

Similar to above.

def test_capture_help(self) -> None:
"""stdout should be captured when --help is passed."""
stdout, _, _ = mypy.api.run(['--help'])
assert stdout != ''

This comment has been minimized.

Copy link
@JukkaL

JukkaL Oct 28, 2019

Collaborator

Similar to above.

def test_capture_version(self) -> None:
"""stdout should be captured when --version is passed."""
stdout, _, _ = mypy.api.run(['--version'])
assert stdout != ''

This comment has been minimized.

Copy link
@JukkaL

JukkaL Oct 28, 2019

Collaborator

Similar to above.

self.exit(2, gettext('%(prog)s: error: %(message)s\n') % args)


class CapturableVersionAction(argparse.Action):

This comment has been minimized.

Copy link
@JukkaL

JukkaL Oct 28, 2019

Collaborator

Add docstring (maybe refer to the docstring that I requested for CapturableArgumentParser).

@@ -252,6 +257,89 @@ def infer_python_executable(options: Options,
Define MYPY_CACHE_DIR to override configuration cache_dir path.""" # type: Final


class CapturableArgumentParser(argparse.ArgumentParser):

This comment has been minimized.

Copy link
@JukkaL

JukkaL Oct 28, 2019

Collaborator

Add docstring that explains what this does and why we need to do this.


def __init__(self, *args: Any, **kwargs: Any):
try:
self.stdout = kwargs.pop('stdout')

This comment has been minimized.

Copy link
@JukkaL

JukkaL Oct 28, 2019

Collaborator

Maybe you can avoid the try statements by using kwargs.pop('stdout', sys.stdout) (and similarly for stderr).

@dmtucker dmtucker force-pushed the dmtucker:master branch from 5d365ac to 8ef261c Oct 28, 2019
@JukkaL
JukkaL approved these changes Oct 29, 2019
Copy link
Collaborator

JukkaL left a comment

Thanks for the updates! Looks good now.

@JukkaL JukkaL merged commit dec5451 into python:master Oct 29, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.