From 676f38d04aa7fc96ecf06235d855820aba05b83a Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Tue, 16 Jan 2024 20:56:32 +0200 Subject: [PATCH] findpaths: rely on invocation_dir instead of cwd We should aim to remove all `cwd()` calls except one, otherwise things will go bad if the working directory changes. Use the invocation dir instead. --- src/_pytest/config/__init__.py | 4 +- src/_pytest/config/findpaths.py | 32 +++++----- testing/test_config.py | 105 ++++++++++++++++++++++++++------ testing/test_findpaths.py | 13 ++-- 4 files changed, 111 insertions(+), 43 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 70fceef7b..e4e8f700a 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -1179,8 +1179,8 @@ class Config: args, namespace=copy.copy(self.option) ) rootpath, inipath, inicfg = determine_setup( - ns.inifilename, - ns.file_or_dir + unknown_args, + inifile=ns.inifilename, + args=ns.file_or_dir + unknown_args, rootdir_cmd_arg=ns.rootdir or None, invocation_dir=self.invocation_params.dir, ) diff --git a/src/_pytest/config/findpaths.py b/src/_pytest/config/findpaths.py index fc30533b6..8e69a46fd 100644 --- a/src/_pytest/config/findpaths.py +++ b/src/_pytest/config/findpaths.py @@ -87,6 +87,7 @@ def load_config_dict_from_file( def locate_config( + invocation_dir: Path, args: Iterable[Path], ) -> Tuple[Optional[Path], Optional[Path], Dict[str, Union[str, List[str]]]]: """Search in the list of arguments for a valid ini-file for pytest, @@ -100,7 +101,7 @@ def locate_config( ] args = [x for x in args if not str(x).startswith("-")] if not args: - args = [Path.cwd()] + args = [invocation_dir] for arg in args: argpath = absolutepath(arg) for base in (argpath, *argpath.parents): @@ -113,7 +114,10 @@ def locate_config( return None, None, {} -def get_common_ancestor(paths: Iterable[Path]) -> Path: +def get_common_ancestor( + invocation_dir: Path, + paths: Iterable[Path], +) -> Path: common_ancestor: Optional[Path] = None for path in paths: if not path.exists(): @@ -130,7 +134,7 @@ def get_common_ancestor(paths: Iterable[Path]) -> Path: if shared is not None: common_ancestor = shared if common_ancestor is None: - common_ancestor = Path.cwd() + common_ancestor = invocation_dir elif common_ancestor.is_file(): common_ancestor = common_ancestor.parent return common_ancestor @@ -162,10 +166,11 @@ CFG_PYTEST_SECTION = "[pytest] section in {filename} files is no longer supporte def determine_setup( + *, inifile: Optional[str], args: Sequence[str], - rootdir_cmd_arg: Optional[str] = None, - invocation_dir: Optional[Path] = None, + rootdir_cmd_arg: Optional[str], + invocation_dir: Path, ) -> Tuple[Path, Optional[Path], Dict[str, Union[str, List[str]]]]: """Determine the rootdir, inifile and ini configuration values from the command line arguments. @@ -177,8 +182,7 @@ def determine_setup( :param rootdir_cmd_arg: The `--rootdir` command line argument, if given. :param invocation_dir: - The working directory when pytest was invoked, if known. - If not known, the current working directory is used. + The working directory when pytest was invoked. """ rootdir = None dirs = get_dirs_from_args(args) @@ -189,8 +193,8 @@ def determine_setup( if rootdir_cmd_arg is None: rootdir = inipath_.parent else: - ancestor = get_common_ancestor(dirs) - rootdir, inipath, inicfg = locate_config([ancestor]) + ancestor = get_common_ancestor(invocation_dir, dirs) + rootdir, inipath, inicfg = locate_config(invocation_dir, [ancestor]) if rootdir is None and rootdir_cmd_arg is None: for possible_rootdir in (ancestor, *ancestor.parents): if (possible_rootdir / "setup.py").is_file(): @@ -198,13 +202,11 @@ def determine_setup( break else: if dirs != [ancestor]: - rootdir, inipath, inicfg = locate_config(dirs) + rootdir, inipath, inicfg = locate_config(invocation_dir, dirs) if rootdir is None: - if invocation_dir is not None: - cwd = invocation_dir - else: - cwd = Path.cwd() - rootdir = get_common_ancestor([cwd, ancestor]) + rootdir = get_common_ancestor( + invocation_dir, [invocation_dir, ancestor] + ) if is_fs_root(rootdir): rootdir = ancestor if rootdir_cmd_arg: diff --git a/testing/test_config.py b/testing/test_config.py index 0f586c043..1eb530ace 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -59,7 +59,7 @@ class TestParseIni: ), encoding="utf-8", ) - _, _, cfg = locate_config([sub]) + _, _, cfg = locate_config(Path.cwd(), [sub]) assert cfg["name"] == "value" config = pytester.parseconfigure(str(sub)) assert config.inicfg["name"] == "value" @@ -1436,16 +1436,16 @@ def test_collect_pytest_prefix_bug(pytestconfig): class TestRootdir: def test_simple_noini(self, tmp_path: Path, monkeypatch: MonkeyPatch) -> None: - assert get_common_ancestor([tmp_path]) == tmp_path + assert get_common_ancestor(Path.cwd(), [tmp_path]) == tmp_path a = tmp_path / "a" a.mkdir() - assert get_common_ancestor([a, tmp_path]) == tmp_path - assert get_common_ancestor([tmp_path, a]) == tmp_path + assert get_common_ancestor(Path.cwd(), [a, tmp_path]) == tmp_path + assert get_common_ancestor(Path.cwd(), [tmp_path, a]) == tmp_path monkeypatch.chdir(tmp_path) - assert get_common_ancestor([]) == tmp_path + assert get_common_ancestor(Path.cwd(), []) == tmp_path no_path = tmp_path / "does-not-exist" - assert get_common_ancestor([no_path]) == tmp_path - assert get_common_ancestor([no_path / "a"]) == tmp_path + assert get_common_ancestor(Path.cwd(), [no_path]) == tmp_path + assert get_common_ancestor(Path.cwd(), [no_path / "a"]) == tmp_path @pytest.mark.parametrize( "name, contents", @@ -1467,10 +1467,20 @@ class TestRootdir: b = a / "b" b.mkdir() for args in ([str(tmp_path)], [str(a)], [str(b)]): - rootpath, parsed_inipath, _ = determine_setup(None, args) + rootpath, parsed_inipath, _ = determine_setup( + inifile=None, + args=args, + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path assert parsed_inipath == inipath - rootpath, parsed_inipath, ini_config = determine_setup(None, [str(b), str(a)]) + rootpath, parsed_inipath, ini_config = determine_setup( + inifile=None, + args=[str(b), str(a)], + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path assert parsed_inipath == inipath assert ini_config == {"x": "10"} @@ -1482,7 +1492,12 @@ class TestRootdir: a = tmp_path / "a" a.mkdir() (a / name).touch() - rootpath, parsed_inipath, _ = determine_setup(None, [str(a)]) + rootpath, parsed_inipath, _ = determine_setup( + inifile=None, + args=[str(a)], + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path assert parsed_inipath == inipath @@ -1491,14 +1506,24 @@ class TestRootdir: a.mkdir() (a / "setup.cfg").touch() (tmp_path / "setup.py").touch() - rootpath, inipath, inicfg = determine_setup(None, [str(a)]) + rootpath, inipath, inicfg = determine_setup( + inifile=None, + args=[str(a)], + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path assert inipath is None assert inicfg == {} def test_nothing(self, tmp_path: Path, monkeypatch: MonkeyPatch) -> None: monkeypatch.chdir(tmp_path) - rootpath, inipath, inicfg = determine_setup(None, [str(tmp_path)]) + rootpath, inipath, inicfg = determine_setup( + inifile=None, + args=[str(tmp_path)], + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path assert inipath is None assert inicfg == {} @@ -1520,7 +1545,12 @@ class TestRootdir: p = tmp_path / name p.touch() p.write_text(contents, encoding="utf-8") - rootpath, inipath, ini_config = determine_setup(str(p), [str(tmp_path)]) + rootpath, inipath, ini_config = determine_setup( + inifile=str(p), + args=[str(tmp_path)], + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path assert inipath == p assert ini_config == {"x": "10"} @@ -1534,14 +1564,24 @@ class TestRootdir: monkeypatch.chdir(tmp_path) # No config file is explicitly given: rootdir is determined to be cwd. - rootpath, found_inipath, *_ = determine_setup(None, [str(tests_dir)]) + rootpath, found_inipath, *_ = determine_setup( + inifile=None, + args=[str(tests_dir)], + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path assert found_inipath is None # Config file is explicitly given: rootdir is determined to be inifile's directory. inipath = tmp_path / "pytest.ini" inipath.touch() - rootpath, found_inipath, *_ = determine_setup(str(inipath), [str(tests_dir)]) + rootpath, found_inipath, *_ = determine_setup( + inifile=str(inipath), + args=[str(tests_dir)], + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path assert found_inipath == inipath @@ -1553,7 +1593,12 @@ class TestRootdir: a.mkdir() b = tmp_path / "b" b.mkdir() - rootpath, inifile, _ = determine_setup(None, [str(a), str(b)]) + rootpath, inifile, _ = determine_setup( + inifile=None, + args=[str(a), str(b)], + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path assert inifile is None @@ -1564,7 +1609,12 @@ class TestRootdir: b.mkdir() inipath = a / "pytest.ini" inipath.touch() - rootpath, parsed_inipath, _ = determine_setup(None, [str(a), str(b)]) + rootpath, parsed_inipath, _ = determine_setup( + inifile=None, + args=[str(a), str(b)], + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == a assert inipath == parsed_inipath @@ -1573,7 +1623,12 @@ class TestRootdir: self, dirs: Sequence[str], tmp_path: Path, monkeypatch: MonkeyPatch ) -> None: monkeypatch.chdir(tmp_path) - rootpath, inipath, _ = determine_setup(None, dirs) + rootpath, inipath, _ = determine_setup( + inifile=None, + args=dirs, + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path assert inipath is None @@ -1584,7 +1639,12 @@ class TestRootdir: a.mkdir() (a / "exists").touch() monkeypatch.chdir(tmp_path) - rootpath, inipath, _ = determine_setup(None, ["a/exist"]) + rootpath, inipath, _ = determine_setup( + inifile=None, + args=["a/exist"], + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path assert inipath is None @@ -1598,7 +1658,12 @@ class TestRootdir: (tmp_path / "myproject" / "tests").mkdir() monkeypatch.chdir(tmp_path / "myproject") - rootpath, inipath, _ = determine_setup(None, ["tests/"]) + rootpath, inipath, _ = determine_setup( + inifile=None, + args=["tests/"], + rootdir_cmd_arg=None, + invocation_dir=Path.cwd(), + ) assert rootpath == tmp_path / "myproject" assert inipath == tmp_path / "myproject" / "setup.cfg" diff --git a/testing/test_findpaths.py b/testing/test_findpaths.py index 8287de603..65370a0ba 100644 --- a/testing/test_findpaths.py +++ b/testing/test_findpaths.py @@ -109,18 +109,19 @@ class TestCommonAncestor: fn2 = tmp_path / "foo" / "zaz" / "test_2.py" fn2.parent.mkdir(parents=True) fn2.touch() - assert get_common_ancestor([fn1, fn2]) == tmp_path / "foo" - assert get_common_ancestor([fn1.parent, fn2]) == tmp_path / "foo" - assert get_common_ancestor([fn1.parent, fn2.parent]) == tmp_path / "foo" - assert get_common_ancestor([fn1, fn2.parent]) == tmp_path / "foo" + cwd = Path.cwd() + assert get_common_ancestor(cwd, [fn1, fn2]) == tmp_path / "foo" + assert get_common_ancestor(cwd, [fn1.parent, fn2]) == tmp_path / "foo" + assert get_common_ancestor(cwd, [fn1.parent, fn2.parent]) == tmp_path / "foo" + assert get_common_ancestor(cwd, [fn1, fn2.parent]) == tmp_path / "foo" def test_single_dir(self, tmp_path: Path) -> None: - assert get_common_ancestor([tmp_path]) == tmp_path + assert get_common_ancestor(Path.cwd(), [tmp_path]) == tmp_path def test_single_file(self, tmp_path: Path) -> None: fn = tmp_path / "foo.py" fn.touch() - assert get_common_ancestor([fn]) == tmp_path + assert get_common_ancestor(Path.cwd(), [fn]) == tmp_path def test_get_dirs_from_args(tmp_path):