Skip to content

Fix type errors after adding types to the py dependency #6511

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/config/argparsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
7 changes: 5 additions & 2 deletions src/_pytest/config/findpaths.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/_pytest/doctest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
11 changes: 7 additions & 4 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the error code here? (see --show-error-codes, could be added to the config probably) (with flake-ignore then)

Copy link
Member Author

Choose a reason for hiding this comment

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

We should probably do them all at once, maybe on the next mypy bump.


@property
def keywords(self):
Expand Down Expand Up @@ -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"
Expand All @@ -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,
)
)
Expand Down
8 changes: 4 additions & 4 deletions src/_pytest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,9 +367,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
Expand Down Expand Up @@ -399,7 +399,7 @@ def __init__(self, config):
self._node_cache = {}
self._bestrelpathcache = _bestrelpath_cache(
config.rootdir
) # type: Dict[str, str]
) # type: Dict[py.path.local, str]
# Dirnames of pkgs with dunder-init files.
self._pkg_roots = {}

Expand All @@ -414,7 +414,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]

Expand Down
1 change: 1 addition & 0 deletions src/_pytest/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Copy link
Contributor

Choose a reason for hiding this comment

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

You've removed the change from reportinfo, right? (from the initial Git tree)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.

fspath = self.session._node_location_to_relpath(location[0])
assert type(location[2]) is str
return (fspath, location[1], location[2])
6 changes: 3 additions & 3 deletions testing/python/collect.py
Original file line number Diff line number Diff line change
Expand Up @@ -1018,18 +1018,18 @@ class TestReportInfo:
def test_itemreport_reportinfo(self, testdir):
testdir.makeconftest(
"""
import pytest
import pytest, py
class MyFunction(pytest.Function):
def reportinfo(self):
return "ABCDE", 42, "custom"
return py.path.local("foo"), 42, "custom"
Copy link
Contributor

Choose a reason for hiding this comment

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

That's kind of an API change - I've noticed this myself that the tests seem to be the only things using str.
Ok with me in general, just wanted to mention this, i.e. some plugins might return strings there (but likely fail then afterwards, depending on where it is used I guess?)
Maybe this could/should have a test?

Copy link
Member Author

Choose a reason for hiding this comment

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

The _bestrelpathcache backing this only works with py.path.local, that it returned something for str was accidental and incorrect. So I think the assert just codifies the current behavior.

Only this test trigerred the assert, and it's just a mock, so I think it should be fine.

def pytest_pycollect_makeitem(collector, name, obj):
if name == "test_func":
return MyFunction(name, parent=collector)
"""
)
item = testdir.getitem("def test_func(): pass")
item.config.pluginmanager.getplugin("runner")
assert item.location == ("ABCDE", 42, "custom")
assert item.location == ("foo", 42, "custom")

def test_func_reportinfo(self, testdir):
item = testdir.getitem("def test_func(): pass")
Expand Down