Skip to content

[RFC] typing around terminal #6157

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 1 commit into from
Nov 9, 2019
Merged

[RFC] typing around terminal #6157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions src/_pytest/reports.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def getslaveinfoline(node):

class BaseReport:
when = None # type: Optional[str]
location = None
location = None # type: Optional[Tuple[str, Optional[int], str]]
longrepr = None
sections = [] # type: List[Tuple[str, str]]
nodeid = None # type: str
Expand Down Expand Up @@ -207,7 +207,7 @@ class TestReport(BaseReport):
def __init__(
self,
nodeid,
location,
location: Tuple[str, Optional[int], str],
keywords,
outcome,
longrepr,
Expand All @@ -216,14 +216,14 @@ def __init__(
duration=0,
user_properties=None,
**extra
):
) -> None:
#: normalized collection node id
self.nodeid = nodeid

#: a (filesystempath, lineno, domaininfo) tuple indicating the
#: actual location of a test item - it might be different from the
#: collected one e.g. if a method is inherited from a different module.
self.location = location
self.location = location # type: Tuple[str, Optional[int], str]
Copy link
Member

Choose a reason for hiding this comment

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

I think that adding an annotation to a function whose own signature is unannotated doesn't have an effect (ref).

So it is better to add this annotation to the location parameter instead, if possible. This will make mypy consider the function to be typed.

Copy link
Contributor Author

@blueyed blueyed Nov 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

reveal_type gives me 'Tuple[builtins.str, Union[builtins.int, None], builtins.str]' here (mypy 0.750+dev.84126ab9985844d8cda3809688034af895fdaf7c.dirty).

This appears to come / being enabled via BaseReport - adding it there (and to the def here) instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But it is needed to fix "List or tuple expected as variable arguments" with https://github.com/blueyed/pytest/blob/0133097943e0140ab82e07d4d201405145d35e59/src/_pytest/terminal.py#L442.
But it seems strange that with "location: Tuple[str, Optional[int], str]" in TestReport.__init__ and rep being _pytest.reports.TestReport, rep.location is Union[Tuple[builtins.str, Union[builtins.int, None], builtins.str], None] then?

The problem appears to be that BaseReport has location = None # type: Optional[Tuple[str, Optional[int], str]], and we cannot have location: Tuple[str, Optional[int], str] in TestReport (py35).
Adding an assert appears to work also, but then again it is better to keep the type comment there.


#: a name -> value dictionary containing all keywords and
#: markers associated with a test invocation.
Expand Down
50 changes: 30 additions & 20 deletions src/_pytest/terminal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import sys
import time
from functools import partial
from typing import Any
from typing import Callable
from typing import Dict
from typing import List
Expand All @@ -24,7 +25,11 @@

import pytest
from _pytest import nodes
from _pytest.config import Config
from _pytest.main import ExitCode
from _pytest.main import Session
from _pytest.reports import CollectReport
from _pytest.reports import TestReport

REPORT_COLLECTING_RESOLUTION = 0.5

Expand Down Expand Up @@ -148,7 +153,7 @@ def pytest_addoption(parser):
)


def pytest_configure(config):
def pytest_configure(config: Config) -> None:
reporter = TerminalReporter(config, sys.stdout)
config.pluginmanager.register(reporter, "terminalreporter")
if config.option.debug or config.option.traceconfig:
Expand All @@ -160,7 +165,7 @@ def mywriter(tags, args):
config.trace.root.setprocessor("pytest:config", mywriter)


def getreportopt(config):
def getreportopt(config: Config) -> str:
reportopts = ""
reportchars = config.option.reportchars
if not config.option.disable_warnings and "w" not in reportchars:
Expand All @@ -179,7 +184,7 @@ def getreportopt(config):


@pytest.hookimpl(trylast=True) # after _pytest.runner
def pytest_report_teststatus(report):
def pytest_report_teststatus(report: TestReport) -> Tuple[str, str, str]:
if report.passed:
letter = "."
elif report.skipped:
Expand Down Expand Up @@ -233,29 +238,29 @@ def get_location(self, config):


class TerminalReporter:
def __init__(self, config, file=None):
def __init__(self, config: Config, file=None) -> None:
import _pytest.config

self.config = config
self._numcollected = 0
self._session = None
self._session = None # type: Optional[Session]
self._showfspath = None

self.stats = {}
self.stats = {} # type: Dict[str, List[Any]]
self.startdir = config.invocation_dir
if file is None:
file = sys.stdout
self._tw = _pytest.config.create_terminal_writer(config, file)
# self.writer will be deprecated in pytest-3.4
self.writer = self._tw
self._screen_width = self._tw.fullwidth
self.currentfspath = None
self.currentfspath = None # type: Optional[int]
self.reportchars = getreportopt(config)
self.hasmarkup = self._tw.hasmarkup
self.isatty = file.isatty()
self._progress_nodeids_reported = set() # type: Set[str]
self._show_progress_info = self._determine_show_progress_info()
self._collect_report_last_write = None
self._collect_report_last_write = None # type: Optional[float]

