From b426bb3443b0895f8a84379ff4a9949330012d38 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 15 Aug 2020 10:31:17 -0300 Subject: [PATCH] Refactor Session._parsearg into a separate function for testing --- src/_pytest/main.py | 91 +++++++++++++++--------- testing/acceptance_test.py | 6 +- testing/test_collection.py | 58 --------------- testing/test_main.py | 141 +++++++++++++++++++++++++++++++++++++ testing/test_terminal.py | 4 +- 5 files changed, 206 insertions(+), 94 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index e0ca1c0b5..9df9d33b9 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -549,7 +549,9 @@ class Session(nodes.FSCollector): self._initial_parts = [] # type: List[Tuple[py.path.local, List[str]]] self.items = items = [] # type: List[nodes.Item] for arg in args: - fspath, parts = self._parsearg(arg) + fspath, parts = resolve_collection_argument( + self.config.invocation_dir, arg, as_pypath=self.config.option.pyargs + ) self._initial_parts.append((fspath, parts)) initialpaths.append(fspath) self._initialpaths = frozenset(initialpaths) @@ -673,37 +675,6 @@ class Session(nodes.FSCollector): return yield from m - def _tryconvertpyarg(self, x: str) -> str: - """Convert a dotted module name to path.""" - try: - spec = importlib.util.find_spec(x) - # AttributeError: looks like package module, but actually filename - # ImportError: module does not exist - # ValueError: not a module name - except (AttributeError, ImportError, ValueError): - return x - if spec is None or spec.origin is None or spec.origin == "namespace": - return x - elif spec.submodule_search_locations: - return os.path.dirname(spec.origin) - else: - return spec.origin - - def _parsearg(self, arg: str) -> Tuple[py.path.local, List[str]]: - """Return (fspath, names) tuple after checking the file exists.""" - strpath, *parts = str(arg).split("::") - if self.config.option.pyargs: - strpath = self._tryconvertpyarg(strpath) - fspath = Path(str(self.config.invocation_dir), strpath) - fspath = absolutepath(fspath) - if not fspath.exists(): - if self.config.option.pyargs: - raise UsageError( - "file or package not found: " + arg + " (missing __init__.py?)" - ) - raise UsageError("file not found: " + arg) - return py.path.local(str(fspath)), parts - def matchnodes( self, matching: Sequence[Union[nodes.Item, nodes.Collector]], names: List[str], ) -> Sequence[Union[nodes.Item, nodes.Collector]]: @@ -770,3 +741,59 @@ class Session(nodes.FSCollector): for subnode in rep.result: yield from self.genitems(subnode) node.ihook.pytest_collectreport(report=rep) + + +def search_pypath(module_name: str) -> str: + """Search sys.path for the given a dotted module name, and return its file system path.""" + try: + spec = importlib.util.find_spec(module_name) + # AttributeError: looks like package module, but actually filename + # ImportError: module does not exist + # ValueError: not a module name + except (AttributeError, ImportError, ValueError): + return module_name + if spec is None or spec.origin is None or spec.origin == "namespace": + return module_name + elif spec.submodule_search_locations: + return os.path.dirname(spec.origin) + else: + return spec.origin + + +def resolve_collection_argument( + invocation_dir: py.path.local, arg: str, *, as_pypath: bool = False +) -> Tuple[py.path.local, List[str]]: + """Parse path arguments optionally containing selection parts and return (fspath, names). + + Command-line arguments can point to files and/or directories, and optionally contain + parts for specific tests selection, for example: + + "pkg/tests/test_foo.py::TestClass::test_foo" + + This function ensures the path exists, and returns a tuple: + + (py.path.path("/full/path/to/pkg/tests/test_foo.py"), ["TestClass", "test_foo"]) + + When as_pypath is True, expects that the command-line argument actually contains + module paths instead of file-system paths: + + "pkg.tests.test_foo::TestClass::test_foo" + + In which case we search sys.path for a matching module, and then return the *path* to the + found module. + + If the path doesn't exist, raise UsageError. + """ + strpath, *parts = str(arg).split("::") + if as_pypath: + strpath = search_pypath(strpath) + fspath = Path(str(invocation_dir), strpath) + fspath = absolutepath(fspath) + if not fspath.exists(): + msg = ( + "module or package not found: {arg} (missing __init__.py?)" + if as_pypath + else "file or directory not found: {arg}" + ) + raise UsageError(msg.format(arg=arg)) + return py.path.local(str(fspath)), parts diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index b37cfa0cb..d58b4fd03 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -70,7 +70,7 @@ class TestGeneralUsage: def test_file_not_found(self, testdir): result = testdir.runpytest("asd") assert result.ret != 0 - result.stderr.fnmatch_lines(["ERROR: file not found*asd"]) + result.stderr.fnmatch_lines(["ERROR: file or directory not found: asd"]) def test_file_not_found_unconfigure_issue143(self, testdir): testdir.makeconftest( @@ -83,7 +83,7 @@ class TestGeneralUsage: ) result = testdir.runpytest("-s", "asd") assert result.ret == ExitCode.USAGE_ERROR - result.stderr.fnmatch_lines(["ERROR: file not found*asd"]) + result.stderr.fnmatch_lines(["ERROR: file or directory not found: asd"]) result.stdout.fnmatch_lines(["*---configure", "*---unconfigure"]) def test_config_preparse_plugin_option(self, testdir): @@ -791,7 +791,7 @@ class TestInvocationVariants: def test_cmdline_python_package_not_exists(self, testdir): result = testdir.runpytest("--pyargs", "tpkgwhatv") assert result.ret - result.stderr.fnmatch_lines(["ERROR*file*or*package*not*found*"]) + result.stderr.fnmatch_lines(["ERROR*module*or*package*not*found*"]) @pytest.mark.xfail(reason="decide: feature or bug") def test_noclass_discovery_if_not_testcase(self, testdir): diff --git a/testing/test_collection.py b/testing/test_collection.py index 283df082f..01dbcc731 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -443,25 +443,6 @@ class TestCustomConftests: class TestSession: - def test_parsearg(self, testdir) -> None: - p = testdir.makepyfile("def test_func(): pass") - subdir = testdir.mkdir("sub") - subdir.ensure("__init__.py") - target = subdir.join(p.basename) - p.move(target) - subdir.chdir() - config = testdir.parseconfig(p.basename) - rcol = Session.from_config(config) - assert rcol.fspath == subdir - fspath, parts = rcol._parsearg(p.basename) - - assert fspath == target - assert len(parts) == 0 - fspath, parts = rcol._parsearg(p.basename + "::test_func") - assert fspath == target - assert parts[0] == "test_func" - assert len(parts) == 1 - def test_collect_topdir(self, testdir): p = testdir.makepyfile("def test_func(): pass") id = "::".join([p.basename, "test_func"]) @@ -1426,42 +1407,3 @@ class TestImportModeImportlib: "* 1 failed in *", ] ) - - -def test_module_full_path_without_drive(testdir): - """Collect and run test using full path except for the drive letter (#7628) - - Passing a full path without a drive letter would trigger a bug in py.path.local - where it would keep the full path without the drive letter around, instead of resolving - to the full path, resulting in fixtures node ids not matching against test node ids correctly. - """ - testdir.makepyfile( - **{ - "project/conftest.py": """ - import pytest - @pytest.fixture - def fix(): return 1 - """, - } - ) - - testdir.makepyfile( - **{ - "project/tests/dummy_test.py": """ - def test(fix): - assert fix == 1 - """ - } - ) - fn = testdir.tmpdir.join("project/tests/dummy_test.py") - assert fn.isfile() - - drive, path = os.path.splitdrive(str(fn)) - - result = testdir.runpytest(path, "-v") - result.stdout.fnmatch_lines( - [ - os.path.join("project", "tests", "dummy_test.py") + "::test PASSED *", - "* 1 passed in *", - ] - ) diff --git a/testing/test_main.py b/testing/test_main.py index ee8349a9f..4546c83ba 100644 --- a/testing/test_main.py +++ b/testing/test_main.py @@ -1,8 +1,14 @@ import argparse +import os +import re from typing import Optional +import py.path + import pytest from _pytest.config import ExitCode +from _pytest.config import UsageError +from _pytest.main import resolve_collection_argument from _pytest.main import validate_basetemp from _pytest.pytester import Testdir @@ -98,3 +104,138 @@ def test_validate_basetemp_fails(tmp_path, basetemp, monkeypatch): def test_validate_basetemp_integration(testdir): result = testdir.runpytest("--basetemp=.") result.stderr.fnmatch_lines("*basetemp must not be*") + + +class TestResolveCollectionArgument: + @pytest.fixture + def root(self, testdir): + testdir.syspathinsert(str(testdir.tmpdir / "src")) + testdir.chdir() + + pkg = testdir.tmpdir.join("src/pkg").ensure_dir() + pkg.join("__init__.py").ensure(file=True) + pkg.join("test.py").ensure(file=True) + return testdir.tmpdir + + def test_file(self, root): + """File and parts.""" + assert resolve_collection_argument(root, "src/pkg/test.py") == ( + root / "src/pkg/test.py", + [], + ) + assert resolve_collection_argument(root, "src/pkg/test.py::") == ( + root / "src/pkg/test.py", + [""], + ) + assert resolve_collection_argument(root, "src/pkg/test.py::foo::bar") == ( + root / "src/pkg/test.py", + ["foo", "bar"], + ) + assert resolve_collection_argument(root, "src/pkg/test.py::foo::bar::") == ( + root / "src/pkg/test.py", + ["foo", "bar", ""], + ) + + def test_dir(self, root): + """Directory and parts.""" + assert resolve_collection_argument(root, "src/pkg") == (root / "src/pkg", []) + assert resolve_collection_argument(root, "src/pkg::") == ( + root / "src/pkg", + [""], + ) + assert resolve_collection_argument(root, "src/pkg::foo::bar") == ( + root / "src/pkg", + ["foo", "bar"], + ) + assert resolve_collection_argument(root, "src/pkg::foo::bar::") == ( + root / "src/pkg", + ["foo", "bar", ""], + ) + + def test_pypath(self, root): + """Dotted name and parts.""" + assert resolve_collection_argument(root, "pkg.test", as_pypath=True) == ( + root / "src/pkg/test.py", + [], + ) + assert resolve_collection_argument( + root, "pkg.test::foo::bar", as_pypath=True + ) == (root / "src/pkg/test.py", ["foo", "bar"],) + assert resolve_collection_argument(root, "pkg", as_pypath=True) == ( + root / "src/pkg", + [], + ) + assert resolve_collection_argument(root, "pkg::foo::bar", as_pypath=True) == ( + root / "src/pkg", + ["foo", "bar"], + ) + + def test_does_not_exist(self, root): + """Given a file/module that does not exist raises UsageError.""" + with pytest.raises( + UsageError, match=re.escape("file or directory not found: foobar") + ): + resolve_collection_argument(root, "foobar") + + with pytest.raises( + UsageError, + match=re.escape( + "module or package not found: foobar (missing __init__.py?)" + ), + ): + resolve_collection_argument(root, "foobar", as_pypath=True) + + def test_absolute_paths_are_resolved_correctly(self, root): + """Absolute paths resolve back to absolute paths.""" + full_path = str(root / "src") + assert resolve_collection_argument(root, full_path) == ( + py.path.local(os.path.abspath("src")), + [], + ) + + # ensure full paths given in the command-line without the drive letter resolve + # to the full path correctly (#7628) + drive, full_path_without_drive = os.path.splitdrive(full_path) + assert resolve_collection_argument(root, full_path_without_drive) == ( + py.path.local(os.path.abspath("src")), + [], + ) + + +def test_module_full_path_without_drive(testdir): + """Collect and run test using full path except for the drive letter (#7628). + + Passing a full path without a drive letter would trigger a bug in py.path.local + where it would keep the full path without the drive letter around, instead of resolving + to the full path, resulting in fixtures node ids not matching against test node ids correctly. + """ + testdir.makepyfile( + **{ + "project/conftest.py": """ + import pytest + @pytest.fixture + def fix(): return 1 + """, + } + ) + + testdir.makepyfile( + **{ + "project/tests/dummy_test.py": """ + def test(fix): + assert fix == 1 + """ + } + ) + fn = testdir.tmpdir.join("project/tests/dummy_test.py") + assert fn.isfile() + + drive, path = os.path.splitdrive(str(fn)) + + result = testdir.runpytest(path, "-v") + result.stdout.fnmatch_lines( + [ + os.path.join("project", "tests", "dummy_test.py") + "::test PASSED *", + "* 1 passed in *", + ] + ) diff --git a/testing/test_terminal.py b/testing/test_terminal.py index 36604ece7..0e26ae13c 100644 --- a/testing/test_terminal.py +++ b/testing/test_terminal.py @@ -442,7 +442,9 @@ class TestCollectonly: have the items attribute.""" result = testdir.runpytest("--collect-only", "uhm_missing_path") assert result.ret == 4 - result.stderr.fnmatch_lines(["*ERROR: file not found*"]) + result.stderr.fnmatch_lines( + ["*ERROR: file or directory not found: uhm_missing_path"] + ) def test_collectonly_quiet(self, testdir): testdir.makepyfile("def test_foo(): pass")