From b48e23d54c7133e2612d6b1974dd26d994be10b9 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sat, 15 Sep 2018 22:30:44 +0200 Subject: [PATCH 01/25] port interals of tmpdir to a basic pathlib implementation this is still lacking locking and cleanup of the folders --- .pre-commit-config.yaml | 2 + src/_pytest/main.py | 5 +- src/_pytest/tmpdir.py | 145 ++++++++++++++++++++++++++++------------ testing/test_tmpdir.py | 4 +- 4 files changed, 109 insertions(+), 47 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7f48cd5d7..fc6d9e10c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -20,7 +20,9 @@ repos: - id: check-yaml - id: debug-statements exclude: _pytest/debugging.py + language_version: python3 - id: flake8 + language_version: python3 - repo: https://github.com/asottile/pyupgrade rev: v1.8.0 hooks: diff --git a/src/_pytest/main.py b/src/_pytest/main.py index ce07285a4..bf4faaf6a 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -156,7 +156,10 @@ def pytest_addoption(parser): dest="basetemp", default=None, metavar="dir", - help="base temporary directory for this test run.", + help=( + "base temporary directory for this test run." + "(warning: this directory is removed if it exists)" + ), ) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 260d28422..f1d52d497 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -6,17 +6,110 @@ import re import pytest import py from _pytest.monkeypatch import MonkeyPatch +from .compat import Path +import attr +import shutil +import tempfile +def make_numbered_dir(root, prefix): + l_prefix = prefix.lower() + + def parse_num(p, cut=len(l_prefix)): + maybe_num = p.name[cut:] + try: + return int(maybe_num) + except ValueError: + return -1 + + for i in range(10): + # try up to 10 times to create the folder + max_existing = max( + ( + parse_num(x) + for x in root.iterdir() + if x.name.lower().startswith(l_prefix) + ), + default=-1, + ) + new_number = max_existing + 1 + new_path = root.joinpath("{}{}".format(prefix, new_number)) + try: + new_path.mkdir() + except Exception: + pass + else: + return new_path + + +def make_numbered_dir_with_cleanup(root, prefix, keep, consider_lock_dead_after): + p = make_numbered_dir(root, prefix) + # todo cleanup + return p + + +@attr.s +class TempPathFactory(object): + """docstring for ClassName""" + + given_basetemp = attr.ib() + trace = attr.ib() + _basetemp = attr.ib(default=None) + + @classmethod + def from_config(cls, config): + return cls( + given_basetemp=config.option.basetemp, trace=config.trace.get("tmpdir") + ) + + def mktemp(self, basename, numbered=True): + if not numbered: + p = self.getbasetemp().joinpath(basename) + p.mkdir() + else: + p = make_numbered_dir(root=self.getbasetemp(), prefix=basename) + self.trace("mktemp", p) + return p + + def getbasetemp(self): + """ return base temporary directory. """ + if self._basetemp is None: + if self.given_basetemp: + basetemp = Path(self.given_basetemp) + if basetemp.exists(): + shutil.rmtree(str(basetemp)) + basetemp.mkdir() + else: + temproot = Path(tempfile.gettempdir()) + user = get_user() + if user: + # use a sub-directory in the temproot to speed-up + # make_numbered_dir() call + rootdir = temproot.joinpath("pytest-of-{}".format(user)) + else: + rootdir = temproot + rootdir.mkdir(exist_ok=True) + basetemp = make_numbered_dir_with_cleanup( + prefix="pytest-", + root=rootdir, + keep=None, + consider_lock_dead_after=None, + ) + self._basetemp = t = basetemp + self.trace("new basetemp", t) + return t + else: + return self._basetemp + + +@attr.s class TempdirFactory(object): """Factory for temporary directories under the common base temp directory. The base directory can be configured using the ``--basetemp`` option. """ - def __init__(self, config): - self.config = config - self.trace = config.trace.get("tmpdir") + tmppath_factory = attr.ib() def ensuretemp(self, string, dir=1): """ (deprecated) return temporary directory path with @@ -33,46 +126,13 @@ class TempdirFactory(object): If ``numbered``, ensure the directory is unique by adding a number prefix greater than any existing one. """ - basetemp = self.getbasetemp() - if not numbered: - p = basetemp.mkdir(basename) - else: - p = py.path.local.make_numbered_dir( - prefix=basename, keep=0, rootdir=basetemp, lock_timeout=None - ) - self.trace("mktemp", p) - return p + return py.path.local(self.tmppath_factory.mktemp(basename, numbered).resolve()) def getbasetemp(self): - """ return base temporary directory. """ - try: - return self._basetemp - except AttributeError: - basetemp = self.config.option.basetemp - if basetemp: - basetemp = py.path.local(basetemp) - if basetemp.check(): - basetemp.remove() - basetemp.mkdir() - else: - temproot = py.path.local.get_temproot() - user = get_user() - if user: - # use a sub-directory in the temproot to speed-up - # make_numbered_dir() call - rootdir = temproot.join("pytest-of-%s" % user) - else: - rootdir = temproot - rootdir.ensure(dir=1) - basetemp = py.path.local.make_numbered_dir( - prefix="pytest-", rootdir=rootdir - ) - self._basetemp = t = basetemp.realpath() - self.trace("new basetemp", t) - return t + return py.path.local(self.tmppath_factory.getbasetemp().resolve()) def finish(self): - self.trace("finish") + self.tmppath_factory.trace("finish") def get_user(): @@ -87,10 +147,6 @@ def get_user(): return None -# backward compatibility -TempdirHandler = TempdirFactory - - def pytest_configure(config): """Create a TempdirFactory and attach it to the config object. @@ -99,7 +155,8 @@ def pytest_configure(config): to the tmpdir_factory session fixture. """ mp = MonkeyPatch() - t = TempdirFactory(config) + tmppath_handler = TempPathFactory.from_config(config) + t = TempdirFactory(tmppath_handler) config._cleanup.extend([mp.undo, t.finish]) mp.setattr(config, "_tmpdirhandler", t, raising=False) mp.setattr(pytest, "ensuretemp", t.ensuretemp, raising=False) diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 487f9b21e..2acfec5c0 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -19,11 +19,11 @@ def test_ensuretemp(recwarn): class TestTempdirHandler(object): def test_mktemp(self, testdir): - from _pytest.tmpdir import TempdirFactory + from _pytest.tmpdir import TempdirFactory, TempPathFactory config = testdir.parseconfig() config.option.basetemp = testdir.mkdir("hello") - t = TempdirFactory(config) + t = TempdirFactory(TempPathFactory.from_config(config)) tmp = t.mktemp("world") assert tmp.relto(t.getbasetemp()) == "world0" tmp = t.mktemp("this") From 2e39fd89d1f54d924c10094659254e7c66096e0a Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sun, 16 Sep 2018 14:34:50 +0200 Subject: [PATCH 02/25] add python27 support by using reduce instead of max --- src/_pytest/tmpdir.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index f1d52d497..bf4e9e6b0 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, division, print_function import re +from six.moves import reduce import pytest import py @@ -24,13 +25,14 @@ def make_numbered_dir(root, prefix): for i in range(10): # try up to 10 times to create the folder - max_existing = max( + max_existing = reduce( + max, ( parse_num(x) for x in root.iterdir() if x.name.lower().startswith(l_prefix) ), - default=-1, + -1, ) new_number = max_existing + 1 new_path = root.joinpath("{}{}".format(prefix, new_number)) From d053cdfbbb5a9da7a726d0f564c1ff7139e5e70c Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Tue, 18 Sep 2018 22:43:48 +0200 Subject: [PATCH 03/25] factor out max and iterate on locks and cleanups --- src/_pytest/tmpdir.py | 92 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 74 insertions(+), 18 deletions(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index bf4e9e6b0..45a5e2d1b 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -2,8 +2,13 @@ from __future__ import absolute_import, division, print_function import re -from six.moves import reduce +import os +import atexit +import six +from functools import reduce + +from six.moves import map import pytest import py from _pytest.monkeypatch import MonkeyPatch @@ -13,10 +18,21 @@ import shutil import tempfile -def make_numbered_dir(root, prefix): +def find_prefixed(root, prefix): l_prefix = prefix.lower() + for x in root.iterdir(): + if x.name.lower().startswith(l_prefix): + yield x - def parse_num(p, cut=len(l_prefix)): + +def _max(iterable, default): + # needed due to python2.7 lacking the default argument for max + return reduce(max, iterable, default) + + +def make_numbered_dir(root, prefix): + + def parse_num(p, cut=len(prefix)): maybe_num = p.name[cut:] try: return int(maybe_num) @@ -25,15 +41,7 @@ def make_numbered_dir(root, prefix): for i in range(10): # try up to 10 times to create the folder - max_existing = reduce( - max, - ( - parse_num(x) - for x in root.iterdir() - if x.name.lower().startswith(l_prefix) - ), - -1, - ) + max_existing = _max(map(parse_num, find_prefixed(root, prefix)), -1) new_number = max_existing + 1 new_path = root.joinpath("{}{}".format(prefix, new_number)) try: @@ -42,13 +50,60 @@ def make_numbered_dir(root, prefix): pass else: return new_path + else: + raise EnvironmentError( + "could not create numbered dir with prefix {prefix} in {root})".format( + prefix=prefix, root=root)) + + +def create_cleanup_lock(p): + lock_path = p.joinpath('.lock') + fd = os.open(str(lock_path), os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) + pid = os.getpid() + spid = str(pid) + if not isinstance(spid, six.binary_type): + spid = spid.encode("ascii") + os.write(fd, spid) + os.close(fd) + if not lock_path.is_file(): + raise EnvironmentError("lock path got renamed after sucessfull creation") + return lock_path + + +def register_cleanup_lock_removal(lock_path): + pid = os.getpid() + + def cleanup_on_exit(lock_path=lock_path, original_pid=pid): + current_pid = os.getpid() + if current_pid != original_pid: + # fork + return + try: + lock_path.unlink() + except (OSError, IOError): + pass + return atexit.register(cleanup_on_exit) + + +def cleanup_numbered_dir(root, prefix, keep): + # todo + pass def make_numbered_dir_with_cleanup(root, prefix, keep, consider_lock_dead_after): - p = make_numbered_dir(root, prefix) - # todo cleanup - return p + for i in range(10): + try: + p = make_numbered_dir(root, prefix) + lock_path = create_cleanup_lock(p) + register_cleanup_lock_removal(lock_path) + except Exception as e: + raise + else: + cleanup_numbered_dir(root=root, prefix=prefix, keep=keep) + return p + else: + raise e @attr.s class TempPathFactory(object): @@ -76,7 +131,7 @@ class TempPathFactory(object): def getbasetemp(self): """ return base temporary directory. """ if self._basetemp is None: - if self.given_basetemp: + if self.given_basetemp is not None: basetemp = Path(self.given_basetemp) if basetemp.exists(): shutil.rmtree(str(basetemp)) @@ -94,9 +149,10 @@ class TempPathFactory(object): basetemp = make_numbered_dir_with_cleanup( prefix="pytest-", root=rootdir, - keep=None, - consider_lock_dead_after=None, + keep=3, + consider_lock_dead_after=10000, ) + assert basetemp is not None self._basetemp = t = basetemp self.trace("new basetemp", t) return t From 8e00280fc1d50bc2fb74cc1ea128e938bb29a691 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 19 Sep 2018 09:07:02 +0200 Subject: [PATCH 04/25] fix linting --- src/_pytest/tmpdir.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 45a5e2d1b..eea2a3c38 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -31,7 +31,6 @@ def _max(iterable, default): def make_numbered_dir(root, prefix): - def parse_num(p, cut=len(prefix)): maybe_num = p.name[cut:] try: @@ -53,11 +52,13 @@ def make_numbered_dir(root, prefix): else: raise EnvironmentError( "could not create numbered dir with prefix {prefix} in {root})".format( - prefix=prefix, root=root)) + prefix=prefix, root=root + ) + ) def create_cleanup_lock(p): - lock_path = p.joinpath('.lock') + lock_path = p.joinpath(".lock") fd = os.open(str(lock_path), os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) pid = os.getpid() spid = str(pid) @@ -82,6 +83,7 @@ def register_cleanup_lock_removal(lock_path): lock_path.unlink() except (OSError, IOError): pass + return atexit.register(cleanup_on_exit) @@ -96,14 +98,12 @@ def make_numbered_dir_with_cleanup(root, prefix, keep, consider_lock_dead_after) p = make_numbered_dir(root, prefix) lock_path = create_cleanup_lock(p) register_cleanup_lock_removal(lock_path) - except Exception as e: + except Exception: raise else: cleanup_numbered_dir(root=root, prefix=prefix, keep=keep) return p - else: - raise e @attr.s class TempPathFactory(object): From 66a690928cc847c48774906e5ea60f578ddb216c Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 19 Sep 2018 18:17:10 +0200 Subject: [PATCH 05/25] bring in purepath and fix an assertion --- src/pytest.py | 3 +++ testing/test_tmpdir.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pytest.py b/src/pytest.py index e173fd3d4..9ed7536d1 100644 --- a/src/pytest.py +++ b/src/pytest.py @@ -26,6 +26,7 @@ from _pytest.warning_types import ( RemovedInPytest4Warning, PytestExperimentalApiWarning, ) +from _pytest.compat import Path, PurePath set_trace = __pytestPDB.set_trace @@ -67,6 +68,8 @@ __all__ = [ "warns", "xfail", "yield_fixture", + "Path", + "PurePath", ] if __name__ == "__main__": diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 2acfec5c0..0ea47aef4 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -111,7 +111,7 @@ def test_tmpdir_factory(testdir): def session_dir(tmpdir_factory): return tmpdir_factory.mktemp('data', numbered=False) def test_some(session_dir): - session_dir.isdir() + assert session_dir.isdir() """ ) reprec = testdir.inline_run() From ab3637d486cf48f46aaaa3627c3314ec7099ab06 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 20 Sep 2018 16:10:33 +0200 Subject: [PATCH 06/25] implement cleanup for unlocked folders --- src/_pytest/tmpdir.py | 109 +++++++++++++++++++++++++++++++---------- testing/test_tmpdir.py | 57 +++++++++++++++++++++ 2 files changed, 139 insertions(+), 27 deletions(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index eea2a3c38..276085fdc 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -3,11 +3,12 @@ from __future__ import absolute_import, division, print_function import re import os +import errno import atexit - +import operator import six from functools import reduce - +import uuid from six.moves import map import pytest import py @@ -16,6 +17,10 @@ from .compat import Path import attr import shutil import tempfile +import itertools + + +get_lock_path = operator.methodcaller("joinpath", ".lock") def find_prefixed(root, prefix): @@ -25,22 +30,32 @@ def find_prefixed(root, prefix): yield x +def extract_suffixees(iter, prefix): + p_len = len(prefix) + for p in iter: + yield p.name[p_len:] + + +def find_suffixes(root, prefix): + return extract_suffixees(find_prefixed(root, prefix), prefix) + + +def parse_num(maybe_num): + try: + return int(maybe_num) + except ValueError: + return -1 + + def _max(iterable, default): # needed due to python2.7 lacking the default argument for max return reduce(max, iterable, default) def make_numbered_dir(root, prefix): - def parse_num(p, cut=len(prefix)): - maybe_num = p.name[cut:] - try: - return int(maybe_num) - except ValueError: - return -1 - for i in range(10): # try up to 10 times to create the folder - max_existing = _max(map(parse_num, find_prefixed(root, prefix)), -1) + max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1) new_number = max_existing + 1 new_path = root.joinpath("{}{}".format(prefix, new_number)) try: @@ -58,20 +73,29 @@ def make_numbered_dir(root, prefix): def create_cleanup_lock(p): - lock_path = p.joinpath(".lock") - fd = os.open(str(lock_path), os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) - pid = os.getpid() - spid = str(pid) - if not isinstance(spid, six.binary_type): - spid = spid.encode("ascii") - os.write(fd, spid) - os.close(fd) - if not lock_path.is_file(): - raise EnvironmentError("lock path got renamed after sucessfull creation") - return lock_path + lock_path = get_lock_path(p) + try: + fd = os.open(str(lock_path), os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) + except OSError as e: + if e.errno == errno.EEXIST: + six.raise_from( + EnvironmentError("cannot create lockfile in {path}".format(path=p)), e + ) + else: + raise + else: + pid = os.getpid() + spid = str(pid) + if not isinstance(spid, six.binary_type): + spid = spid.encode("ascii") + os.write(fd, spid) + os.close(fd) + if not lock_path.is_file(): + raise EnvironmentError("lock path got renamed after sucessfull creation") + return lock_path -def register_cleanup_lock_removal(lock_path): +def register_cleanup_lock_removal(lock_path, register=atexit.register): pid = os.getpid() def cleanup_on_exit(lock_path=lock_path, original_pid=pid): @@ -84,12 +108,33 @@ def register_cleanup_lock_removal(lock_path): except (OSError, IOError): pass - return atexit.register(cleanup_on_exit) + return register(cleanup_on_exit) -def cleanup_numbered_dir(root, prefix, keep): - # todo - pass +def delete_a_numbered_dir(path): + create_cleanup_lock(path) + parent = path.parent + + garbage = parent.joinpath("garbage-{}".format(uuid.uuid4())) + path.rename(garbage) + shutil.rmtree(str(garbage)) + + +def is_deletable(path, consider_lock_dead_after): + lock = get_lock_path(path) + if not lock.exists(): + return True + + +def cleanup_numbered_dir(root, prefix, keep, consider_lock_dead_after): + max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1) + max_delete = max_existing - keep + paths = find_prefixed(root, prefix) + paths, paths2 = itertools.tee(paths) + numbers = map(parse_num, extract_suffixees(paths2, prefix)) + for path, number in zip(paths, numbers): + if number <= max_delete and is_deletable(path, consider_lock_dead_after): + delete_a_numbered_dir(path) def make_numbered_dir_with_cleanup(root, prefix, keep, consider_lock_dead_after): @@ -101,7 +146,12 @@ def make_numbered_dir_with_cleanup(root, prefix, keep, consider_lock_dead_after) except Exception: raise else: - cleanup_numbered_dir(root=root, prefix=prefix, keep=keep) + cleanup_numbered_dir( + root=root, + prefix=prefix, + keep=keep, + consider_lock_dead_after=consider_lock_dead_after, + ) return p @@ -244,3 +294,8 @@ def tmpdir(request, tmpdir_factory): name = name[:MAXVAL] x = tmpdir_factory.mktemp(name, numbered=True) return x + + +@pytest.fixture +def tmp_path(tmpdir): + return Path(tmpdir) diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 0ea47aef4..5b460e628 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -184,3 +184,60 @@ def test_get_user(monkeypatch): monkeypatch.delenv("USER", raising=False) monkeypatch.delenv("USERNAME", raising=False) assert get_user() is None + + +class TestNumberedDir(object): + PREFIX = "fun-" + + def test_make(self, tmp_path): + from _pytest.tmpdir import make_numbered_dir + + for i in range(10): + d = make_numbered_dir(root=tmp_path, prefix=self.PREFIX) + assert d.name.startswith(self.PREFIX) + assert d.name.endswith(str(i)) + + def test_cleanup_lock_create(self, tmp_path): + d = tmp_path.joinpath("test") + d.mkdir() + from _pytest.tmpdir import create_cleanup_lock + + lockfile = create_cleanup_lock(d) + with pytest.raises(EnvironmentError, match="cannot create lockfile in .*"): + create_cleanup_lock(d) + + lockfile.unlink() + + def test_lock_register_cleanup_removal(self, tmp_path): + from _pytest.tmpdir import create_cleanup_lock, register_cleanup_lock_removal + + lock = create_cleanup_lock(tmp_path) + + registry = [] + register_cleanup_lock_removal(lock, register=registry.append) + + cleanup_func, = registry + + assert lock.is_file() + + cleanup_func(original_pid="intentionally_different") + + assert lock.is_file() + + cleanup_func() + + assert not lock.exists() + + cleanup_func() + + assert not lock.exists() + + def test_cleanup_keep(self, tmp_path): + self.test_make(tmp_path) + from _pytest.tmpdir import cleanup_numbered_dir + + cleanup_numbered_dir( + root=tmp_path, prefix=self.PREFIX, keep=2, consider_lock_dead_after=0 + ) + a, b = tmp_path.iterdir() + print(a, b) From 8b4a29357e24cab087202135a92ef603439c194d Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 21 Sep 2018 15:28:49 +0200 Subject: [PATCH 07/25] fix typo --- src/_pytest/tmpdir.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 276085fdc..894a6b70e 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -30,14 +30,14 @@ def find_prefixed(root, prefix): yield x -def extract_suffixees(iter, prefix): +def extract_suffixes(iter, prefix): p_len = len(prefix) for p in iter: yield p.name[p_len:] def find_suffixes(root, prefix): - return extract_suffixees(find_prefixed(root, prefix), prefix) + return extract_suffixes(find_prefixed(root, prefix), prefix) def parse_num(maybe_num): @@ -111,7 +111,7 @@ def register_cleanup_lock_removal(lock_path, register=atexit.register): return register(cleanup_on_exit) -def delete_a_numbered_dir(path): +def delete_a_numbered_dir(path, consider_lock_dead_after): create_cleanup_lock(path) parent = path.parent @@ -131,10 +131,10 @@ def cleanup_numbered_dir(root, prefix, keep, consider_lock_dead_after): max_delete = max_existing - keep paths = find_prefixed(root, prefix) paths, paths2 = itertools.tee(paths) - numbers = map(parse_num, extract_suffixees(paths2, prefix)) + numbers = map(parse_num, extract_suffixes(paths2, prefix)) for path, number in zip(paths, numbers): if number <= max_delete and is_deletable(path, consider_lock_dead_after): - delete_a_numbered_dir(path) + delete_a_numbered_dir(path, consider_lock_dead_after) def make_numbered_dir_with_cleanup(root, prefix, keep, consider_lock_dead_after): From b3a5b0ebe1fa759cea7dcdd82d17020648234c61 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 26 Sep 2018 11:36:03 +0200 Subject: [PATCH 08/25] remove path from exposure --- src/pytest.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/pytest.py b/src/pytest.py index 9ed7536d1..e173fd3d4 100644 --- a/src/pytest.py +++ b/src/pytest.py @@ -26,7 +26,6 @@ from _pytest.warning_types import ( RemovedInPytest4Warning, PytestExperimentalApiWarning, ) -from _pytest.compat import Path, PurePath set_trace = __pytestPDB.set_trace @@ -68,8 +67,6 @@ __all__ = [ "warns", "xfail", "yield_fixture", - "Path", - "PurePath", ] if __name__ == "__main__": From 642cd86dd1723b4ed997a99f3e089eeefdf8af08 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 26 Sep 2018 20:41:33 +0200 Subject: [PATCH 09/25] shape up removal and lock destruction --- src/_pytest/tmpdir.py | 52 ++++++++++++++++++++++++++++-------------- testing/test_tmpdir.py | 12 ++++++++++ 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 894a6b70e..a843b55c5 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -111,7 +111,7 @@ def register_cleanup_lock_removal(lock_path, register=atexit.register): return register(cleanup_on_exit) -def delete_a_numbered_dir(path, consider_lock_dead_after): +def delete_a_numbered_dir(path): create_cleanup_lock(path) parent = path.parent @@ -120,24 +120,47 @@ def delete_a_numbered_dir(path, consider_lock_dead_after): shutil.rmtree(str(garbage)) -def is_deletable(path, consider_lock_dead_after): +def ensure_deletable(path, consider_lock_dead_after): lock = get_lock_path(path) if not lock.exists(): return True + try: + lock_time = lock.stat().st_mtime + except Exception: + return False + else: + if lock_time > consider_lock_dead_after: + lock.unlink() + return True + else: + return False -def cleanup_numbered_dir(root, prefix, keep, consider_lock_dead_after): +def try_cleanup(path, consider_lock_dead_after): + if ensure_deletable(path, consider_lock_dead_after): + delete_a_numbered_dir(path) + + +def cleanup_candidates(root, prefix, keep): max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1) max_delete = max_existing - keep paths = find_prefixed(root, prefix) paths, paths2 = itertools.tee(paths) numbers = map(parse_num, extract_suffixes(paths2, prefix)) for path, number in zip(paths, numbers): - if number <= max_delete and is_deletable(path, consider_lock_dead_after): - delete_a_numbered_dir(path, consider_lock_dead_after) + if number <= max_delete: + yield path -def make_numbered_dir_with_cleanup(root, prefix, keep, consider_lock_dead_after): +def cleanup_numbered_dir(root, prefix, keep, consider_lock_dead_after): + for path in cleanup_candidates(root, prefix, keep): + try_cleanup(path, consider_lock_dead_after) + known_garbage = list(root.glob("garbage-*")) + for path in known_garbage: + try_cleanup(path, consider_lock_dead_after) + + +def make_numbered_dir_with_cleanup(root, prefix, keep, lock_timeout): for i in range(10): try: p = make_numbered_dir(root, prefix) @@ -146,6 +169,7 @@ def make_numbered_dir_with_cleanup(root, prefix, keep, consider_lock_dead_after) except Exception: raise else: + consider_lock_dead_after = p.stat().st_mtime + lock_timeout cleanup_numbered_dir( root=root, prefix=prefix, @@ -188,19 +212,13 @@ class TempPathFactory(object): basetemp.mkdir() else: temproot = Path(tempfile.gettempdir()) - user = get_user() - if user: - # use a sub-directory in the temproot to speed-up - # make_numbered_dir() call - rootdir = temproot.joinpath("pytest-of-{}".format(user)) - else: - rootdir = temproot + user = get_user() or "unknown" + # use a sub-directory in the temproot to speed-up + # make_numbered_dir() call + rootdir = temproot.joinpath("pytest-of-{}".format(user)) rootdir.mkdir(exist_ok=True) basetemp = make_numbered_dir_with_cleanup( - prefix="pytest-", - root=rootdir, - keep=3, - consider_lock_dead_after=10000, + prefix="pytest-", root=rootdir, keep=3, lock_timeout=10000 ) assert basetemp is not None self._basetemp = t = basetemp diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 5b460e628..360614673 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -143,6 +143,7 @@ def break_getuser(monkeypatch): monkeypatch.delenv(envvar, raising=False) +@pytest.mark.skip(reason="creates random tmpdirs as part of a system level test") @pytest.mark.usefixtures("break_getuser") @pytest.mark.skipif(sys.platform.startswith("win"), reason="no os.getuid on windows") def test_tmpdir_fallback_uid_not_found(testdir): @@ -161,6 +162,7 @@ def test_tmpdir_fallback_uid_not_found(testdir): reprec.assertoutcome(passed=1) +@pytest.mark.skip(reason="creates random tmpdirs as part of a system level test") @pytest.mark.usefixtures("break_getuser") @pytest.mark.skipif(sys.platform.startswith("win"), reason="no os.getuid on windows") def test_get_user_uid_not_found(): @@ -241,3 +243,13 @@ class TestNumberedDir(object): ) a, b = tmp_path.iterdir() print(a, b) + + def test_cleanup_locked(self, tmp_path): + + from _pytest import tmpdir + + p = tmpdir.make_numbered_dir(root=tmp_path, prefix=self.PREFIX) + + tmpdir.create_cleanup_lock(p) + assert not tmpdir.ensure_deletable(p, p.stat().st_mtime + 1) + assert tmpdir.ensure_deletable(p, p.stat().st_mtime - 1) From 2532dc1dbbdf49f2c32638d3caa19c4a7579a9b8 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 27 Sep 2018 11:23:21 +0200 Subject: [PATCH 10/25] fix up lock consideration argument --- src/_pytest/tmpdir.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index a843b55c5..c6a079f4e 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -120,7 +120,7 @@ def delete_a_numbered_dir(path): shutil.rmtree(str(garbage)) -def ensure_deletable(path, consider_lock_dead_after): +def ensure_deletable(path, consider_lock_dead_if_created_before): lock = get_lock_path(path) if not lock.exists(): return True @@ -129,15 +129,15 @@ def ensure_deletable(path, consider_lock_dead_after): except Exception: return False else: - if lock_time > consider_lock_dead_after: + if lock_time < consider_lock_dead_if_created_before: lock.unlink() return True else: return False -def try_cleanup(path, consider_lock_dead_after): - if ensure_deletable(path, consider_lock_dead_after): +def try_cleanup(path, consider_lock_dead_if_created_before): + if ensure_deletable(path, consider_lock_dead_if_created_before): delete_a_numbered_dir(path) @@ -152,12 +152,12 @@ def cleanup_candidates(root, prefix, keep): yield path -def cleanup_numbered_dir(root, prefix, keep, consider_lock_dead_after): +def cleanup_numbered_dir(root, prefix, keep, consider_lock_dead_if_created_before): for path in cleanup_candidates(root, prefix, keep): - try_cleanup(path, consider_lock_dead_after) + try_cleanup(path, consider_lock_dead_if_created_before) known_garbage = list(root.glob("garbage-*")) for path in known_garbage: - try_cleanup(path, consider_lock_dead_after) + try_cleanup(path, consider_lock_dead_if_created_before) def make_numbered_dir_with_cleanup(root, prefix, keep, lock_timeout): @@ -169,12 +169,12 @@ def make_numbered_dir_with_cleanup(root, prefix, keep, lock_timeout): except Exception: raise else: - consider_lock_dead_after = p.stat().st_mtime + lock_timeout + consider_lock_dead_if_created_before = p.stat().st_mtime + lock_timeout cleanup_numbered_dir( root=root, prefix=prefix, keep=keep, - consider_lock_dead_after=consider_lock_dead_after, + consider_lock_dead_if_created_before=consider_lock_dead_if_created_before, ) return p From d76fa59b35edfffa91ce24e19c9f17f75f510546 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 28 Sep 2018 22:06:43 +0200 Subject: [PATCH 11/25] fix lock timeouts for good this time --- src/_pytest/tmpdir.py | 14 +++++++++----- testing/test_tmpdir.py | 14 +++++++++++--- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index c6a079f4e..b9bd12afc 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -20,6 +20,8 @@ import tempfile import itertools +LOCK_TIMEOUT = 60 * 60 * 3 + get_lock_path = operator.methodcaller("joinpath", ".lock") @@ -155,21 +157,21 @@ def cleanup_candidates(root, prefix, keep): def cleanup_numbered_dir(root, prefix, keep, consider_lock_dead_if_created_before): for path in cleanup_candidates(root, prefix, keep): try_cleanup(path, consider_lock_dead_if_created_before) - known_garbage = list(root.glob("garbage-*")) - for path in known_garbage: + for path in root.glob("garbage-*"): try_cleanup(path, consider_lock_dead_if_created_before) def make_numbered_dir_with_cleanup(root, prefix, keep, lock_timeout): + e = None for i in range(10): try: p = make_numbered_dir(root, prefix) lock_path = create_cleanup_lock(p) register_cleanup_lock_removal(lock_path) - except Exception: - raise + except Exception as e: + pass else: - consider_lock_dead_if_created_before = p.stat().st_mtime + lock_timeout + consider_lock_dead_if_created_before = p.stat().st_mtime - lock_timeout cleanup_numbered_dir( root=root, prefix=prefix, @@ -177,6 +179,8 @@ def make_numbered_dir_with_cleanup(root, prefix, keep, lock_timeout): consider_lock_dead_if_created_before=consider_lock_dead_if_created_before, ) return p + assert e is not None + raise e @attr.s diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 360614673..02687f6fc 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -239,7 +239,10 @@ class TestNumberedDir(object): from _pytest.tmpdir import cleanup_numbered_dir cleanup_numbered_dir( - root=tmp_path, prefix=self.PREFIX, keep=2, consider_lock_dead_after=0 + root=tmp_path, + prefix=self.PREFIX, + keep=2, + consider_lock_dead_if_created_before=0, ) a, b = tmp_path.iterdir() print(a, b) @@ -251,5 +254,10 @@ class TestNumberedDir(object): p = tmpdir.make_numbered_dir(root=tmp_path, prefix=self.PREFIX) tmpdir.create_cleanup_lock(p) - assert not tmpdir.ensure_deletable(p, p.stat().st_mtime + 1) - assert tmpdir.ensure_deletable(p, p.stat().st_mtime - 1) + + assert not tmpdir.ensure_deletable( + p, consider_lock_dead_if_created_before=p.stat().st_mtime - 1 + ) + assert tmpdir.ensure_deletable( + p, consider_lock_dead_if_created_before=p.stat().st_mtime + 1 + ) From fed4f73a61384189f591294e8ca0dc9ad50ddb89 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 28 Sep 2018 23:09:00 +0200 Subject: [PATCH 12/25] ignore rmtree errors --- src/_pytest/tmpdir.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index b9bd12afc..80856c0f1 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -119,7 +119,7 @@ def delete_a_numbered_dir(path): garbage = parent.joinpath("garbage-{}".format(uuid.uuid4())) path.rename(garbage) - shutil.rmtree(str(garbage)) + shutil.rmtree(str(garbage), ignore_errors=True) def ensure_deletable(path, consider_lock_dead_if_created_before): From 85cc9b8f128ae3cc5fd14b0b301b69ef8ea87128 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sat, 29 Sep 2018 11:42:31 +0200 Subject: [PATCH 13/25] move all the things into _pytest.pathlib --- src/_pytest/assertion/rewrite.py | 3 +- src/_pytest/cacheprovider.py | 3 +- src/_pytest/compat.py | 7 -- src/_pytest/pathlib.py | 183 +++++++++++++++++++++++++++++++ src/_pytest/paths.py | 2 +- src/_pytest/pytester.py | 7 +- src/_pytest/tmpdir.py | 174 +---------------------------- testing/test_tmpdir.py | 18 +-- 8 files changed, 202 insertions(+), 195 deletions(-) create mode 100644 src/_pytest/pathlib.py diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 7a11c4ec1..f61496050 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -17,7 +17,8 @@ import atomicwrites import py from _pytest.assertion import util -from _pytest.compat import PurePath, spec_from_file_location +from _pytest.pathlib import PurePath +from _pytest.compat import spec_from_file_location from _pytest.paths import fnmatch_ex # pytest caches rewritten pycs in __pycache__. diff --git a/src/_pytest/cacheprovider.py b/src/_pytest/cacheprovider.py index 87e24894b..be291bbff 100755 --- a/src/_pytest/cacheprovider.py +++ b/src/_pytest/cacheprovider.py @@ -16,7 +16,8 @@ import json import shutil from . import paths -from .compat import _PY2 as PY2, Path +from .compat import _PY2 as PY2 +from .pathlib import Path README_CONTENT = u"""\ # pytest cache directory # diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index 02cad24cc..3798f4eb1 100644 --- a/src/_pytest/compat.py +++ b/src/_pytest/compat.py @@ -23,8 +23,6 @@ except ImportError: # pragma: no cover # Only available in Python 3.4+ or as a backport enum = None -__all__ = ["Path", "PurePath"] - _PY3 = sys.version_info > (3, 0) _PY2 = not _PY3 @@ -41,11 +39,6 @@ PY35 = sys.version_info[:2] >= (3, 5) PY36 = sys.version_info[:2] >= (3, 6) MODULE_NOT_FOUND_ERROR = "ModuleNotFoundError" if PY36 else "ImportError" -if PY36: - from pathlib import Path, PurePath -else: - from pathlib2 import Path, PurePath - if _PY3: from collections.abc import MutableMapping as MappingMixin diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py new file mode 100644 index 000000000..2d9f1d640 --- /dev/null +++ b/src/_pytest/pathlib.py @@ -0,0 +1,183 @@ + +import os +import errno +import atexit +import operator +import six +from functools import reduce +import uuid +from six.moves import map +import itertools +import shutil + +from .compat import PY36 + +if PY36: + from pathlib import Path, PurePath +else: + from pathlib2 import Path, PurePath + +__all__ = ["Path", "PurePath"] + + +LOCK_TIMEOUT = 60 * 60 * 3 + +get_lock_path = operator.methodcaller("joinpath", ".lock") + + +def find_prefixed(root, prefix): + l_prefix = prefix.lower() + for x in root.iterdir(): + if x.name.lower().startswith(l_prefix): + yield x + + +def extract_suffixes(iter, prefix): + p_len = len(prefix) + for p in iter: + yield p.name[p_len:] + + +def find_suffixes(root, prefix): + return extract_suffixes(find_prefixed(root, prefix), prefix) + + +def parse_num(maybe_num): + try: + return int(maybe_num) + except ValueError: + return -1 + + +def _max(iterable, default): + # needed due to python2.7 lacking the default argument for max + return reduce(max, iterable, default) + + +def make_numbered_dir(root, prefix): + for i in range(10): + # try up to 10 times to create the folder + max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1) + new_number = max_existing + 1 + new_path = root.joinpath("{}{}".format(prefix, new_number)) + try: + new_path.mkdir() + except Exception: + pass + else: + return new_path + else: + raise EnvironmentError( + "could not create numbered dir with prefix {prefix} in {root})".format( + prefix=prefix, root=root + ) + ) + + +def create_cleanup_lock(p): + lock_path = get_lock_path(p) + try: + fd = os.open(str(lock_path), os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) + except OSError as e: + if e.errno == errno.EEXIST: + six.raise_from( + EnvironmentError("cannot create lockfile in {path}".format(path=p)), e + ) + else: + raise + else: + pid = os.getpid() + spid = str(pid) + if not isinstance(spid, six.binary_type): + spid = spid.encode("ascii") + os.write(fd, spid) + os.close(fd) + if not lock_path.is_file(): + raise EnvironmentError("lock path got renamed after sucessfull creation") + return lock_path + + +def register_cleanup_lock_removal(lock_path, register=atexit.register): + pid = os.getpid() + + def cleanup_on_exit(lock_path=lock_path, original_pid=pid): + current_pid = os.getpid() + if current_pid != original_pid: + # fork + return + try: + lock_path.unlink() + except (OSError, IOError): + pass + + return register(cleanup_on_exit) + + +def delete_a_numbered_dir(path): + create_cleanup_lock(path) + parent = path.parent + + garbage = parent.joinpath("garbage-{}".format(uuid.uuid4())) + path.rename(garbage) + shutil.rmtree(str(garbage), ignore_errors=True) + + +def ensure_deletable(path, consider_lock_dead_if_created_before): + lock = get_lock_path(path) + if not lock.exists(): + return True + try: + lock_time = lock.stat().st_mtime + except Exception: + return False + else: + if lock_time < consider_lock_dead_if_created_before: + lock.unlink() + return True + else: + return False + + +def try_cleanup(path, consider_lock_dead_if_created_before): + if ensure_deletable(path, consider_lock_dead_if_created_before): + delete_a_numbered_dir(path) + + +def cleanup_candidates(root, prefix, keep): + max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1) + max_delete = max_existing - keep + paths = find_prefixed(root, prefix) + paths, paths2 = itertools.tee(paths) + numbers = map(parse_num, extract_suffixes(paths2, prefix)) + for path, number in zip(paths, numbers): + if number <= max_delete: + yield path + + +def cleanup_numbered_dir(root, prefix, keep, consider_lock_dead_if_created_before): + for path in cleanup_candidates(root, prefix, keep): + try_cleanup(path, consider_lock_dead_if_created_before) + for path in root.glob("garbage-*"): + try_cleanup(path, consider_lock_dead_if_created_before) + + +def make_numbered_dir_with_cleanup(root, prefix, keep, lock_timeout): + e = None + for i in range(10): + try: + p = make_numbered_dir(root, prefix) + lock_path = create_cleanup_lock(p) + register_cleanup_lock_removal(lock_path) + except Exception as e: + pass + else: + consider_lock_dead_if_created_before = p.stat().st_mtime - lock_timeout + cleanup_numbered_dir( + root=root, + prefix=prefix, + keep=keep, + consider_lock_dead_if_created_before=consider_lock_dead_if_created_before, + ) + return p + assert e is not None + raise e diff --git a/src/_pytest/paths.py b/src/_pytest/paths.py index 031ea6b26..3507cae7f 100644 --- a/src/_pytest/paths.py +++ b/src/_pytest/paths.py @@ -5,7 +5,7 @@ import sys import six -from .compat import Path, PurePath +from .pathlib import Path, PurePath def resolve_from_str(input, root): diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index 1e64c1747..85f824784 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -17,13 +17,14 @@ from weakref import WeakKeyDictionary from _pytest.capture import MultiCapture, SysCapture from _pytest._code import Source -import py -import pytest from _pytest.main import Session, EXIT_OK from _pytest.assertion.rewrite import AssertionRewritingHook -from _pytest.compat import Path +from _pytest.pathlib import Path from _pytest.compat import safe_str +import py +import pytest + IGNORE_PAM = [ # filenames added when obtaining details about the current user u"/var/lib/sss/mc/passwd" ] diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 80856c0f1..40a9cbf90 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -2,185 +2,13 @@ from __future__ import absolute_import, division, print_function import re -import os -import errno -import atexit -import operator -import six -from functools import reduce -import uuid -from six.moves import map import pytest import py from _pytest.monkeypatch import MonkeyPatch -from .compat import Path import attr import shutil import tempfile -import itertools - - -LOCK_TIMEOUT = 60 * 60 * 3 - -get_lock_path = operator.methodcaller("joinpath", ".lock") - - -def find_prefixed(root, prefix): - l_prefix = prefix.lower() - for x in root.iterdir(): - if x.name.lower().startswith(l_prefix): - yield x - - -def extract_suffixes(iter, prefix): - p_len = len(prefix) - for p in iter: - yield p.name[p_len:] - - -def find_suffixes(root, prefix): - return extract_suffixes(find_prefixed(root, prefix), prefix) - - -def parse_num(maybe_num): - try: - return int(maybe_num) - except ValueError: - return -1 - - -def _max(iterable, default): - # needed due to python2.7 lacking the default argument for max - return reduce(max, iterable, default) - - -def make_numbered_dir(root, prefix): - for i in range(10): - # try up to 10 times to create the folder - max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1) - new_number = max_existing + 1 - new_path = root.joinpath("{}{}".format(prefix, new_number)) - try: - new_path.mkdir() - except Exception: - pass - else: - return new_path - else: - raise EnvironmentError( - "could not create numbered dir with prefix {prefix} in {root})".format( - prefix=prefix, root=root - ) - ) - - -def create_cleanup_lock(p): - lock_path = get_lock_path(p) - try: - fd = os.open(str(lock_path), os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) - except OSError as e: - if e.errno == errno.EEXIST: - six.raise_from( - EnvironmentError("cannot create lockfile in {path}".format(path=p)), e - ) - else: - raise - else: - pid = os.getpid() - spid = str(pid) - if not isinstance(spid, six.binary_type): - spid = spid.encode("ascii") - os.write(fd, spid) - os.close(fd) - if not lock_path.is_file(): - raise EnvironmentError("lock path got renamed after sucessfull creation") - return lock_path - - -def register_cleanup_lock_removal(lock_path, register=atexit.register): - pid = os.getpid() - - def cleanup_on_exit(lock_path=lock_path, original_pid=pid): - current_pid = os.getpid() - if current_pid != original_pid: - # fork - return - try: - lock_path.unlink() - except (OSError, IOError): - pass - - return register(cleanup_on_exit) - - -def delete_a_numbered_dir(path): - create_cleanup_lock(path) - parent = path.parent - - garbage = parent.joinpath("garbage-{}".format(uuid.uuid4())) - path.rename(garbage) - shutil.rmtree(str(garbage), ignore_errors=True) - - -def ensure_deletable(path, consider_lock_dead_if_created_before): - lock = get_lock_path(path) - if not lock.exists(): - return True - try: - lock_time = lock.stat().st_mtime - except Exception: - return False - else: - if lock_time < consider_lock_dead_if_created_before: - lock.unlink() - return True - else: - return False - - -def try_cleanup(path, consider_lock_dead_if_created_before): - if ensure_deletable(path, consider_lock_dead_if_created_before): - delete_a_numbered_dir(path) - - -def cleanup_candidates(root, prefix, keep): - max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1) - max_delete = max_existing - keep - paths = find_prefixed(root, prefix) - paths, paths2 = itertools.tee(paths) - numbers = map(parse_num, extract_suffixes(paths2, prefix)) - for path, number in zip(paths, numbers): - if number <= max_delete: - yield path - - -def cleanup_numbered_dir(root, prefix, keep, consider_lock_dead_if_created_before): - for path in cleanup_candidates(root, prefix, keep): - try_cleanup(path, consider_lock_dead_if_created_before) - for path in root.glob("garbage-*"): - try_cleanup(path, consider_lock_dead_if_created_before) - - -def make_numbered_dir_with_cleanup(root, prefix, keep, lock_timeout): - e = None - for i in range(10): - try: - p = make_numbered_dir(root, prefix) - lock_path = create_cleanup_lock(p) - register_cleanup_lock_removal(lock_path) - except Exception as e: - pass - else: - consider_lock_dead_if_created_before = p.stat().st_mtime - lock_timeout - cleanup_numbered_dir( - root=root, - prefix=prefix, - keep=keep, - consider_lock_dead_if_created_before=consider_lock_dead_if_created_before, - ) - return p - assert e is not None - raise e +from .pathlib import Path, make_numbered_dir, make_numbered_dir_with_cleanup @attr.s diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 02687f6fc..2148a8efe 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -192,7 +192,7 @@ class TestNumberedDir(object): PREFIX = "fun-" def test_make(self, tmp_path): - from _pytest.tmpdir import make_numbered_dir + from _pytest.pathlib import make_numbered_dir for i in range(10): d = make_numbered_dir(root=tmp_path, prefix=self.PREFIX) @@ -202,7 +202,7 @@ class TestNumberedDir(object): def test_cleanup_lock_create(self, tmp_path): d = tmp_path.joinpath("test") d.mkdir() - from _pytest.tmpdir import create_cleanup_lock + from _pytest.pathlib import create_cleanup_lock lockfile = create_cleanup_lock(d) with pytest.raises(EnvironmentError, match="cannot create lockfile in .*"): @@ -211,7 +211,7 @@ class TestNumberedDir(object): lockfile.unlink() def test_lock_register_cleanup_removal(self, tmp_path): - from _pytest.tmpdir import create_cleanup_lock, register_cleanup_lock_removal + from _pytest.pathlib import create_cleanup_lock, register_cleanup_lock_removal lock = create_cleanup_lock(tmp_path) @@ -236,7 +236,7 @@ class TestNumberedDir(object): def test_cleanup_keep(self, tmp_path): self.test_make(tmp_path) - from _pytest.tmpdir import cleanup_numbered_dir + from _pytest.pathlib import cleanup_numbered_dir cleanup_numbered_dir( root=tmp_path, @@ -249,15 +249,15 @@ class TestNumberedDir(object): def test_cleanup_locked(self, tmp_path): - from _pytest import tmpdir + from _pytest import pathlib - p = tmpdir.make_numbered_dir(root=tmp_path, prefix=self.PREFIX) + p = pathlib.make_numbered_dir(root=tmp_path, prefix=self.PREFIX) - tmpdir.create_cleanup_lock(p) + pathlib.create_cleanup_lock(p) - assert not tmpdir.ensure_deletable( + assert not pathlib.ensure_deletable( p, consider_lock_dead_if_created_before=p.stat().st_mtime - 1 ) - assert tmpdir.ensure_deletable( + assert pathlib.ensure_deletable( p, consider_lock_dead_if_created_before=p.stat().st_mtime + 1 ) From 00716177b4af3dd885ea1c1cb0ccfd71a3893b51 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sat, 29 Sep 2018 13:16:27 +0200 Subject: [PATCH 14/25] fix missed Path import --- testing/python/fixture.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/python/fixture.py b/testing/python/fixture.py index a9e33b455..8d7b07b0d 100644 --- a/testing/python/fixture.py +++ b/testing/python/fixture.py @@ -6,7 +6,7 @@ import pytest from _pytest.pytester import get_public_names from _pytest.fixtures import FixtureLookupError, FixtureRequest from _pytest import fixtures -from _pytest.compat import Path +from _pytest.pathlib import Path def test_getfuncargnames(): From 2831cb9ab5eb5acdc2eb0e55becf507a93ea3ac0 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Mon, 1 Oct 2018 13:44:52 +0200 Subject: [PATCH 15/25] unify paths.py and pathlib.py --- src/_pytest/assertion/rewrite.py | 2 +- src/_pytest/cacheprovider.py | 5 ++- src/_pytest/pathlib.py | 49 ++++++++++++++++++++++++++++++ src/_pytest/paths.py | 52 -------------------------------- testing/test_paths.py | 2 +- 5 files changed, 53 insertions(+), 57 deletions(-) delete mode 100644 src/_pytest/paths.py diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index f61496050..88331dd4b 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -19,7 +19,7 @@ import py from _pytest.assertion import util from _pytest.pathlib import PurePath from _pytest.compat import spec_from_file_location -from _pytest.paths import fnmatch_ex +from _pytest.pathlib import fnmatch_ex # pytest caches rewritten pycs in __pycache__. if hasattr(imp, "get_tag"): diff --git a/src/_pytest/cacheprovider.py b/src/_pytest/cacheprovider.py index be291bbff..eaa470fbf 100755 --- a/src/_pytest/cacheprovider.py +++ b/src/_pytest/cacheprovider.py @@ -15,9 +15,8 @@ import pytest import json import shutil -from . import paths from .compat import _PY2 as PY2 -from .pathlib import Path +from .pathlib import Path, resolve_from_str README_CONTENT = u"""\ # pytest cache directory # @@ -46,7 +45,7 @@ class Cache(object): @staticmethod def cache_dir_from_config(config): - return paths.resolve_from_str(config.getini("cache_dir"), config.rootdir) + return resolve_from_str(config.getini("cache_dir"), config.rootdir) def warn(self, fmt, **args): from _pytest.warnings import _issue_config_warning diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 2d9f1d640..a86f1e40a 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -4,14 +4,19 @@ import errno import atexit import operator import six +import sys from functools import reduce import uuid from six.moves import map import itertools import shutil +from os.path import expanduser, expandvars, isabs, sep +from posixpath import sep as posix_sep +import fnmatch from .compat import PY36 + if PY36: from pathlib import Path, PurePath else: @@ -181,3 +186,47 @@ def make_numbered_dir_with_cleanup(root, prefix, keep, lock_timeout): return p assert e is not None raise e + + +def resolve_from_str(input, root): + assert not isinstance(input, Path), "would break on py2" + root = Path(root) + input = expanduser(input) + input = expandvars(input) + if isabs(input): + return Path(input) + else: + return root.joinpath(input) + + +def fnmatch_ex(pattern, path): + """FNMatcher port from py.path.common which works with PurePath() instances. + + The difference between this algorithm and PurePath.match() is that the latter matches "**" glob expressions + for each part of the path, while this algorithm uses the whole path instead. + + For example: + "tests/foo/bar/doc/test_foo.py" matches pattern "tests/**/doc/test*.py" with this algorithm, but not with + PurePath.match(). + + This algorithm was ported to keep backward-compatibility with existing settings which assume paths match according + this logic. + + References: + * https://bugs.python.org/issue29249 + * https://bugs.python.org/issue34731 + """ + path = PurePath(path) + iswin32 = sys.platform.startswith("win") + + if iswin32 and sep not in pattern and posix_sep in pattern: + # Running on Windows, the pattern has no Windows path separators, + # and the pattern has one or more Posix path separators. Replace + # the Posix path separators with the Windows path separator. + pattern = pattern.replace(posix_sep, sep) + + if sep not in pattern: + name = path.name + else: + name = six.text_type(path) + return fnmatch.fnmatch(name, pattern) diff --git a/src/_pytest/paths.py b/src/_pytest/paths.py deleted file mode 100644 index 3507cae7f..000000000 --- a/src/_pytest/paths.py +++ /dev/null @@ -1,52 +0,0 @@ -from os.path import expanduser, expandvars, isabs, sep -from posixpath import sep as posix_sep -import fnmatch -import sys - -import six - -from .pathlib import Path, PurePath - - -def resolve_from_str(input, root): - assert not isinstance(input, Path), "would break on py2" - root = Path(root) - input = expanduser(input) - input = expandvars(input) - if isabs(input): - return Path(input) - else: - return root.joinpath(input) - - -def fnmatch_ex(pattern, path): - """FNMatcher port from py.path.common which works with PurePath() instances. - - The difference between this algorithm and PurePath.match() is that the latter matches "**" glob expressions - for each part of the path, while this algorithm uses the whole path instead. - - For example: - "tests/foo/bar/doc/test_foo.py" matches pattern "tests/**/doc/test*.py" with this algorithm, but not with - PurePath.match(). - - This algorithm was ported to keep backward-compatibility with existing settings which assume paths match according - this logic. - - References: - * https://bugs.python.org/issue29249 - * https://bugs.python.org/issue34731 - """ - path = PurePath(path) - iswin32 = sys.platform.startswith("win") - - if iswin32 and sep not in pattern and posix_sep in pattern: - # Running on Windows, the pattern has no Windows path separators, - # and the pattern has one or more Posix path separators. Replace - # the Posix path separators with the Windows path separator. - pattern = pattern.replace(posix_sep, sep) - - if sep not in pattern: - name = path.name - else: - name = six.text_type(path) - return fnmatch.fnmatch(name, pattern) diff --git a/testing/test_paths.py b/testing/test_paths.py index 2bb1335fb..2eb07bbd4 100644 --- a/testing/test_paths.py +++ b/testing/test_paths.py @@ -4,7 +4,7 @@ import py import pytest -from _pytest.paths import fnmatch_ex +from _pytest.pathlib import fnmatch_ex class TestPort: From 30369140979973c486b0c2c55f8d346db8d9f252 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Mon, 1 Oct 2018 16:39:24 +0200 Subject: [PATCH 16/25] sort out rmtree expectations --- src/_pytest/cacheprovider.py | 5 ++-- src/_pytest/pathlib.py | 48 ++++++++++++++++++++++++++++++++++-- src/_pytest/tmpdir.py | 12 +++++---- testing/test_tmpdir.py | 16 ++++++++++++ 4 files changed, 71 insertions(+), 10 deletions(-) diff --git a/src/_pytest/cacheprovider.py b/src/_pytest/cacheprovider.py index eaa470fbf..fd8ef8fc0 100755 --- a/src/_pytest/cacheprovider.py +++ b/src/_pytest/cacheprovider.py @@ -13,10 +13,9 @@ import attr import pytest import json -import shutil from .compat import _PY2 as PY2 -from .pathlib import Path, resolve_from_str +from .pathlib import Path, resolve_from_str, rmtree README_CONTENT = u"""\ # pytest cache directory # @@ -39,7 +38,7 @@ class Cache(object): def for_config(cls, config): cachedir = cls.cache_dir_from_config(config) if config.getoption("cacheclear") and cachedir.exists(): - shutil.rmtree(str(cachedir)) + rmtree(cachedir, force=True) cachedir.mkdir() return cls(cachedir, config) diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index a86f1e40a..cd9796973 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -13,6 +13,7 @@ import shutil from os.path import expanduser, expandvars, isabs, sep from posixpath import sep as posix_sep import fnmatch +import stat from .compat import PY36 @@ -30,7 +31,33 @@ LOCK_TIMEOUT = 60 * 60 * 3 get_lock_path = operator.methodcaller("joinpath", ".lock") +def ensure_reset_dir(path): + """ + ensures the given path is a empty directory + """ + if path.exists(): + rmtree(path, force=True) + path.mkdir() + + +def _shutil_rmtree_remove_writable(func, fspath, _): + "Clear the readonly bit and reattempt the removal" + os.chmod(fspath, stat.S_IWRITE) + func(fspath) + + +def rmtree(path, force=False): + if force: + # ignore_errors leaves dead folders around + # python needs a rm -rf as a followup + # the trick with _shutil_rmtree_remove_writable is unreliable + shutil.rmtree(str(path), ignore_errors=True) + else: + shutil.rmtree(str(path)) + + def find_prefixed(root, prefix): + """finds all elements in root that begin with the prefix, case insensitive""" l_prefix = prefix.lower() for x in root.iterdir(): if x.name.lower().startswith(l_prefix): @@ -38,16 +65,24 @@ def find_prefixed(root, prefix): def extract_suffixes(iter, prefix): + """ + :param iter: iterator over path names + :param prefix: expected prefix of the path names + :returns: the parts of the paths following the prefix + """ p_len = len(prefix) for p in iter: yield p.name[p_len:] def find_suffixes(root, prefix): + """combines find_prefixes and extract_suffixes + """ return extract_suffixes(find_prefixed(root, prefix), prefix) def parse_num(maybe_num): + """parses number path suffixes, returns -1 on error""" try: return int(maybe_num) except ValueError: @@ -55,11 +90,12 @@ def parse_num(maybe_num): def _max(iterable, default): - # needed due to python2.7 lacking the default argument for max + """needed due to python2.7 lacking the default argument for max""" return reduce(max, iterable, default) def make_numbered_dir(root, prefix): + """create a directory with a increased number as suffix for the given prefix""" for i in range(10): # try up to 10 times to create the folder max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1) @@ -80,6 +116,7 @@ def make_numbered_dir(root, prefix): def create_cleanup_lock(p): + """crates a lock to prevent premature folder cleanup""" lock_path = get_lock_path(p) try: fd = os.open(str(lock_path), os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) @@ -103,6 +140,7 @@ def create_cleanup_lock(p): def register_cleanup_lock_removal(lock_path, register=atexit.register): + """registers a cleanup function for removing a lock, by default on atexit""" pid = os.getpid() def cleanup_on_exit(lock_path=lock_path, original_pid=pid): @@ -119,15 +157,17 @@ def register_cleanup_lock_removal(lock_path, register=atexit.register): def delete_a_numbered_dir(path): + """removes a numbered directory""" create_cleanup_lock(path) parent = path.parent garbage = parent.joinpath("garbage-{}".format(uuid.uuid4())) path.rename(garbage) - shutil.rmtree(str(garbage), ignore_errors=True) + rmtree(garbage, force=True) def ensure_deletable(path, consider_lock_dead_if_created_before): + """checks if a lock exists and breaks it if its considered dead""" lock = get_lock_path(path) if not lock.exists(): return True @@ -144,11 +184,13 @@ def ensure_deletable(path, consider_lock_dead_if_created_before): def try_cleanup(path, consider_lock_dead_if_created_before): + """tries to cleanup a folder if we can ensure its deletable""" if ensure_deletable(path, consider_lock_dead_if_created_before): delete_a_numbered_dir(path) def cleanup_candidates(root, prefix, keep): + """lists candidates for numbered directories to be removed - follows py.path""" max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1) max_delete = max_existing - keep paths = find_prefixed(root, prefix) @@ -160,6 +202,7 @@ def cleanup_candidates(root, prefix, keep): def cleanup_numbered_dir(root, prefix, keep, consider_lock_dead_if_created_before): + """cleanup for lock driven numbered directories""" for path in cleanup_candidates(root, prefix, keep): try_cleanup(path, consider_lock_dead_if_created_before) for path in root.glob("garbage-*"): @@ -167,6 +210,7 @@ def cleanup_numbered_dir(root, prefix, keep, consider_lock_dead_if_created_befor def make_numbered_dir_with_cleanup(root, prefix, keep, lock_timeout): + """creates a numbered dir with a cleanup lock and removes old ones""" e = None for i in range(10): try: diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 40a9cbf90..65562db4d 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -6,9 +6,13 @@ import pytest import py from _pytest.monkeypatch import MonkeyPatch import attr -import shutil import tempfile -from .pathlib import Path, make_numbered_dir, make_numbered_dir_with_cleanup +from .pathlib import ( + Path, + make_numbered_dir, + make_numbered_dir_with_cleanup, + ensure_reset_dir, +) @attr.s @@ -39,9 +43,7 @@ class TempPathFactory(object): if self._basetemp is None: if self.given_basetemp is not None: basetemp = Path(self.given_basetemp) - if basetemp.exists(): - shutil.rmtree(str(basetemp)) - basetemp.mkdir() + ensure_reset_dir(basetemp) else: temproot = Path(tempfile.gettempdir()) user = get_user() or "unknown" diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 2148a8efe..db1e8b00c 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -261,3 +261,19 @@ class TestNumberedDir(object): assert pathlib.ensure_deletable( p, consider_lock_dead_if_created_before=p.stat().st_mtime + 1 ) + + def test_rmtree(self, tmp_path): + from _pytest.pathlib import rmtree + + adir = tmp_path / "adir" + adir.mkdir() + rmtree(adir) + + assert not adir.exists() + + adir.mkdir() + afile = adir / "afile" + afile.write_bytes(b"aa") + + rmtree(adir, force=True) + assert not adir.exists() From ad6f63edda671733d4ad08a390da3bc6c1b758d1 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Mon, 1 Oct 2018 21:28:42 +0200 Subject: [PATCH 17/25] add changelog --- changelog/3985.feature.rst | 1 + changelog/3988.trivial.rst | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog/3985.feature.rst create mode 100644 changelog/3988.trivial.rst diff --git a/changelog/3985.feature.rst b/changelog/3985.feature.rst new file mode 100644 index 000000000..19070cad0 --- /dev/null +++ b/changelog/3985.feature.rst @@ -0,0 +1 @@ +Introduce ``tmp_path`` as a fixture providing a Path object. diff --git a/changelog/3988.trivial.rst b/changelog/3988.trivial.rst new file mode 100644 index 000000000..876db9798 --- /dev/null +++ b/changelog/3988.trivial.rst @@ -0,0 +1 @@ +Port the implementation of tmpdir to pathlib. From 4a436b54709b4d6a530cb2931e6f51a17c97945b Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Tue, 2 Oct 2018 08:03:58 +0200 Subject: [PATCH 18/25] resolve in code review commments --- src/_pytest/pathlib.py | 21 +++++++++++++-------- src/_pytest/tmpdir.py | 33 +++++++++++++++++++++++++-------- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index cd9796973..439f4d9ba 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -89,16 +89,22 @@ def parse_num(maybe_num): return -1 -def _max(iterable, default): - """needed due to python2.7 lacking the default argument for max""" - return reduce(max, iterable, default) +if six.PY2: + + def _max(iterable, default): + """needed due to python2.7 lacking the default argument for max""" + return reduce(max, iterable, default) + + +else: + _max = max def make_numbered_dir(root, prefix): """create a directory with a increased number as suffix for the given prefix""" for i in range(10): # try up to 10 times to create the folder - max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1) + max_existing = _max(map(parse_num, find_suffixes(root, prefix)), default=-1) new_number = max_existing + 1 new_path = root.joinpath("{}{}".format(prefix, new_number)) try: @@ -109,9 +115,8 @@ def make_numbered_dir(root, prefix): return new_path else: raise EnvironmentError( - "could not create numbered dir with prefix {prefix} in {root})".format( - prefix=prefix, root=root - ) + "could not create numbered dir with prefix " + "{prefix} in {root} after 10 tries".format(prefix=prefix, root=root) ) @@ -191,7 +196,7 @@ def try_cleanup(path, consider_lock_dead_if_created_before): def cleanup_candidates(root, prefix, keep): """lists candidates for numbered directories to be removed - follows py.path""" - max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1) + max_existing = _max(map(parse_num, find_suffixes(root, prefix)), default=-1) max_delete = max_existing - keep paths = find_prefixed(root, prefix) paths, paths2 = itertools.tee(paths) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 65562db4d..422f1f7fb 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -17,7 +17,9 @@ from .pathlib import ( @attr.s class TempPathFactory(object): - """docstring for ClassName""" + """Factory for temporary directories under the common base temp directory. + + The base directory can be configured using the ``--basetemp`` option.""" given_basetemp = attr.ib() trace = attr.ib() @@ -25,11 +27,15 @@ class TempPathFactory(object): @classmethod def from_config(cls, config): + """ + :param config: a pytest configuration + """ return cls( given_basetemp=config.option.basetemp, trace=config.trace.get("tmpdir") ) def mktemp(self, basename, numbered=True): + """makes a temporary directory managed by the factory""" if not numbered: p = self.getbasetemp().joinpath(basename) p.mkdir() @@ -64,12 +70,12 @@ class TempPathFactory(object): @attr.s class TempdirFactory(object): - """Factory for temporary directories under the common base temp directory. - - The base directory can be configured using the ``--basetemp`` option. + """ + backward comptibility wrapper that implements + :class:``py.path.local`` for :class:``TempPathFactory`` """ - tmppath_factory = attr.ib() + _tmppath_factory = attr.ib() def ensuretemp(self, string, dir=1): """ (deprecated) return temporary directory path with @@ -86,13 +92,13 @@ class TempdirFactory(object): If ``numbered``, ensure the directory is unique by adding a number prefix greater than any existing one. """ - return py.path.local(self.tmppath_factory.mktemp(basename, numbered).resolve()) + return py.path.local(self._tmppath_factory.mktemp(basename, numbered).resolve()) def getbasetemp(self): - return py.path.local(self.tmppath_factory.getbasetemp().resolve()) + return py.path.local(self._tmppath_factory.getbasetemp().resolve()) def finish(self): - self.tmppath_factory.trace("finish") + self._tmppath_factory.trace("finish") def get_user(): @@ -150,4 +156,15 @@ def tmpdir(request, tmpdir_factory): @pytest.fixture def tmp_path(tmpdir): + """Return a temporary directory path object + which is unique to each test function invocation, + created as a sub directory of the base temporary + directory. The returned object is a :class:`pathlib.Path` + object. + + .. note:: + + in python < 3.6 this is a pathlib2.Path + """ + return Path(tmpdir) From b82d6f7a0b628a5f1cb6d326ec25389f3c804abf Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Tue, 2 Oct 2018 08:34:02 +0200 Subject: [PATCH 19/25] pytester: use per test tmproot --- src/_pytest/pytester.py | 3 +++ src/_pytest/tmpdir.py | 4 +++- testing/test_tmpdir.py | 2 -- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index 85f824784..228f9a193 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -496,6 +496,8 @@ class Testdir(object): self._mod_collections = WeakKeyDictionary() name = request.function.__name__ self.tmpdir = tmpdir_factory.mktemp(name, numbered=True) + self.test_tmproot = tmpdir_factory.mktemp("tmp-" + name, numbered=True) + os.environ["PYTEST_DEBUG_TEMPROOT"] = str(self.test_tmproot) self.plugins = [] self._cwd_snapshot = CwdSnapshot() self._sys_path_snapshot = SysPathsSnapshot() @@ -522,6 +524,7 @@ class Testdir(object): self._sys_modules_snapshot.restore() self._sys_path_snapshot.restore() self._cwd_snapshot.restore() + del os.environ["PYTEST_DEBUG_TEMPROOT"] def __take_sys_modules_snapshot(self): # some zope modules used by twisted-related tests keep internal state diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 422f1f7fb..7b5f6510e 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -1,6 +1,7 @@ """ support for providing temporary directories to test functions. """ from __future__ import absolute_import, division, print_function +import os import re import pytest import py @@ -51,7 +52,8 @@ class TempPathFactory(object): basetemp = Path(self.given_basetemp) ensure_reset_dir(basetemp) else: - temproot = Path(tempfile.gettempdir()) + from_env = os.environ.get("PYTEST_DEBUG_TEMPROOT") + temproot = Path(from_env or tempfile.gettempdir()) user = get_user() or "unknown" # use a sub-directory in the temproot to speed-up # make_numbered_dir() call diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index db1e8b00c..9244e309d 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -143,7 +143,6 @@ def break_getuser(monkeypatch): monkeypatch.delenv(envvar, raising=False) -@pytest.mark.skip(reason="creates random tmpdirs as part of a system level test") @pytest.mark.usefixtures("break_getuser") @pytest.mark.skipif(sys.platform.startswith("win"), reason="no os.getuid on windows") def test_tmpdir_fallback_uid_not_found(testdir): @@ -162,7 +161,6 @@ def test_tmpdir_fallback_uid_not_found(testdir): reprec.assertoutcome(passed=1) -@pytest.mark.skip(reason="creates random tmpdirs as part of a system level test") @pytest.mark.usefixtures("break_getuser") @pytest.mark.skipif(sys.platform.startswith("win"), reason="no os.getuid on windows") def test_get_user_uid_not_found(): From 94829c391bc868f40f727be7f86d16fd1c223756 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Tue, 2 Oct 2018 10:20:09 +0200 Subject: [PATCH 20/25] make tmpdir env cleanup idempotent --- src/_pytest/pytester.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index 228f9a193..956e00087 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -524,7 +524,7 @@ class Testdir(object): self._sys_modules_snapshot.restore() self._sys_path_snapshot.restore() self._cwd_snapshot.restore() - del os.environ["PYTEST_DEBUG_TEMPROOT"] + os.environ.pop("PYTEST_DEBUG_TEMPROOT", None) def __take_sys_modules_snapshot(self): # some zope modules used by twisted-related tests keep internal state From ebd597b2fdba6faca1645c75cf3cf82849c76715 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Tue, 2 Oct 2018 20:29:33 +0200 Subject: [PATCH 21/25] use the constant for lock timeouts --- src/_pytest/tmpdir.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 7b5f6510e..cc38dcbf2 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -13,6 +13,7 @@ from .pathlib import ( make_numbered_dir, make_numbered_dir_with_cleanup, ensure_reset_dir, + LOCK_TIMEOUT, ) @@ -60,7 +61,7 @@ class TempPathFactory(object): rootdir = temproot.joinpath("pytest-of-{}".format(user)) rootdir.mkdir(exist_ok=True) basetemp = make_numbered_dir_with_cleanup( - prefix="pytest-", root=rootdir, keep=3, lock_timeout=10000 + prefix="pytest-", root=rootdir, keep=3, lock_timeout=LOCK_TIMEOUT ) assert basetemp is not None self._basetemp = t = basetemp From 36c2a101cbf047df9303c57c94a1f2cae7282f52 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Tue, 2 Oct 2018 20:46:09 +0200 Subject: [PATCH 22/25] add missing docstring --- src/_pytest/tmpdir.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index cc38dcbf2..520a01a8e 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -98,6 +98,7 @@ class TempdirFactory(object): return py.path.local(self._tmppath_factory.mktemp(basename, numbered).resolve()) def getbasetemp(self): + """backward compat wrapper for ``_tmppath_factory.getbasetemp``""" return py.path.local(self._tmppath_factory.getbasetemp().resolve()) def finish(self): From 16e2737da3e25a076d61166f2c341db5ee27e977 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 11 Oct 2018 09:41:37 +0200 Subject: [PATCH 23/25] implement tmp_path_factory and deprecate pytest.ensuretemp as intended --- src/_pytest/deprecated.py | 5 +++++ src/_pytest/tmpdir.py | 40 +++++++++++++++++++++++++-------------- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 69beeab5f..19c2d2818 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -109,3 +109,8 @@ PYTEST_PLUGINS_FROM_NON_TOP_LEVEL_CONFTEST = RemovedInPytest4Warning( PYTEST_NAMESPACE = RemovedInPytest4Warning( "pytest_namespace is deprecated and will be removed soon" ) + +PYTEST_ENSURETEMP = RemovedInPytest4Warning( + "pytest/tmpdir_factory.ensuretemp is deprecated, \n" + "please use the tmp_path fixture or tmp_path_factory.mktemp" +) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 520a01a8e..e86994b8e 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -8,6 +8,8 @@ import py from _pytest.monkeypatch import MonkeyPatch import attr import tempfile +import warnings + from .pathlib import ( Path, make_numbered_dir, @@ -88,6 +90,9 @@ class TempdirFactory(object): and is guaranteed to be empty. """ # py.log._apiwarn(">1.1", "use tmpdir function argument") + from .deprecated import PYTEST_ENSURETEMP + + warnings.warn(PYTEST_ENSURETEMP, stacklevel=2) return self.getbasetemp().ensure(string, dir=dir) def mktemp(self, basename, numbered=True): @@ -101,9 +106,6 @@ class TempdirFactory(object): """backward compat wrapper for ``_tmppath_factory.getbasetemp``""" return py.path.local(self._tmppath_factory.getbasetemp().resolve()) - def finish(self): - self._tmppath_factory.trace("finish") - def get_user(): """Return the current user name, or None if getuser() does not work @@ -127,18 +129,34 @@ def pytest_configure(config): mp = MonkeyPatch() tmppath_handler = TempPathFactory.from_config(config) t = TempdirFactory(tmppath_handler) - config._cleanup.extend([mp.undo, t.finish]) + config._cleanup.append(mp.undo) + mp.setattr(config, "_tmp_path_factory", tmppath_handler, raising=False) mp.setattr(config, "_tmpdirhandler", t, raising=False) mp.setattr(pytest, "ensuretemp", t.ensuretemp, raising=False) @pytest.fixture(scope="session") def tmpdir_factory(request): - """Return a TempdirFactory instance for the test session. + """Return a :class:`_pytest.tmpdir.TempdirFactory` instance for the test session. """ return request.config._tmpdirhandler +@pytest.fixture(scope="session") +def tmp_path_factory(request): + """Return a :class:`_pytest.tmpdir.TempPathFactory` instance for the test session. + """ + return request.config._tmp_path_factory + + +def _mk_tmp(request, factory): + name = request.node.name + name = re.sub(r"[\W]", "_", name) + MAXVAL = 30 + name = name[:MAXVAL] + return factory.mktemp(name, numbered=True) + + @pytest.fixture def tmpdir(request, tmpdir_factory): """Return a temporary directory path object @@ -149,17 +167,11 @@ def tmpdir(request, tmpdir_factory): .. _`py.path.local`: https://py.readthedocs.io/en/latest/path.html """ - name = request.node.name - name = re.sub(r"[\W]", "_", name) - MAXVAL = 30 - if len(name) > MAXVAL: - name = name[:MAXVAL] - x = tmpdir_factory.mktemp(name, numbered=True) - return x + return _mk_tmp(request, tmpdir_factory) @pytest.fixture -def tmp_path(tmpdir): +def tmp_path(request, tmp_path_factory): """Return a temporary directory path object which is unique to each test function invocation, created as a sub directory of the base temporary @@ -171,4 +183,4 @@ def tmp_path(tmpdir): in python < 3.6 this is a pathlib2.Path """ - return Path(tmpdir) + return _mk_tmp(request, tmp_path_factory) From 584051aa90b3437bcbd509a6591393e7e4d1569d Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 11 Oct 2018 10:33:59 +0200 Subject: [PATCH 24/25] extend docs with basics about tmp_path and tmp_path_facotry --- doc/en/tmpdir.rst | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/doc/en/tmpdir.rst b/doc/en/tmpdir.rst index 421b4c898..1c815c8d1 100644 --- a/doc/en/tmpdir.rst +++ b/doc/en/tmpdir.rst @@ -5,6 +5,51 @@ Temporary directories and files ================================================ +The ``tmp_path`` fixture +---------------------- + +.. versionadded:: 3.9 + + +You can use the ``tmpdir`` fixture which will +provide a temporary directory unique to the test invocation, +created in the `base temporary directory`_. + +``tmpdir`` is a `pathlib/pathlib2.Path`_ object. Here is an example test usage:: + + # content of test_tmp_path.py + import os + CONTENT = u"content" + + def test_create_file(tmp_path): + d = tmp_path / "sub" + d.mkdir() + p = d / "hello.txt" + p.write_text(CONTENT) + assert p.read_text() == CONTENT + assert len(tmpdir.listdir()) == 1 + assert 0 + +Running this would result in a passed test except for the last +``assert 0`` line which we use to look at values:: + + $ pytest test_tmp_path.py + ... #fill fom regendoc + + + +The ``tmp_path_facotry`` fixture +------------------------------ + +.. versionadded:: 3.9 + + +The ``tmp_path_facotry`` is a session-scoped fixture which can be used +to create arbitrary temporary directories from any other fixture or test. + +its intended to replace ``tmpdir_factory`` + + The 'tmpdir' fixture -------------------- From 4736b2bdfb99f66f998cdc387627d64fd38447d1 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 11 Oct 2018 15:33:19 +0200 Subject: [PATCH 25/25] address review comments --- .pre-commit-config.yaml | 2 +- changelog/3988.deprecation.rst | 1 + doc/en/tmpdir.rst | 14 +++++++++----- src/_pytest/tmpdir.py | 12 ++++++------ 4 files changed, 17 insertions(+), 12 deletions(-) create mode 100644 changelog/3988.deprecation.rst diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index fc6d9e10c..e5cc56230 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -43,6 +43,6 @@ repos: - id: changelogs-rst name: changelog filenames language: fail - entry: 'changelog files must be named ####.(feature|bugfix|doc|removal|vendor|trivial).rst' + entry: 'changelog files must be named ####.(feature|bugfix|doc|deprecation|removal|vendor|trivial).rst' exclude: changelog/(\d+\.(feature|bugfix|doc|deprecation|removal|vendor|trivial).rst|README.rst|_template.rst) files: ^changelog/ diff --git a/changelog/3988.deprecation.rst b/changelog/3988.deprecation.rst new file mode 100644 index 000000000..b731112e4 --- /dev/null +++ b/changelog/3988.deprecation.rst @@ -0,0 +1 @@ +Add a Deprecation warning for pytest.ensuretemp as it was deprecated since a while. diff --git a/doc/en/tmpdir.rst b/doc/en/tmpdir.rst index 1c815c8d1..728621152 100644 --- a/doc/en/tmpdir.rst +++ b/doc/en/tmpdir.rst @@ -6,7 +6,7 @@ Temporary directories and files ================================================ The ``tmp_path`` fixture ----------------------- +------------------------ .. versionadded:: 3.9 @@ -15,12 +15,16 @@ You can use the ``tmpdir`` fixture which will provide a temporary directory unique to the test invocation, created in the `base temporary directory`_. -``tmpdir`` is a `pathlib/pathlib2.Path`_ object. Here is an example test usage:: +``tmpdir`` is a ``pathlib/pathlib2.Path`` object. Here is an example test usage: + +.. code-block:: python # content of test_tmp_path.py import os + CONTENT = u"content" + def test_create_file(tmp_path): d = tmp_path / "sub" d.mkdir() @@ -38,8 +42,8 @@ Running this would result in a passed test except for the last -The ``tmp_path_facotry`` fixture ------------------------------- +The ``tmp_path_factory`` fixture +-------------------------------- .. versionadded:: 3.9 @@ -47,7 +51,7 @@ The ``tmp_path_facotry`` fixture The ``tmp_path_facotry`` is a session-scoped fixture which can be used to create arbitrary temporary directories from any other fixture or test. -its intended to replace ``tmpdir_factory`` +its intended to replace ``tmpdir_factory`` and returns :class:`pathlib.Path` instances. The 'tmpdir' fixture diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index e86994b8e..1963f14c0 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -25,8 +25,8 @@ class TempPathFactory(object): The base directory can be configured using the ``--basetemp`` option.""" - given_basetemp = attr.ib() - trace = attr.ib() + _given_basetemp = attr.ib() + _trace = attr.ib() _basetemp = attr.ib(default=None) @classmethod @@ -45,14 +45,14 @@ class TempPathFactory(object): p.mkdir() else: p = make_numbered_dir(root=self.getbasetemp(), prefix=basename) - self.trace("mktemp", p) + self._trace("mktemp", p) return p def getbasetemp(self): """ return base temporary directory. """ if self._basetemp is None: - if self.given_basetemp is not None: - basetemp = Path(self.given_basetemp) + if self._given_basetemp is not None: + basetemp = Path(self._given_basetemp) ensure_reset_dir(basetemp) else: from_env = os.environ.get("PYTEST_DEBUG_TEMPROOT") @@ -67,7 +67,7 @@ class TempPathFactory(object): ) assert basetemp is not None self._basetemp = t = basetemp - self.trace("new basetemp", t) + self._trace("new basetemp", t) return t else: return self._basetemp