From fc61bdd9073d86819eb54c8603384a10c626a9d4 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Tue, 20 Nov 2018 13:47:40 +0100 Subject: [PATCH 1/5] fix 4425: resolve --basetemp to absolute paths --- changelog/4425.bugfix.rst | 1 + src/_pytest/monkeypatch.py | 6 +++++- src/_pytest/tmpdir.py | 6 ++++-- testing/test_tmpdir.py | 33 ++++++++++++++++++++++++++++++--- 4 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 changelog/4425.bugfix.rst diff --git a/changelog/4425.bugfix.rst b/changelog/4425.bugfix.rst new file mode 100644 index 000000000..7c869cd4c --- /dev/null +++ b/changelog/4425.bugfix.rst @@ -0,0 +1 @@ +Ensure we resolve the absolute path when the given ``--basetemp`` is a relative path. diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py index d536b7746..2c81de177 100644 --- a/src/_pytest/monkeypatch.py +++ b/src/_pytest/monkeypatch.py @@ -13,8 +13,9 @@ import six import pytest from _pytest.fixtures import fixture +from _pytest.pathlib import Path -RE_IMPORT_ERROR_NAME = re.compile("^No module named (.*)$") +RE_IMPORT_ERROR_NAME = re.compile(r"^No module named (.*)$") @fixture @@ -267,6 +268,9 @@ class MonkeyPatch(object): self._cwd = os.getcwd() if hasattr(path, "chdir"): path.chdir() + elif isinstance(path, Path): + # modern python uses the fspath protocol here LEGACY + os.chdir(str(path)) else: os.chdir(path) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 6287c1705..81430e4f0 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -26,7 +26,9 @@ class TempPathFactory(object): The base directory can be configured using the ``--basetemp`` option.""" - _given_basetemp = attr.ib() + _given_basetemp = attr.ib( + convert=attr.converters.optional(lambda p: Path(p).resolve()) + ) _trace = attr.ib() _basetemp = attr.ib(default=None) @@ -53,7 +55,7 @@ class TempPathFactory(object): """ return base temporary directory. """ if self._basetemp is None: if self._given_basetemp is not None: - basetemp = Path(self._given_basetemp) + basetemp = self._given_basetemp ensure_reset_dir(basetemp) else: from_env = os.environ.get("PYTEST_DEBUG_TEMPROOT") diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 38b0672b7..fddece2cf 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -4,6 +4,7 @@ from __future__ import print_function import sys +import attr import six import pytest @@ -25,12 +26,29 @@ def test_ensuretemp(recwarn): assert d1.check(dir=1) +@attr.s +class FakeConfig(object): + basetemp = attr.ib() + trace = attr.ib(default=None) + + @property + def trace(self): + return self + + def get(self, key): + return lambda *k: None + + @property + def option(self): + return self + + class TestTempdirHandler(object): - def test_mktemp(self, testdir): + def test_mktemp(self, tmp_path): + from _pytest.tmpdir import TempdirFactory, TempPathFactory - config = testdir.parseconfig() - config.option.basetemp = testdir.mkdir("hello") + config = FakeConfig(tmp_path) t = TempdirFactory(TempPathFactory.from_config(config)) tmp = t.mktemp("world") assert tmp.relto(t.getbasetemp()) == "world0" @@ -40,6 +58,15 @@ class TestTempdirHandler(object): assert tmp2.relto(t.getbasetemp()).startswith("this") assert tmp2 != tmp + @pytest.mark.issue(4425) + def test_tmppath_relative_basetemp_absolute(self, tmp_path, monkeypatch): + from _pytest.tmpdir import TempPathFactory + + monkeypatch.chdir(tmp_path) + config = FakeConfig("hello") + t = TempPathFactory.from_config(config) + assert t.getbasetemp() == (tmp_path / "hello") + class TestConfigTmpdir(object): def test_getbasetemp_custom_removes_old(self, testdir): From f180ab3e69438559df309e8eac563499bde4fba2 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 20 Nov 2018 20:08:01 -0200 Subject: [PATCH 2/5] Use os.path.abspath to get absolute path instead of Path.resolve() Unfortunately it seems there is a difference in resolve() behavior depending on the platform --- src/_pytest/tmpdir.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 81430e4f0..937d90c2f 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -27,7 +27,9 @@ class TempPathFactory(object): The base directory can be configured using the ``--basetemp`` option.""" _given_basetemp = attr.ib( - convert=attr.converters.optional(lambda p: Path(p).resolve()) + # using os.path.abspath() to get absolute path instead of resolve() as it + # does not work the same in all platforms + convert=attr.converters.optional(lambda p: Path(os.path.abspath(p))) ) _trace = attr.ib() _basetemp = attr.ib(default=None) From 4f5c153d29eb91a76e8f2aa519684e9710ef6624 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 21 Nov 2018 20:46:08 -0200 Subject: [PATCH 3/5] Fix call to os.path.abspath: the argument might already be a Path instance There's Path.absolute(), but it is not public, see https://bugs.python.org/issue25012. --- src/_pytest/tmpdir.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 937d90c2f..dadb196ea 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -10,6 +10,7 @@ import warnings import attr import py +import six import pytest from .pathlib import ensure_reset_dir @@ -28,8 +29,11 @@ class TempPathFactory(object): _given_basetemp = attr.ib( # using os.path.abspath() to get absolute path instead of resolve() as it - # does not work the same in all platforms - convert=attr.converters.optional(lambda p: Path(os.path.abspath(p))) + # does not work the same in all platforms; there's Path.absolute(), but it is not + # public (see https://bugs.python.org/issue25012) + convert=attr.converters.optional( + lambda p: Path(os.path.abspath(six.text_type(p))) + ) ) _trace = attr.ib() _basetemp = attr.ib(default=None) From f1fe9e41acf6e2099270a931c46cf390cdc6e348 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 21 Nov 2018 20:49:17 -0200 Subject: [PATCH 4/5] Mention PR# in the comment for future reference --- src/_pytest/tmpdir.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index dadb196ea..860c2d4af 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -29,8 +29,8 @@ class TempPathFactory(object): _given_basetemp = attr.ib( # using os.path.abspath() to get absolute path instead of resolve() as it - # does not work the same in all platforms; there's Path.absolute(), but it is not - # public (see https://bugs.python.org/issue25012) + # does not work the same in all platforms (see #4427) + # Path.absolute() exists, but it is not public (see https://bugs.python.org/issue25012) convert=attr.converters.optional( lambda p: Path(os.path.abspath(six.text_type(p))) ) From 5f1d69207260297738ff4dd96687fe049d2963a3 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 22 Nov 2018 16:10:12 +0100 Subject: [PATCH 5/5] use Path.resolve in test to sort out osx temporary folder being a symlink --- testing/test_tmpdir.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index fddece2cf..6040d9444 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -65,7 +65,7 @@ class TestTempdirHandler(object): monkeypatch.chdir(tmp_path) config = FakeConfig("hello") t = TempPathFactory.from_config(config) - assert t.getbasetemp() == (tmp_path / "hello") + assert t.getbasetemp().resolve() == (tmp_path / "hello").resolve() class TestConfigTmpdir(object):