From 98cfcce85f3592d8cc340d0c65157ae478dfa1a0 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 29 Mar 2024 15:51:51 +0000 Subject: [PATCH] Ensure proper cache initialization before writing Writing cache data is interruptible; this prevents a pathological case where interrupting a cache write can cause the cache directory to never be properly initialized with its supporting files. Unify `Cache.mkdir` with `Cache.set` while I'm here so the former also properly initializes the cache directory. Fixes #12167. --- changelog/12167.trivial.rst | 1 + src/_pytest/cacheprovider.py | 46 +++++++++++++++++++++-------------- testing/test_cacheprovider.py | 22 ++++++++++------- 3 files changed, 42 insertions(+), 27 deletions(-) create mode 100644 changelog/12167.trivial.rst diff --git a/changelog/12167.trivial.rst b/changelog/12167.trivial.rst new file mode 100644 index 00000000000..061425576fc --- /dev/null +++ b/changelog/12167.trivial.rst @@ -0,0 +1 @@ +cache: ensure supporting files (``CACHEDIR.TAG``, ``.gitignore``, etc.) are always created in the cache directory, even in the event of the test session being interrupted. diff --git a/src/_pytest/cacheprovider.py b/src/_pytest/cacheprovider.py index 81703ddac44..ebedd4a6f19 100755 --- a/src/_pytest/cacheprovider.py +++ b/src/_pytest/cacheprovider.py @@ -14,6 +14,7 @@ from typing import List from typing import Optional from typing import Set +from typing import Tuple from typing import Union from .pathlib import resolve_from_str @@ -21,6 +22,7 @@ from .reports import CollectReport from _pytest import nodes from _pytest._io import TerminalWriter +from _pytest.compat import assert_never from _pytest.config import Config from _pytest.config import ExitCode from _pytest.config import hookimpl @@ -123,6 +125,10 @@ def warn(self, fmt: str, *, _ispytest: bool = False, **args: object) -> None: stacklevel=3, ) + def _mkdir(self, path: Path) -> None: + self._ensure_cache_dir_and_supporting_files() + path.mkdir(exist_ok=True, parents=True) + def mkdir(self, name: str) -> Path: """Return a directory path object with the given name. @@ -141,7 +147,7 @@ def mkdir(self, name: str) -> Path: if len(path.parts) > 1: raise ValueError("name is not allowed to contain path separators") res = self._cachedir.joinpath(self._CACHE_PREFIX_DIRS, path) - res.mkdir(exist_ok=True, parents=True) + self._mkdir(res) return res def _getvaluepath(self, key: str) -> Path: @@ -178,19 +184,13 @@ def set(self, key: str, value: object) -> None: """ path = self._getvaluepath(key) try: - if path.parent.is_dir(): - cache_dir_exists_already = True - else: - cache_dir_exists_already = self._cachedir.exists() - path.parent.mkdir(exist_ok=True, parents=True) + self._mkdir(path.parent) except OSError as exc: self.warn( f"could not create cache path {path}: {exc}", _ispytest=True, ) return - if not cache_dir_exists_already: - self._ensure_supporting_files() data = json.dumps(value, ensure_ascii=False, indent=2) try: f = path.open("w", encoding="UTF-8") @@ -203,17 +203,27 @@ def set(self, key: str, value: object) -> None: with f: f.write(data) - def _ensure_supporting_files(self) -> None: - """Create supporting files in the cache dir that are not really part of the cache.""" - readme_path = self._cachedir / "README.md" - readme_path.write_text(README_CONTENT, encoding="UTF-8") - - gitignore_path = self._cachedir.joinpath(".gitignore") - msg = "# Created by pytest automatically.\n*\n" - gitignore_path.write_text(msg, encoding="UTF-8") + def _ensure_cache_dir_and_supporting_files(self) -> None: + """Create the cache dir and its supporting files.""" + self._cachedir.mkdir(exist_ok=True, parents=True) - cachedir_tag_path = self._cachedir.joinpath("CACHEDIR.TAG") - cachedir_tag_path.write_bytes(CACHEDIR_TAG_CONTENT) + files: Iterable[Tuple[str, Union[str, bytes]]] = ( + ("README.md", README_CONTENT), + (".gitignore", "# Created by pytest automatically.\n*\n"), + ("CACHEDIR.TAG", CACHEDIR_TAG_CONTENT), + ) + for file, content in files: + if isinstance(content, str): + mode = "xt" + elif isinstance(content, bytes): + mode = "xb" + else: + assert_never(content) + try: + with open(self._cachedir.joinpath(file), mode, encoding="UTF-8") as f: + f.write(content) + except FileExistsError: + pass class LFPluginCollWrapper: diff --git a/testing/test_cacheprovider.py b/testing/test_cacheprovider.py index c020b77f978..c7b719919c7 100644 --- a/testing/test_cacheprovider.py +++ b/testing/test_cacheprovider.py @@ -1263,12 +1263,7 @@ def test_gitignore(pytester: Pytester) -> None: cache.set("foo", "bar") msg = "# Created by pytest automatically.\n*\n" gitignore_path = cache._cachedir.joinpath(".gitignore") - assert gitignore_path.read_text(encoding="UTF-8") == msg - - # Does not overwrite existing/custom one. - gitignore_path.write_text("custom", encoding="utf-8") - cache.set("something", "else") - assert gitignore_path.read_text(encoding="UTF-8") == "custom" + assert gitignore_path.read_text(encoding="utf-8") == msg def test_preserve_keys_order(pytester: Pytester) -> None: @@ -1282,9 +1277,15 @@ def test_preserve_keys_order(pytester: Pytester) -> None: assert list(read_back.items()) == [("z", 1), ("b", 2), ("a", 3), ("d", 10)] -def test_does_not_create_boilerplate_in_existing_dirs(pytester: Pytester) -> None: +def test_does_not_overwrite_with_boilerplate(pytester: Pytester) -> None: from _pytest.cacheprovider import Cache + files = ["README.md", ".gitignore"] + + for filename in files: + with open(filename, "w", encoding="utf-8") as f: + f.write(filename) + pytester.makeini( """ [pytest] @@ -1296,8 +1297,11 @@ def test_does_not_create_boilerplate_in_existing_dirs(pytester: Pytester) -> Non cache.set("foo", "bar") assert os.path.isdir("v") # cache contents - assert not os.path.exists(".gitignore") - assert not os.path.exists("README.md") + + # Original files already existed so left unchanged. + for filename in files: + with open(filename, encoding="utf-8") as f: + assert f.read() == filename def test_cachedir_tag(pytester: Pytester) -> None: