From 8fa57c8384818efcc83af604f0ccfc2620dd111c Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 21 Jan 2020 11:30:32 +0100 Subject: [PATCH 1/6] tests: improve test for `nose.raises` This should probably get transferred into a `pytest.fail` really, but tests/documents the current behavior. --- testing/test_nose.py | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/testing/test_nose.py b/testing/test_nose.py index 469c127afc4..15be7b73a20 100644 --- a/testing/test_nose.py +++ b/testing/test_nose.py @@ -377,15 +377,45 @@ def test_io(self): result.stdout.fnmatch_lines(["* 1 skipped *"]) -def test_issue_6517(testdir): +def test_raises(testdir): testdir.makepyfile( """ from nose.tools import raises @raises(RuntimeError) - def test_fail_without_tcp(): + def test_raises_runtimeerror(): raise RuntimeError + + @raises(Exception) + def test_raises_baseexception_not_caught(): + raise BaseException + + @raises(BaseException) + def test_raises_baseexception_caught(): + raise BaseException """ ) result = testdir.runpytest() - result.stdout.fnmatch_lines(["* 1 passed *"]) + result.stdout.fnmatch_lines( + [ + "*= FAILURES =*", + "*_ test_raises_baseexception_not_caught _*", + "", + "arg = (), kw = {}", + "", + " def newfunc(*arg, **kw):", + " try:", + "> func(*arg, **kw)", + "", + "*/nose/*: ", + "_ _ *", + "", + " @raises(Exception)", + " def test_raises_baseexception_not_caught():", + "> raise BaseException", + "E BaseException", + "", + "test_raises.py:9: BaseException", + "* 1 failed, 2 passed *", + ] + ) From ef112fd7dd8ee220865f7569c0ecc4fa5eb24464 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 21 Jan 2020 12:29:22 +0100 Subject: [PATCH 2/6] Revert "Revert "Fix type errors after adding types to the `py` dependency"" Without changes to test_itemreport_reportinfo. This reverts commit fb99b5c66ee06ad0bd3336d8599448d1d3da4f7f. Conflicts: testing/test_nose.py --- src/_pytest/config/argparsing.py | 8 ++++---- src/_pytest/config/findpaths.py | 7 +++++-- src/_pytest/doctest.py | 2 +- src/_pytest/fixtures.py | 11 +++++++---- src/_pytest/main.py | 8 ++++---- src/_pytest/nodes.py | 1 + 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/_pytest/config/argparsing.py b/src/_pytest/config/argparsing.py index 7cbb676bd70..8817c57495a 100644 --- a/src/_pytest/config/argparsing.py +++ b/src/_pytest/config/argparsing.py @@ -82,8 +82,8 @@ def parse(self, args, namespace=None): self.optparser = self._getparser() try_argcomplete(self.optparser) - args = [str(x) if isinstance(x, py.path.local) else x for x in args] - return self.optparser.parse_args(args, namespace=namespace) + strargs = [str(x) if isinstance(x, py.path.local) else x for x in args] + return self.optparser.parse_args(strargs, namespace=namespace) def _getparser(self) -> "MyOptionParser": from _pytest._argcomplete import filescompleter @@ -124,8 +124,8 @@ def parse_known_and_unknown_args( the remaining arguments unknown at this point. """ optparser = self._getparser() - args = [str(x) if isinstance(x, py.path.local) else x for x in args] - return optparser.parse_known_args(args, namespace=namespace) + strargs = [str(x) if isinstance(x, py.path.local) else x for x in args] + return optparser.parse_known_args(strargs, namespace=namespace) def addini(self, name, help, type=None, default=None): """ register an ini-file option. diff --git a/src/_pytest/config/findpaths.py b/src/_pytest/config/findpaths.py index 707ce969d93..fb84160c1ff 100644 --- a/src/_pytest/config/findpaths.py +++ b/src/_pytest/config/findpaths.py @@ -1,6 +1,9 @@ import os +from typing import Any +from typing import Iterable from typing import List from typing import Optional +from typing import Tuple import py @@ -60,7 +63,7 @@ def getcfg(args, config=None): return None, None, None -def get_common_ancestor(paths): +def get_common_ancestor(paths: Iterable[py.path.local]) -> py.path.local: common_ancestor = None for path in paths: if not path.exists(): @@ -113,7 +116,7 @@ def determine_setup( args: List[str], rootdir_cmd_arg: Optional[str] = None, config: Optional["Config"] = None, -): +) -> Tuple[py.path.local, Optional[str], Any]: dirs = get_dirs_from_args(args) if inifile: iniconfig = py.iniconfig.IniConfig(inifile) diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py index e62c5e17ea4..d7ca888cc5f 100644 --- a/src/_pytest/doctest.py +++ b/src/_pytest/doctest.py @@ -308,7 +308,7 @@ def repr_failure(self, excinfo): else: return super().repr_failure(excinfo) - def reportinfo(self) -> Tuple[str, int, str]: + def reportinfo(self) -> Tuple[py.path.local, int, str]: return self.fspath, self.dtest.lineno, "[doctest] %s" % self.name diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index bae3d071674..f0a1a2ed078 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -351,7 +351,7 @@ def __init__(self, pyfuncitem): self.fixturename = None #: Scope string, one of "function", "class", "module", "session" self.scope = "function" - self._fixture_defs = {} # argname -> FixtureDef + self._fixture_defs = {} # type: Dict[str, FixtureDef] fixtureinfo = pyfuncitem._fixtureinfo self._arg2fixturedefs = fixtureinfo.name2fixturedefs.copy() self._arg2index = {} @@ -426,7 +426,8 @@ def module(self): @scopeproperty() def fspath(self) -> py.path.local: """ the file system path of the test module which collected this test. """ - return self._pyfuncitem.fspath + # TODO: Remove ignore once _pyfuncitem is properly typed. + return self._pyfuncitem.fspath # type: ignore @property def keywords(self): @@ -549,7 +550,9 @@ def _compute_fixture_value(self, fixturedef): source_lineno = frameinfo.lineno source_path = py.path.local(source_path) if source_path.relto(funcitem.config.rootdir): - source_path = source_path.relto(funcitem.config.rootdir) + source_path_str = source_path.relto(funcitem.config.rootdir) + else: + source_path_str = str(source_path) msg = ( "The requested fixture has no parameter defined for test:\n" " {}\n\n" @@ -558,7 +561,7 @@ def _compute_fixture_value(self, fixturedef): funcitem.nodeid, fixturedef.argname, getlocation(fixturedef.func, funcitem.config.rootdir), - source_path, + source_path_str, source_lineno, ) ) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 5137713d95c..ae115b1f8a9 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -376,9 +376,9 @@ class Failed(Exception): @attr.s class _bestrelpath_cache(dict): - path = attr.ib() + path = attr.ib(type=py.path.local) - def __missing__(self, path: str) -> str: + def __missing__(self, path: py.path.local) -> str: r = self.path.bestrelpath(path) # type: str self[path] = r return r @@ -412,7 +412,7 @@ def __init__(self, config: Config) -> None: self._bestrelpathcache = _bestrelpath_cache( config.rootdir - ) # type: Dict[str, str] + ) # type: Dict[py.path.local, str] self.config.pluginmanager.register(self, name="session") @@ -425,7 +425,7 @@ def __repr__(self): self.testscollected, ) - def _node_location_to_relpath(self, node_path: str) -> str: + def _node_location_to_relpath(self, node_path: py.path.local) -> str: # bestrelpath is a quite slow function return self._bestrelpathcache[node_path] diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 3cfbf46264c..fc951d2bc75 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -462,6 +462,7 @@ def reportinfo(self) -> Tuple[Union[py.path.local, str], Optional[int], str]: @cached_property def location(self) -> Tuple[str, Optional[int], str]: location = self.reportinfo() + assert isinstance(location[0], py.path.local), location[0] fspath = self.session._node_location_to_relpath(location[0]) assert type(location[2]) is str return (fspath, location[1], location[2]) From 1350c601dcf013ae773233206ac1912093dddad9 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 21 Jan 2020 12:26:29 +0100 Subject: [PATCH 3/6] Node.location: handle str with _node_location_to_relpath --- src/_pytest/nodes.py | 6 ++++-- testing/test_nose.py | 5 ++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index fc951d2bc75..080f079cd86 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -462,7 +462,9 @@ def reportinfo(self) -> Tuple[Union[py.path.local, str], Optional[int], str]: @cached_property def location(self) -> Tuple[str, Optional[int], str]: location = self.reportinfo() - assert isinstance(location[0], py.path.local), location[0] - fspath = self.session._node_location_to_relpath(location[0]) + fspath = location[0] + if not isinstance(fspath, py.path.local): + fspath = py.path.local(fspath) + fspath = self.session._node_location_to_relpath(fspath) assert type(location[2]) is str return (fspath, location[1], location[2]) diff --git a/testing/test_nose.py b/testing/test_nose.py index 15be7b73a20..b6200c6c9ad 100644 --- a/testing/test_nose.py +++ b/testing/test_nose.py @@ -395,9 +395,12 @@ def test_raises_baseexception_caught(): raise BaseException """ ) - result = testdir.runpytest() + result = testdir.runpytest("-vv") result.stdout.fnmatch_lines( [ + "test_raises.py::test_raises_runtimeerror PASSED*", + "test_raises.py::test_raises_baseexception_not_caught FAILED*", + "test_raises.py::test_raises_baseexception_caught PASSED*", "*= FAILURES =*", "*_ test_raises_baseexception_not_caught _*", "", From 9c7b3c57d7d6e965eb135d793bb69ef0041aabf5 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 21 Jan 2020 12:51:16 +0100 Subject: [PATCH 4/6] typing: PyobjMixin.reportinfo, getfslineno --- src/_pytest/compat.py | 2 +- src/_pytest/python.py | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index 6a62e88cfca..f0b0d548f07 100644 --- a/src/_pytest/compat.py +++ b/src/_pytest/compat.py @@ -307,7 +307,7 @@ def get_real_method(obj, holder): return obj -def getfslineno(obj): +def getfslineno(obj) -> Tuple[Union[str, py.path.local], int]: # xxx let decorators etc specify a sane ordering obj = get_real_func(obj) if hasattr(obj, "place_as"): diff --git a/src/_pytest/python.py b/src/_pytest/python.py index bc35ccf5f10..3d2916c83a2 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -12,6 +12,7 @@ from textwrap import dedent from typing import List from typing import Tuple +from typing import Union import py @@ -280,15 +281,16 @@ def getmodpath(self, stopatmodule=True, includemodule=False): parts.reverse() return ".".join(parts) - def reportinfo(self) -> Tuple[str, int, str]: + def reportinfo(self) -> Tuple[Union[py.path.local, str], int, str]: # XXX caching? obj = self.obj compat_co_firstlineno = getattr(obj, "compat_co_firstlineno", None) if isinstance(compat_co_firstlineno, int): # nose compatibility - fspath = sys.modules[obj.__module__].__file__ - if fspath.endswith(".pyc"): - fspath = fspath[:-1] + file_path = sys.modules[obj.__module__].__file__ + if file_path.endswith(".pyc"): + file_path = file_path[:-1] + fspath = file_path # type: Union[py.path.local, str] lineno = compat_co_firstlineno else: fspath, lineno = getfslineno(obj) From 0b6258ab5bce985b14f28aa9e7bfe828b1d5b8c3 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Thu, 23 Jan 2020 12:54:52 +0100 Subject: [PATCH 5/6] PyCollector.collect: use explicit cast to `str` Ref: https://github.com/pytest-dev/pytest/pull/6521#pullrequestreview-347234792 --- src/_pytest/python.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 3d2916c83a2..82dca3bcc9b 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -369,7 +369,12 @@ def collect(self): if not isinstance(res, list): res = [res] values.extend(res) - values.sort(key=lambda item: item.reportinfo()[:2]) + + def sort_key(item): + fspath, lineno, _ = item.reportinfo() + return (str(fspath), lineno) + + values.sort(key=sort_key) return values def _makeitem(self, name, obj): From 9dcdea5de7a9d7f3d24a59182682fa6a0fd59bdb Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Thu, 23 Jan 2020 13:25:15 +0100 Subject: [PATCH 6/6] Rewrite Item.location to be clearer with regard to types --- src/_pytest/nodes.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 080f079cd86..ab976efae72 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -462,9 +462,10 @@ def reportinfo(self) -> Tuple[Union[py.path.local, str], Optional[int], str]: @cached_property def location(self) -> Tuple[str, Optional[int], str]: location = self.reportinfo() - fspath = location[0] - if not isinstance(fspath, py.path.local): - fspath = py.path.local(fspath) - fspath = self.session._node_location_to_relpath(fspath) + if isinstance(location[0], py.path.local): + fspath = location[0] + else: + fspath = py.path.local(location[0]) + relfspath = self.session._node_location_to_relpath(fspath) assert type(location[2]) is str - return (fspath, location[1], location[2]) + return (relfspath, location[1], location[2])