Handle flags in pytest passthrough #3467
Conversation
| @@ -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), | |||
There was a problem hiding this comment.
Additionally, I'd append -q to args if verbosity is < 0 and a number of -v flags if it is >= 1?
There was a problem hiding this comment.
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".
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
mypy/waiter.py
Outdated
| @@ -37,13 +37,11 @@ def __init__(self, name: str, args: List[str], *, cwd: str = None, | |||
| self.passthrough = passthrough | |||
|
|
|||
| def start(self) -> None: | |||
| if self.passthrough is None: | |||
| if self.passthrough is None or self.passthrough < 0: | |||
| self.outfile = tempfile.TemporaryFile() | |||
| else: | |||
| if self.passthrough >= 0: | |||
There was a problem hiding this comment.
You don't need this if -- it makes it look as if there's a case where self.outfile won't be set at all.
There was a problem hiding this comment.
Oh right. Also, currently with python runtests.py -q I completely suppress pytest output (since that was the original behavior of python runtests.py -q). Should I perhaps still let it pass through its output? With -q, pytest output is just a bunch of dots, so it actually fits well with runtests.py output.
There was a problem hiding this comment.
Never mind, pytest -q misbehaves in the presence of xdist, so I'm keeping the behavior as is.
There was a problem hiding this comment.
Even without xdist I can't get pytest to not print the row of dots. So agreed, let's keep the existing behavior of runtests.py not passing through any output on -q.
|
-q and -v are compatible in the sense that they decrease/increase verbosity in pytest; and lower/higher verbosity does behave similarly for pytest and runtests.py. |
|
OK, let's see how people like this. |
* master: Support accessing modules imported in class bodies within methods. (python#3450) Make Type[X] compatible with metaclass of X (python#3346) Sync typeshed (python#3479) Handle flags in pytest passthrough (python#3467)
Continuing #3463 to handle flags.
We should probably collect a few more reports on flags not handled well before merging.