Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance: do not reload tempfile #1076

Merged
merged 1 commit into from
Oct 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ The released versions correspond to PyPI releases.

## Unreleased

### Performance
* avoid reloading `tempfile` in Posix systems

### Infrastructure
* use trusted publisher for release (see https://docs.pypi.org/trusted-publishers/)

Expand All @@ -33,7 +36,9 @@ Adds official Python 3.13 support, improves OS emulation behavior.
* the `additional_skip_names` parameter now works with more modules (see [#1023](../../issues/1023))
* added support for `os.fchmod`, allow file descriptor argument for `os.chmod` only for POSIX
for Python < 3.13
* avoid reloading `glob` in Python 3.13 (did affect test performance)

### Performance
* avoid reloading `glob` in Python 3.13

### Fixes
* removing files while iterating over `scandir` results is now possible (see [#1051](../../issues/1051))
Expand Down Expand Up @@ -574,6 +579,8 @@ release.
default to avoid a large performance impact. An additional parameter
`patch_default_args` has been added that switches this behavior on
(see [#567](../../issues/567)).

### Performance
* Added performance improvements in the test setup, including caching the
unpatched modules

Expand Down
43 changes: 40 additions & 3 deletions pyfakefs/fake_filesystem_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,41 @@
PATH_MODULE = "ntpath" if sys.platform == "win32" else "posixpath"


class TempfilePatcher:
"""Handles tempfile patching for Posix systems."""

def __init__(self):
self.tempfile_cleanup = None

def start_patching(self):
if self.tempfile_cleanup is not None:
return
if sys.version_info >= (3, 12):

def cleanup(self_, windows=(os.name == "nt"), unlink=None):
self.tempfile_cleanup(self_, windows, unlink or os.unlink)

self.tempfile_cleanup = tempfile._TemporaryFileCloser.cleanup # type: ignore[module-attr]
tempfile._TemporaryFileCloser.cleanup = cleanup # type: ignore[module-attr]
elif sys.platform != "win32":

def close(self_, unlink=None):
self.tempfile_cleanup(self_, unlink or os.unlink)

self.tempfile_cleanup = tempfile._TemporaryFileCloser.close # type: ignore[module-attr]
tempfile._TemporaryFileCloser.close = close # type: ignore[module-attr]

def stop_patching(self):
if self.tempfile_cleanup is None:
return
if sys.version_info < (3, 12):
tempfile._TemporaryFileCloser.close = self.tempfile_cleanup # type: ignore[module-attr]
else:
tempfile._TemporaryFileCloser.cleanup = self.tempfile_cleanup # type: ignore[module-attr]
# reset the cached tempdir in tempfile
tempfile.tempdir = None


def patchfs(
_func: Optional[Callable] = None,
*,
Expand Down Expand Up @@ -576,6 +611,7 @@ def __init__(
self.patch_open_code = patch_open_code
self.linecache_updatecache = None
self.linecache_checkcache = None
self.tempfile_patcher = TempfilePatcher()

if additional_skip_names is not None:
skip_names = [
Expand All @@ -590,9 +626,7 @@ def __init__(
self._init_fake_module_classes()

# reload tempfile under posix to patch default argument
self.modules_to_reload: List[ModuleType] = (
[] if sys.platform == "win32" else [tempfile]
)
self.modules_to_reload: List[ModuleType] = []
if modules_to_reload is not None:
self.modules_to_reload.extend(modules_to_reload)
self.patch_default_args = patch_default_args
Expand Down Expand Up @@ -977,6 +1011,8 @@ def start_patching(self) -> None:
self.linecache_checkcache = linecache.checkcache
linecache.checkcache = self.checkcache

self.tempfile_patcher.start_patching()

self.patch_modules()
self.patch_functions()
self.patch_defaults()
Expand Down Expand Up @@ -1075,6 +1111,7 @@ def stop_patching(self, temporary=False) -> None:
if self.use_dynamic_patch and self._dyn_patcher:
self._dyn_patcher.cleanup()
sys.meta_path.pop(0)
self.tempfile_patcher.stop_patching()
if self.linecache_updatecache is not None:
linecache.updatecache = self.linecache_updatecache
linecache.checkcache = self.linecache_checkcache
Expand Down
41 changes: 18 additions & 23 deletions pyfakefs/tests/fake_filesystem_unittest_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -541,25 +541,20 @@ def test_non_root_behavior(self):

class PauseResumeTest(fake_filesystem_unittest.TestCase):
def setUp(self):
self.real_temp_file = None
self.setUpPyfakefs()

def tearDown(self):
if self.real_temp_file is not None:
self.real_temp_file.close()

def test_pause_resume(self):
fake_temp_file = tempfile.NamedTemporaryFile()
self.assertTrue(self.fs.exists(fake_temp_file.name))
self.assertTrue(os.path.exists(fake_temp_file.name))
self.pause()
self.assertTrue(self.fs.exists(fake_temp_file.name))
self.assertFalse(os.path.exists(fake_temp_file.name))
self.real_temp_file = tempfile.NamedTemporaryFile()
self.assertFalse(self.fs.exists(self.real_temp_file.name))
self.assertTrue(os.path.exists(self.real_temp_file.name))
real_temp_file = tempfile.NamedTemporaryFile()
self.assertFalse(self.fs.exists(real_temp_file.name))
self.assertTrue(os.path.exists(real_temp_file.name))
self.resume()
self.assertFalse(os.path.exists(self.real_temp_file.name))
self.assertFalse(os.path.exists(real_temp_file.name))
self.assertTrue(os.path.exists(fake_temp_file.name))

def test_pause_resume_fs(self):
Expand All @@ -572,15 +567,15 @@ def test_pause_resume_fs(self):
self.fs.pause()
self.assertTrue(self.fs.exists(fake_temp_file.name))
self.assertFalse(os.path.exists(fake_temp_file.name))
self.real_temp_file = tempfile.NamedTemporaryFile()
self.assertFalse(self.fs.exists(self.real_temp_file.name))
self.assertTrue(os.path.exists(self.real_temp_file.name))
real_temp_file = tempfile.NamedTemporaryFile()
self.assertFalse(self.fs.exists(real_temp_file.name))
self.assertTrue(os.path.exists(real_temp_file.name))
# pause does nothing if already paused
self.fs.pause()
self.assertFalse(self.fs.exists(self.real_temp_file.name))
self.assertTrue(os.path.exists(self.real_temp_file.name))
self.assertFalse(self.fs.exists(real_temp_file.name))
self.assertTrue(os.path.exists(real_temp_file.name))
self.fs.resume()
self.assertFalse(os.path.exists(self.real_temp_file.name))
self.assertFalse(os.path.exists(real_temp_file.name))
self.assertTrue(os.path.exists(fake_temp_file.name))

def test_pause_resume_contextmanager(self):
Expand All @@ -590,10 +585,10 @@ def test_pause_resume_contextmanager(self):
with Pause(self):
self.assertTrue(self.fs.exists(fake_temp_file.name))
self.assertFalse(os.path.exists(fake_temp_file.name))
self.real_temp_file = tempfile.NamedTemporaryFile()
self.assertFalse(self.fs.exists(self.real_temp_file.name))
self.assertTrue(os.path.exists(self.real_temp_file.name))
self.assertFalse(os.path.exists(self.real_temp_file.name))
real_temp_file = tempfile.NamedTemporaryFile()
self.assertFalse(self.fs.exists(real_temp_file.name))
self.assertTrue(os.path.exists(real_temp_file.name))
self.assertFalse(os.path.exists(real_temp_file.name))
self.assertTrue(os.path.exists(fake_temp_file.name))

def test_pause_resume_fs_contextmanager(self):
Expand All @@ -603,10 +598,10 @@ def test_pause_resume_fs_contextmanager(self):
with Pause(self.fs):
self.assertTrue(self.fs.exists(fake_temp_file.name))
self.assertFalse(os.path.exists(fake_temp_file.name))
self.real_temp_file = tempfile.NamedTemporaryFile()
self.assertFalse(self.fs.exists(self.real_temp_file.name))
self.assertTrue(os.path.exists(self.real_temp_file.name))
self.assertFalse(os.path.exists(self.real_temp_file.name))
real_temp_file = tempfile.NamedTemporaryFile()
self.assertFalse(self.fs.exists(real_temp_file.name))
self.assertTrue(os.path.exists(real_temp_file.name))
self.assertFalse(os.path.exists(real_temp_file.name))
self.assertTrue(os.path.exists(fake_temp_file.name))

def test_pause_resume_without_patcher(self):
Expand Down
4 changes: 2 additions & 2 deletions pyfakefs/tests/performance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ class TimePerformanceTest(TestCase):
"""

def test_cached_time(self):
self.assertLess(SetupPerformanceTest.elapsed_time, 0.4)
self.assertLess(SetupPerformanceTest.elapsed_time, 0.18)

def test_uncached_time(self):
self.assertLess(SetupNoCachePerformanceTest.elapsed_time, 6)
self.assertLess(SetupNoCachePerformanceTest.elapsed_time, 4)

def test_setup(self):
pass
Expand Down