Skip to content

Commit 37328d5

Browse files
authored
Broaden safe read text caught exception scope (#3705)
## Changes Broaden safe read text exception scope to be more safe when reading a text file. Also the tests are extended. ### Linked issues Relates to #3703 Relates to #3704 ### Functionality - [x] Changes in the code linting parts ### Tests - [x] added unit tests
1 parent bb0c232 commit 37328d5

File tree

2 files changed

+42
-1
lines changed

2 files changed

+42
-1
lines changed

src/databricks/labs/ucx/source_code/base.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ def safe_read_text(path: Path, size: int = -1) -> str | None:
408408
"""
409409
try:
410410
return read_text(path, size=size)
411-
except (FileNotFoundError, UnicodeDecodeError, PermissionError) as e:
411+
except (OSError, UnicodeError) as e:
412412
logger.warning(f"Could not read file: {path}", exc_info=e)
413413
return None
414414

tests/unit/source_code/test_base.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
back_up_path,
1717
revert_back_up_path,
1818
safe_write_text,
19+
safe_read_text,
1920
write_text,
2021
)
2122
from databricks.labs.ucx.source_code.linters.base import Fixer
@@ -109,6 +110,46 @@ def apply(self, code) -> str:
109110
assert not fixer.is_supported("other-code")
110111

111112

113+
def test_safe_read_text(tmp_path) -> None:
114+
"""Should read the contents of a file."""
115+
path = tmp_path / "file.txt"
116+
path.write_text("contents")
117+
118+
contents = safe_read_text(path)
119+
120+
assert contents == "contents"
121+
122+
123+
def test_safe_read_text_handles_non_existing_file() -> None:
124+
"""Should return None when the file does not exist."""
125+
contents = safe_read_text(Path("non-existing-file.txt"))
126+
assert contents is None
127+
128+
129+
def test_safe_read_text_handles_directory(tmp_path) -> None:
130+
"""Should return None when the path is a directory."""
131+
contents = safe_read_text(tmp_path)
132+
assert contents is None
133+
134+
135+
@pytest.mark.parametrize(
136+
"os_error",
137+
[
138+
PermissionError("Permission denied"),
139+
FileNotFoundError("File not found"),
140+
UnicodeDecodeError("utf-8", b"\x80\x81\x82", 0, 1, "invalid start byte"),
141+
],
142+
)
143+
def test_safe_read_text_handles_os_errors(os_error: OSError) -> None:
144+
"""Should return None when an OSError occurs."""
145+
path = create_autospec(Path)
146+
path.open.side_effect = os_error
147+
148+
contents = safe_read_text(path)
149+
150+
assert contents is None
151+
152+
112153
def test_write_text_to_non_existing_file(tmp_path) -> None:
113154
path = tmp_path / "file.txt"
114155

0 commit comments

Comments
 (0)