Skip to content

Commit 3ed046f

Browse files
committed
feat: improve test coverage from 90% to 96%
Refactor Table class to use FileSystem protocol for better testability: - Add _filesystem.py with FileSystem protocol and RealFileSystem implementation - Inject FileSystem dependency into Table via _fs parameter - Create FakeFileSystem test double for edge case testing Add comprehensive tests for previously uncovered code paths: - Surrogate pair validation in encoding - append_lines function in writer module - Header serialization edge cases - Transaction repr and edge cases - Table filesystem error handling paths Mark legitimately untestable code with pragma comments: - Protocol method stubs (abstract definitions) - Defensive unreachable code paths - Windows-specific lock implementation
1 parent fbc37b8 commit 3ed046f

18 files changed

Lines changed: 893 additions & 222 deletions

pyproject.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ exclude_lines = [
8888
"if TYPE_CHECKING:",
8989
"@abstractmethod",
9090
"@abc.abstractmethod",
91+
# Platform-specific exclusions
92+
"class _WindowsLock:",
9193
]
9294
precision = 2
9395
show_missing = true

src/jsonlt/_filesystem.py

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
"""Filesystem abstraction for JSONLT Table operations.
2+
3+
This module provides a filesystem protocol and implementation used by the Table
4+
class for file operations, enabling testability through dependency injection.
5+
"""
6+
7+
import os
8+
from contextlib import contextmanager
9+
from dataclasses import dataclass
10+
from typing import TYPE_CHECKING, ClassVar, Protocol, cast, runtime_checkable
11+
12+
from ._exceptions import FileError
13+
from ._lock import exclusive_lock
14+
from ._writer import atomic_replace as _atomic_replace
15+
16+
if TYPE_CHECKING:
17+
from collections.abc import Iterator, Sequence
18+
from contextlib import AbstractContextManager
19+
from pathlib import Path
20+
from typing import BinaryIO
21+
22+
23+
@dataclass(frozen=True, slots=True)
24+
class FileStats:
25+
"""Immutable container for file stat results."""
26+
27+
mtime: float
28+
size: int
29+
exists: bool
30+
31+
32+
@runtime_checkable
33+
class LockedFile(Protocol):
34+
"""Protocol for a file handle with exclusive lock held."""
35+
36+
def read(self) -> bytes: # pragma: no cover
37+
"""Read all remaining bytes from the file."""
38+
...
39+
40+
def write(self, data: bytes) -> int: # pragma: no cover
41+
"""Write bytes to the file."""
42+
...
43+
44+
def seek(self, offset: int, whence: int = 0) -> int: # pragma: no cover
45+
"""Seek to a position in the file."""
46+
...
47+
48+
def sync(self) -> None: # pragma: no cover
49+
"""Flush and fsync the file."""
50+
...
51+
52+
53+
@runtime_checkable
54+
class FileSystem(Protocol):
55+
"""Protocol for filesystem operations needed by Table."""
56+
57+
def stat(self, path: "Path") -> FileStats: # pragma: no cover
58+
"""Get file stats. Returns FileStats with exists=False if not found."""
59+
...
60+
61+
def read_bytes(
62+
self, path: "Path", *, max_size: int | None = None
63+
) -> bytes: # pragma: no cover
64+
"""Read entire file contents. Raises FileError if not readable."""
65+
...
66+
67+
def ensure_parent_dir(self, path: "Path") -> None: # pragma: no cover
68+
"""Create parent directories if needed."""
69+
...
70+
71+
def open_locked( # pragma: no cover
72+
self,
73+
path: "Path",
74+
mode: str,
75+
timeout: float | None,
76+
) -> "AbstractContextManager[LockedFile]":
77+
"""Open file with exclusive lock."""
78+
...
79+
80+
def atomic_replace(
81+
self, path: "Path", lines: "Sequence[str]"
82+
) -> None: # pragma: no cover
83+
"""Atomically replace file contents with lines."""
84+
...
85+
86+
87+
class _LockedFileHandle:
88+
"""Wrapper around file handle satisfying LockedFile protocol."""
89+
90+
__slots__: ClassVar[tuple[str, ...]] = ("_file",)
91+
92+
_file: "BinaryIO"
93+
94+
def __init__(self, file: "BinaryIO") -> None:
95+
self._file = file
96+
97+
def read(self) -> bytes:
98+
"""Read all remaining bytes from the file."""
99+
return self._file.read()
100+
101+
def write(self, data: bytes) -> int:
102+
"""Write bytes to the file."""
103+
return self._file.write(data)
104+
105+
def seek(self, offset: int, whence: int = 0) -> int:
106+
"""Seek to a position in the file."""
107+
return self._file.seek(offset, whence)
108+
109+
def sync(self) -> None:
110+
"""Flush and fsync the file."""
111+
self._file.flush()
112+
os.fsync(self._file.fileno())
113+
114+
115+
class RealFileSystem:
116+
"""Real filesystem implementation using standard library."""
117+
118+
__slots__: ClassVar[tuple[str, ...]] = ()
119+
120+
def stat(self, path: "Path") -> FileStats:
121+
"""Get file stats. Returns FileStats with exists=False if not found.
122+
123+
Args:
124+
path: Path to the file.
125+
126+
Returns:
127+
FileStats with file metadata, or exists=False if not found.
128+
129+
Raises:
130+
FileError: If stat fails for reasons other than file not found.
131+
"""
132+
try:
133+
st = path.stat()
134+
return FileStats(mtime=st.st_mtime, size=st.st_size, exists=True)
135+
except FileNotFoundError:
136+
return FileStats(mtime=0.0, size=0, exists=False)
137+
except OSError as e:
138+
msg = f"cannot stat file: {e}"
139+
raise FileError(msg) from e
140+
141+
def read_bytes(self, path: "Path", *, max_size: int | None = None) -> bytes:
142+
"""Read entire file contents.
143+
144+
Args:
145+
path: Path to the file.
146+
max_size: Optional maximum file size to allow. If the file exceeds
147+
this size, FileError is raised.
148+
149+
Returns:
150+
The file contents as bytes.
151+
152+
Raises:
153+
FileError: If the file cannot be read or exceeds max_size.
154+
"""
155+
if max_size is not None:
156+
try:
157+
st = path.stat()
158+
except OSError as e:
159+
msg = f"cannot read file: {e}"
160+
raise FileError(msg) from e
161+
if st.st_size > max_size:
162+
msg = f"file size {st.st_size} exceeds maximum {max_size}"
163+
raise FileError(msg)
164+
try:
165+
return path.read_bytes()
166+
except OSError as e:
167+
msg = f"cannot read file: {e}"
168+
raise FileError(msg) from e
169+
170+
def ensure_parent_dir(self, path: "Path") -> None:
171+
"""Create parent directories if needed.
172+
173+
Args:
174+
path: Path whose parent directory should exist.
175+
176+
Raises:
177+
FileError: If directory creation fails.
178+
"""
179+
try:
180+
path.parent.mkdir(parents=True, exist_ok=True)
181+
except OSError as e:
182+
msg = f"cannot create directory: {e}"
183+
raise FileError(msg) from e
184+
185+
@contextmanager
186+
def open_locked(
187+
self,
188+
path: "Path",
189+
mode: str,
190+
timeout: float | None,
191+
) -> "Iterator[LockedFile]":
192+
"""Open file with exclusive lock.
193+
194+
Args:
195+
path: Path to the file.
196+
mode: File mode ("r+b" or "xb").
197+
timeout: Lock acquisition timeout in seconds, or None for no timeout.
198+
199+
Yields:
200+
A LockedFile handle for reading/writing.
201+
202+
Raises:
203+
FileNotFoundError: If mode is "r+b" and file doesn't exist.
204+
FileExistsError: If mode is "xb" and file already exists.
205+
LockError: If lock cannot be acquired within timeout.
206+
FileError: For other OS-level errors.
207+
"""
208+
try:
209+
file = path.open(mode)
210+
except (FileNotFoundError, FileExistsError):
211+
# Let these propagate for control flow in Table
212+
raise
213+
except OSError as e:
214+
msg = f"cannot open file: {e}"
215+
raise FileError(msg) from e
216+
217+
try:
218+
with exclusive_lock(cast("BinaryIO", file), timeout=timeout):
219+
yield _LockedFileHandle(cast("BinaryIO", file))
220+
finally:
221+
file.close()
222+
223+
def atomic_replace(self, path: "Path", lines: "Sequence[str]") -> None:
224+
"""Atomically replace file contents with lines.
225+
226+
Args:
227+
path: Target file path.
228+
lines: Lines to write (newlines added automatically).
229+
230+
Raises:
231+
FileError: If write, sync, or rename fails.
232+
"""
233+
_atomic_replace(path, lines)

