Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>
  • Loading branch information
gaborbernat committed Apr 18, 2023
1 parent 8d297c9 commit 15fd4dc
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 35 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions src/filelock/_soft.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/filelock/_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
]
4 changes: 2 additions & 2 deletions src/filelock/_windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
42 changes: 19 additions & 23 deletions tests/test_filelock.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 0 additions & 1 deletion whitelist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,4 @@ stacklevel
trunc
typehints
unlck
unwritable
win32

0 comments on commit 15fd4dc

Please sign in to comment.