Skip to content

Commit 07a7ad8

Browse files
committed
fix: robust unlink, fail-fast config validation, symlink tests
- uninstall() wraps path.unlink() in try/except OSError to avoid partial cleanup on race conditions or permission errors - setup() raises ValueError on missing config or folder instead of silently returning empty - Added 3 tests: symlink in check_modified, symlink skip/force in uninstall (47 total)
1 parent dcd93e6 commit 07a7ad8

File tree

3 files changed

+52
-3
lines changed

3 files changed

+52
-3
lines changed

src/specify_cli/integrations/base.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,15 @@ def setup(
114114
return created
115115

116116
if not self.config:
117-
return created
117+
raise ValueError(
118+
f"{type(self).__name__}.config is not set; integration "
119+
"subclasses must define a non-empty 'config' mapping."
120+
)
118121
folder = self.config.get("folder")
119122
if not folder:
120-
return created
123+
raise ValueError(
124+
f"{type(self).__name__}.config is missing required 'folder' entry."
125+
)
121126

122127
project_root_resolved = project_root.resolve()
123128
if manifest.project_root != project_root_resolved:

src/specify_cli/integrations/manifest.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,11 @@ def uninstall(
175175
if not force and _sha256(path) != expected_hash:
176176
skipped.append(path)
177177
continue
178-
path.unlink()
178+
try:
179+
path.unlink()
180+
except OSError:
181+
skipped.append(path)
182+
continue
179183
removed.append(path)
180184
# Clean up empty parent directories up to project root
181185
parent = path.parent

tests/test_integrations.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,16 @@ def test_deleted_file_not_reported(self, tmp_path):
233233
(tmp_path / "f.txt").unlink()
234234
assert m.check_modified() == []
235235

236+
def test_symlink_treated_as_modified(self, tmp_path):
237+
"""A tracked file replaced with a symlink is reported as modified."""
238+
m = IntegrationManifest("test", tmp_path)
239+
m.record_file("f.txt", "original")
240+
target = tmp_path / "target.txt"
241+
target.write_text("target", encoding="utf-8")
242+
(tmp_path / "f.txt").unlink()
243+
(tmp_path / "f.txt").symlink_to(target)
244+
assert m.check_modified() == ["f.txt"]
245+
236246

237247
class TestManifestUninstall:
238248
def test_removes_unmodified(self, tmp_path):
@@ -310,6 +320,36 @@ def test_preserves_nonempty_parent_dirs(self, tmp_path):
310320
assert (tmp_path / "a" / "b" / "other.txt").exists()
311321
assert (tmp_path / "a" / "b").is_dir()
312322

323+
def test_symlink_skipped_without_force(self, tmp_path):
324+
"""A tracked file replaced with a symlink is skipped unless force."""
325+
m = IntegrationManifest("test", tmp_path)
326+
m.record_file("f.txt", "original")
327+
m.save()
328+
target = tmp_path / "target.txt"
329+
target.write_text("target", encoding="utf-8")
330+
(tmp_path / "f.txt").unlink()
331+
(tmp_path / "f.txt").symlink_to(target)
332+
333+
removed, skipped = m.uninstall()
334+
assert removed == []
335+
assert len(skipped) == 1
336+
assert (tmp_path / "f.txt").is_symlink() # still there
337+
338+
def test_symlink_removed_with_force(self, tmp_path):
339+
"""A tracked file replaced with a symlink is removed with force."""
340+
m = IntegrationManifest("test", tmp_path)
341+
m.record_file("f.txt", "original")
342+
m.save()
343+
target = tmp_path / "target.txt"
344+
target.write_text("target", encoding="utf-8")
345+
(tmp_path / "f.txt").unlink()
346+
(tmp_path / "f.txt").symlink_to(target)
347+
348+
removed, skipped = m.uninstall(force=True)
349+
assert len(removed) == 1
350+
assert not (tmp_path / "f.txt").exists()
351+
assert target.exists() # target not deleted
352+
313353

314354
class TestManifestPersistence:
315355
def test_save_and_load_roundtrip(self, tmp_path):

0 commit comments

Comments
 (0)