Skip to content

Handle flags in pytest passthrough #3467

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 30, 2017
Merged

Handle flags in pytest passthrough #3467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba2c9f1
Handle -q option
pkch May 26, 2017
faeff18
CR: pass -j, -q, -v to pytest
pkch May 30, 2017
2988e03
CR fix
pkch May 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions mypy/waiter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from multiprocessing import cpu_count
import pipes
import re
from subprocess import Popen, STDOUT
from subprocess import Popen, STDOUT, DEVNULL
import sys
import tempfile
import time
Expand All @@ -25,20 +25,22 @@ class LazySubprocess:
"""Wrapper around a subprocess that runs a test task."""

def __init__(self, name: str, args: List[str], *, cwd: str = None,
env: Dict[str, str] = None, passthrough: bool = False) -> None:
env: Dict[str, str] = None, passthrough: Optional[int] = None) -> None:
self.name = name
self.args = args
self.cwd = cwd
self.env = env
self.start_time = None # type: float
self.end_time = None # type: float
# None means no passthrough
# otherwise, it represents verbosity level
self.passthrough = passthrough

def start(self) -> None:
if self.passthrough:
self.outfile = None
else:
if self.passthrough is None or self.passthrough < 0:
self.outfile = tempfile.TemporaryFile()
else:
self.outfile = None
self.start_time = time.perf_counter()
self.process = Popen(self.args, cwd=self.cwd, env=self.env,
stdout=self.outfile, stderr=STDOUT)
Expand All @@ -51,7 +53,7 @@ def status(self) -> Optional[int]:
return self.process.returncode

def read_output(self) -> str:
if self.passthrough:
if not self.outfile:
return ''
file = self.outfile
file.seek(0)
Expand Down
9 changes: 8 additions & 1 deletion runtests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def add_pytest(self, name: str, pytest_args: List[str], coverage: bool = False)
else:
args = [sys.executable, '-m', 'pytest'] + pytest_args

self.waiter.add(LazySubprocess(full_name, args, env=self.env, passthrough=True),
self.waiter.add(LazySubprocess(full_name, args, env=self.env, passthrough=self.verbosity),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, I'd append -q to args if verbosity is < 0 and a number of -v flags if it is >= 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can handle -j and -q/-v this way, but we'll need to be careful with the logic (e.g., if there's -v and -p -v, should those -v be added up, or does one supercede the other?).

The other option is not to pass through -j / -q / -v, and just let the user set pytest options directly using the existing -p flag which passes the next argument directly to pytest: python runtests.py -j8 -p -n4 currently means "run pytest with 4 parallel workers, and the other tests with 8".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather not have to use the (rather obscure) -p flag. I think we should compute appropriate verbosity and parallelism flags for pytest from the runtests.py flags -- if you want to run pytest more verbose you can run it manually, and if you want to run it less verbose, well, verbosity is not very subtle.

The other alternative is not to do anything more about this (apart from merging this PR) and instead work on getting rid of runtests.py altogether. I can never figure out how to select a test anyways (mostly because I don't know how the output from a failed test translates into arguments that run just that test).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, for now I updated this PR to handle -q, -v, and -j; I think moving to pytests is slowly happening, but will take time.

sequential=True)

def add_python(self, name: str, *args: str, cwd: Optional[str] = None) -> None:
Expand Down Expand Up @@ -422,6 +422,13 @@ def main() -> None:
pyt_arglist.append('--lf')
if ff:
pyt_arglist.append('--ff')
if verbosity >= 1:
pyt_arglist.extend(['-v'] * verbosity)
elif verbosity < 0:
pyt_arglist.extend(['-q'] * (-verbosity))
if parallel_limit:
if '-n' not in pyt_arglist:
pyt_arglist.append('-n{}'.format(parallel_limit))

driver = Driver(whitelist=whitelist, blacklist=blacklist, lf=lf, ff=ff,
arglist=arglist, pyt_arglist=pyt_arglist, verbosity=verbosity,
Expand Down