Skip to content

Commit

Permalink
cacheprovider: fix "Directory not empty" crash from cache directory c…
Browse files Browse the repository at this point in the history
…reation

Fix #12381

Test plan:
It's possible to write a deterministic test case for this, but somewhat
of a hassle so I tested it manually. I reproduced by removing existing
`.pytest_cache`, adding a sleep before the rename and running two
pytests. I verified that it doesn't reproduce after the fix.
  • Loading branch information
bluetech committed Jun 2, 2024
1 parent 9802183 commit 17065cb
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 8 deletions.
1 change: 1 addition & 0 deletions changelog/12381.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix possible "Directory not empty" crashes arising from concurent cache dir (``.pytest_cache``) creation. Regressed in pytest 8.2.0.
27 changes: 19 additions & 8 deletions src/_pytest/cacheprovider.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# This plugin was not named "cache" to avoid conflicts with the external
# pytest-cache version.
import dataclasses
import errno
import json
import os
from pathlib import Path
Expand Down Expand Up @@ -227,14 +228,24 @@ def _ensure_cache_dir_and_supporting_files(self) -> None:
with open(path.joinpath("CACHEDIR.TAG"), "xb") as f:
f.write(CACHEDIR_TAG_CONTENT)

path.rename(self._cachedir)
# Create a directory in place of the one we just moved so that `TemporaryDirectory`'s
# cleanup doesn't complain.
#
# TODO: pass ignore_cleanup_errors=True when we no longer support python < 3.10. See
# https://github.com/python/cpython/issues/74168. Note that passing delete=False would
# do the wrong thing in case of errors and isn't supported until python 3.12.
path.mkdir()
try:
path.rename(self._cachedir)
except OSError as e:

Check warning on line 233 in src/_pytest/cacheprovider.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/cacheprovider.py#L233

Added line #L233 was not covered by tests
# If 2 concurrent pytests both race to the rename, the loser
# gets "Directory not empty" from the rename. In this case,
# everything is handled so just continue (while letting the
# temporary directory be cleaned up).
if e.errno != errno.ENOTEMPTY:
raise

Check warning on line 239 in src/_pytest/cacheprovider.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/cacheprovider.py#L239

Added line #L239 was not covered by tests
else:
# Create a directory in place of the one we just moved so that
# `TemporaryDirectory`'s cleanup doesn't complain.
#
# TODO: pass ignore_cleanup_errors=True when we no longer support python < 3.10.
# See https://github.com/python/cpython/issues/74168. Note that passing
# delete=False would do the wrong thing in case of errors and isn't supported
# until python 3.12.
path.mkdir()


class LFPluginCollWrapper:
Expand Down

0 comments on commit 17065cb

Please sign in to comment.