src/jsonlt/_lock.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@
2727
class _LockModule(Protocol):
2828
"""Protocol for platform-specific lock module."""
2929

30-
def acquire(self, fd: int) -> bool:
30+
def acquire(self, fd: int) -> bool: # pragma: no cover
3131
"""Try to acquire exclusive lock."""
3232
...
3333

34-
def release(self, fd: int) -> None:
34+
def release(self, fd: int) -> None: # pragma: no cover
3535
"""Release exclusive lock."""
3636
...
3737

src/jsonlt/_readable.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,15 +183,15 @@ def __iter__(self) -> "Iterator[JSONObject]":
183183
def find(
184184
self,
185185
predicate: "Callable[[JSONObject], bool]",
186-
) -> "list[JSONObject]": ...
186+
) -> "list[JSONObject]": ... # pragma: no cover
187187

188188
@overload
189189
def find(
190190
self,
191191
predicate: "Callable[[JSONObject], bool]",
192192
*,
193193
limit: int,
194-
) -> "list[JSONObject]": ...
194+
) -> "list[JSONObject]": ... # pragma: no cover
195195

196196
def find(
197197
self,

src/jsonlt/_records.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ def _validate_key_field_value(value: object, field: str) -> str | int:
6767
if isinstance(value, str):
6868
return value
6969

70-
msg = f"key field '{field}' has invalid type {type(value).__name__}"
71-
raise InvalidKeyError(msg)
70+
# Defensive fallback - unreachable with valid JSON input
71+
type_name = type(value).__name__ # pragma: no cover
72+
msg = f"key field '{field}' has invalid type {type_name}" # pragma: no cover
73+
raise InvalidKeyError(msg) # pragma: no cover
7274

7375

7476
def validate_record(record: "JSONObject", key_specifier: KeySpecifier) -> None:

0 commit comments

Comments
 (0)