def _determine_show_progress_info(self):
"""Return True if we should display progress information based on the current config"""
Expand Down Expand Up @@ -400,7 +405,7 @@ def pytest_runtest_logstart(self, nodeid, location):
fsid = nodeid.split("::")[0]
self.write_fspath_result(fsid, "")

def pytest_runtest_logreport(self, report):
def pytest_runtest_logreport(self, report: TestReport) -> None:
self._tests_ran = True
rep = report
res = self.config.hook.pytest_report_teststatus(report=rep, config=self.config)
Expand Down Expand Up @@ -440,7 +445,7 @@ def pytest_runtest_logreport(self, report):
self._write_progress_information_filling_space()
else:
self.ensure_newline()
self._tw.write("[%s]" % rep.node.gateway.id)
self._tw.write("[%s]" % rep.node.gateway.id) # type: ignore
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to have a short comment on why the type: ignore is needed here. This why we'll be able to tell later whether it's a "TODO" or a "workaround mypy" thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a workaround-xdist thing (I assume that xdist just puts it there).

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'll keep it as is (it is somehow obvious from "xdist" in there, and I do not like extra comments for this above). Also it might get reported when not being used/needed later anymore (there's a mode in mypy to do so).

if self._show_progress_info:
self._tw.write(
self._get_progress_information_message() + " ", cyan=True
Expand All @@ -452,6 +457,7 @@ def pytest_runtest_logreport(self, report):
self.currentfspath = -2

def pytest_runtest_logfinish(self, nodeid):
assert self._session
if self.verbosity <= 0 and self._show_progress_info:
if self._show_progress_info == "count":
num_tests = self._session.testscollected
Expand All @@ -474,7 +480,8 @@ def pytest_runtest_logfinish(self, nodeid):
msg = self._get_progress_information_message()
self._tw.write(msg + "\n", **{main_color: True})

def _get_progress_information_message(self):
def _get_progress_information_message(self) -> str:
assert self._session
collected = self._session.testscollected
if self._show_progress_info == "count":
if collected:
Expand All @@ -485,8 +492,9 @@ def _get_progress_information_message(self):
return " [ {} / {} ]".format(collected, collected)
else:
if collected:
progress = len(self._progress_nodeids_reported) * 100 // collected
return " [{:3d}%]".format(progress)
return " [{:3d}%]".format(
len(self._progress_nodeids_reported) * 100 // collected
)
return " [100%]"

def _write_progress_information_filling_space(self, color=None):
Expand Down Expand Up @@ -514,7 +522,7 @@ def pytest_collection(self):
elif self.config.option.verbose >= 1:
self.write("collecting ... ", bold=True)

def pytest_collectreport(self, report):
def pytest_collectreport(self, report: CollectReport) -> None:
if report.failed:
self.stats.setdefault("error", []).append(report)
elif report.skipped:
Expand Down Expand Up @@ -565,17 +573,18 @@ def report_collect(self, final=False):
self.write_line(line)

@pytest.hookimpl(trylast=True)
def pytest_sessionstart(self, session):
def pytest_sessionstart(self, session: Session) -> None:
self._session = session
self._sessionstarttime = time.time()
if not self.showheader:
return
self.write_sep("=", "test session starts", bold=True)
verinfo = platform.python_version()
msg = "platform {} -- Python {}".format(sys.platform, verinfo)
if hasattr(sys, "pypy_version_info"):
verinfo = ".".join(map(str, sys.pypy_version_info[:3]))
msg += "[pypy-{}-{}]".format(verinfo, sys.pypy_version_info[3])
pypy_version_info = getattr(sys, "pypy_version_info", None)
if pypy_version_info:
verinfo = ".".join(map(str, pypy_version_info[:3]))
msg += "[pypy-{}-{}]".format(verinfo, pypy_version_info[3])
msg += ", pytest-{}, py-{}, pluggy-{}".format(
pytest.__version__, py.__version__, pluggy.__version__
)
Expand Down Expand Up @@ -625,9 +634,10 @@ def pytest_collection_finish(self, session):
self._write_report_lines_from_hooks(lines)

if self.config.getoption("collectonly"):
if self.stats.get("failed"):
failed = self.stats.get("failed")
if failed:
self._tw.sep("!", "collection failures")
for rep in self.stats.get("failed"):
for rep in failed:
rep.toterminal(self._tw)

def _printcollecteditems(self, items):
Expand Down