Skip to content

Commit dd5c2b2

Browse files
committed
Refactor Session._initialparts to have a more explicit type
Previously, _initialparts was a list whose first item was a `py.path.local` and the rest were `str`s. This is not something that mypy is capable of modeling. The type `List[Union[str, py.path.local]]` is too broad and would require asserts for every access. Instead, make each item a `Tuple[py.path.local, List[str]]`. This way the structure is clear and the types are accurate. To make sure any users who might have been accessing this (private) field will not break silently, change the name to _initial_parts.
1 parent e17f5fa commit dd5c2b2

File tree

2 files changed

+26
-28
lines changed

2 files changed

+26
-28
lines changed

src/_pytest/main.py

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from typing import Dict
99
from typing import FrozenSet
1010
from typing import List
11+
from typing import Tuple
1112

1213
import attr
1314
import py
@@ -485,13 +486,13 @@ def _perform_collect(self, args, genitems):
485486
self.trace("perform_collect", self, args)
486487
self.trace.root.indent += 1
487488
self._notfound = []
488-
initialpaths = []
489-
self._initialparts = []
489+
initialpaths = [] # type: List[py.path.local]
490+
self._initial_parts = [] # type: List[Tuple[py.path.local, List[str]]]
490491
self.items = items = []
491492
for arg in args:
492-
parts = self._parsearg(arg)
493-
self._initialparts.append(parts)
494-
initialpaths.append(parts[0])
493+
fspath, parts = self._parsearg(arg)
494+
self._initial_parts.append((fspath, parts))
495+
initialpaths.append(fspath)
495496
self._initialpaths = frozenset(initialpaths)
496497
rep = collect_one_node(self)
497498
self.ihook.pytest_collectreport(report=rep)
@@ -511,13 +512,13 @@ def _perform_collect(self, args, genitems):
511512
return items
512513

513514
def collect(self):
514-
for initialpart in self._initialparts:
515-
self.trace("processing argument", initialpart)
515+
for fspath, parts in self._initial_parts:
516+
self.trace("processing argument", (fspath, parts))
516517
self.trace.root.indent += 1
517518
try:
518-
yield from self._collect(initialpart)
519+
yield from self._collect(fspath, parts)
519520
except NoMatch:
520-
report_arg = "::".join(map(str, initialpart))
521+
report_arg = "::".join((str(fspath), *parts))
521522
# we are inside a make_report hook so
522523
# we cannot directly pass through the exception
523524
self._notfound.append((report_arg, sys.exc_info()[1]))
@@ -526,12 +527,9 @@ def collect(self):
526527
self._collection_node_cache.clear()
527528
self._collection_pkg_roots.clear()
528529

529-
def _collect(self, arg):
530+
def _collect(self, argpath, names):
530531
from _pytest.python import Package
531532

532-
names = arg[:]
533-
argpath = names.pop(0)
534-
535533
# Start with a Session root, and delve to argpath item (dir or file)
536534
# and stack all Packages found on the way.
537535
# No point in finding packages when collecting doctests
@@ -555,7 +553,7 @@ def _collect(self, arg):
555553
# If it's a directory argument, recurse and look for any Subpackages.
556554
# Let the Package collector deal with subnodes, don't collect here.
557555
if argpath.check(dir=1):
558-
assert not names, "invalid arg {!r}".format(arg)
556+
assert not names, "invalid arg {!r}".format((argpath, names))
559557

560558
seen_dirs = set()
561559
for path in argpath.visit(
@@ -665,19 +663,19 @@ def _tryconvertpyarg(self, x):
665663

666664
def _parsearg(self, arg):
667665
""" return (fspath, names) tuple after checking the file exists. """
668-
parts = str(arg).split("::")
666+
strpath, *parts = str(arg).split("::")
669667
if self.config.option.pyargs:
670-
parts[0] = self._tryconvertpyarg(parts[0])
671-
relpath = parts[0].replace("/", os.sep)
672-
path = self.config.invocation_dir.join(relpath, abs=True)
673-
if not path.check():
668+
strpath = self._tryconvertpyarg(strpath)
669+
relpath = strpath.replace("/", os.sep)
670+
fspath = self.config.invocation_dir.join(relpath, abs=True)
671+
if not fspath.check():
674672
if self.config.option.pyargs:
675673
raise UsageError(
676674
"file or package not found: " + arg + " (missing __init__.py?)"
677675
)
678676
raise UsageError("file not found: " + arg)
679-
parts[0] = path.realpath()
680-
return parts
677+
fspath = fspath.realpath()
678+
return (fspath, parts)
681679

682680
def matchnodes(self, matching, names):
683681
self.trace("matchnodes", matching, names)

testing/test_collection.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ def pytest_collect_file(path, parent):
438438

439439

440440
class TestSession:
441-
def test_parsearg(self, testdir):
441+
def test_parsearg(self, testdir) -> None:
442442
p = testdir.makepyfile("def test_func(): pass")
443443
subdir = testdir.mkdir("sub")
444444
subdir.ensure("__init__.py")
@@ -448,14 +448,14 @@ def test_parsearg(self, testdir):
448448
config = testdir.parseconfig(p.basename)
449449
rcol = Session.from_config(config)
450450
assert rcol.fspath == subdir
451-
parts = rcol._parsearg(p.basename)
451+
fspath, parts = rcol._parsearg(p.basename)
452452

453-
assert parts[0] == target
453+
assert fspath == target
454+
assert len(parts) == 0
455+
fspath, parts = rcol._parsearg(p.basename + "::test_func")
456+
assert fspath == target
457+
assert parts[0] == "test_func"
454458
assert len(parts) == 1
455-
parts = rcol._parsearg(p.basename + "::test_func")
456-
assert parts[0] == target
457-
assert parts[1] == "test_func"
458-
assert len(parts) == 2
459459

460460
def test_collect_topdir(self, testdir):
461461
p = testdir.makepyfile("def test_func(): pass")

0 commit comments

Comments
 (0)