From 15fd4dcac43e51a3d337dc457d1be5feb3ab3aa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Tue, 18 Apr 2023 09:05:12 -0700 Subject: [PATCH] PR feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bernát Gábor --- .pre-commit-config.yaml | 2 +- pyproject.toml | 4 ++-- src/filelock/_soft.py | 4 ++-- src/filelock/_util.py | 8 ++++---- src/filelock/_windows.py | 4 ++-- tests/test_filelock.py | 42 ++++++++++++++++++---------------------- whitelist.txt | 1 - 7 files changed, 30 insertions(+), 35 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 434ae2d..22d7030 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -54,7 +54,7 @@ repos: - id: flake8 additional_dependencies: - flake8-bugbear==23.3.23 - - flake8-comprehensions==3.11.1 + - flake8-comprehensions==3.12 - flake8-pytest-style==1.7.2 - flake8-spellcheck==0.28 - flake8-unused-arguments==0.0.13 diff --git a/pyproject.toml b/pyproject.toml index 7099566..ccb2ca1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,13 +37,13 @@ dynamic = [ optional-dependencies.docs = [ "furo>=2023.3.27", "sphinx>=6.1.3", - "sphinx-autodoc-typehints!=1.23.4,>=1.22", + "sphinx-autodoc-typehints!=1.23.4,>=1.23", ] optional-dependencies.testing = [ "covdefaults>=2.3", "coverage>=7.2.3", "diff-cover>=7.5", - "pytest>=7.2.2", + "pytest>=7.3.1", "pytest-cov>=4", "pytest-mock>=3.10", "pytest-timeout>=2.1", diff --git a/src/filelock/_soft.py b/src/filelock/_soft.py index 6ab891d..1a14df7 100644 --- a/src/filelock/_soft.py +++ b/src/filelock/_soft.py @@ -5,14 +5,14 @@ from errno import EACCES, EEXIST from ._api import BaseFileLock -from ._util import raise_unwritable_file +from ._util import raise_on_not_writable_file class SoftFileLock(BaseFileLock): """Simply watches the existence of the lock file.""" def _acquire(self) -> None: - raise_unwritable_file(self._lock_file) + raise_on_not_writable_file(self._lock_file) # first check for exists and read-only mode as the open will mask this case as EEXIST flags = ( os.O_WRONLY # open for writing only diff --git a/src/filelock/_util.py b/src/filelock/_util.py index 574f034..81cbeaf 100644 --- a/src/filelock/_util.py +++ b/src/filelock/_util.py @@ -6,7 +6,7 @@ from errno import EACCES, EISDIR -def raise_unwritable_file(filename: str) -> None: +def raise_on_not_writable_file(filename: str) -> None: """ Raise an exception if attempting to open the file for writing would fail. This is done so files that will never be writable can be separated from @@ -24,14 +24,14 @@ def raise_unwritable_file(filename: str) -> None: raise PermissionError(EACCES, "Permission denied", filename) if stat.S_ISDIR(file_stat.st_mode): - if sys.platform == "win32": + if sys.platform == "win32": # pragma: win32 cover # On Windows, this is PermissionError raise PermissionError(EACCES, "Permission denied", filename) - else: + else: # pragma: win32 no cover # On linux / macOS, this is IsADirectoryError raise IsADirectoryError(EISDIR, "Is a directory", filename) __all__ = [ - "raise_unwritable_file", + "raise_on_not_writable_file", ] diff --git a/src/filelock/_windows.py b/src/filelock/_windows.py index e3fe2cc..67dd194 100644 --- a/src/filelock/_windows.py +++ b/src/filelock/_windows.py @@ -6,7 +6,7 @@ from typing import cast from ._api import BaseFileLock -from ._util import raise_unwritable_file +from ._util import raise_on_not_writable_file if sys.platform == "win32": # pragma: win32 cover import msvcrt @@ -15,7 +15,7 @@ class WindowsFileLock(BaseFileLock): """Uses the :func:`msvcrt.locking` function to hard lock the lock file on windows systems.""" def _acquire(self) -> None: - raise_unwritable_file(self._lock_file) + raise_on_not_writable_file(self._lock_file) flags = ( os.O_RDWR # open for read and write | os.O_CREAT # create file if not exists diff --git a/tests/test_filelock.py b/tests/test_filelock.py index 3896dc8..2b94031 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -112,32 +112,28 @@ def test_ro_file(lock_type: type[BaseFileLock], tmp_file_ro: Path) -> None: lock.acquire() -if sys.platform == "win32": - # filenames that will raise errors - bad_lock_file_errors: list[tuple[type[Exception], str, str]] = [ - (FileNotFoundError, "No such file or directory:", "a/b"), # non-existent directory - (FileNotFoundError, "No such file or directory:", ""), # blank filename - (PermissionError, "Permission denied:", "."), # current directory - (PermissionError, "Permission denied:", "/"), # root directory - (OSError, "Invalid argument", '<>:"/\\|?*\a'), # invalid characters - (ValueError, "embedded null (byte|character)", "\0"), # null character - ] -else: - # filenames that will raise errors - bad_lock_file_errors: list[tuple[type[Exception], str, str]] = [ - (FileNotFoundError, "No such file or directory:", "a/b"), # non-existent directory - (FileNotFoundError, "No such file or directory:", ""), # blank filename - (IsADirectoryError, "Is a directory", "."), # current directory - (IsADirectoryError, "Is a directory", "/"), # root directory - (ValueError, "embedded null (byte|character)", "\0"), # null byte - ] - - @pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock]) @pytest.mark.parametrize( ("expected_error", "match", "bad_lock_file"), - bad_lock_file_errors, - ids=[e[0].__name__ for e in bad_lock_file_errors], + [ + pytest.param(FileNotFoundError, "No such file or directory:", "a/b", id="non_existent_directory"), + pytest.param(FileNotFoundError, "No such file or directory:", "", id="blank_filename"), + pytest.param(ValueError, "embedded null (byte|character)", "\0", id="null_byte"), + pytest.param( + PermissionError if sys.platform == "win32" else IsADirectoryError, + "Permission denied:" if sys.platform == "win32" else "Is a directory", + ".", + id="current_directory", + ), + ] + + ( + ( + [pytest.param(OSError, "Invalid argument", i, id=f"invalid_{i}") for i in '<>:"|?*\a'] + + [pytest.param(PermissionError, "Permission denied:", i, id=f"invalid_{i}") for i in "/\\"] + ) + if sys.platform == "win32" + else [] + ), ) @pytest.mark.timeout(5) # timeout in case of infinite loop def test_bad_lock_file( diff --git a/whitelist.txt b/whitelist.txt index 5956053..05ed87f 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -36,5 +36,4 @@ stacklevel trunc typehints unlck -unwritable win32