Skip to content

Commit d74c104

Browse files
EliahKaganclaude
andcommitted
test(util): cover rmfile deleting a read-only file
`git.util.rmfile` exists so that a read-only file can be deleted on Windows, where `os.remove` fails on files with the read-only attribute. On Windows it calls `os.chmod` to clear that attribute before removing the file. Nothing tested this. Before this change, the only callers of `rmfile` were `LockFile._release_lock` and a lock-file helper in `test/test_repo.py`, and no test called either of them on a read-only file. Add `TestRmfile.test_deletes_readonly_file`, alongside the analogous `TestRmtree.test_deletes_dir_with_readonly_files`. Its initial state comes from a new fixture script, `tests/fixtures/rmfile.sh`. The script creates a file named `readonly` and removes its write permissions with `chmod a-w`, as `tests/fixtures/cleanup.sh` does for its read-only case. On Windows this should give the file the read-only attribute. The test uses a copy of the cached fixture output, which keeps the file's mode, so it needs no `execute=True`. The script is separate from `tests/fixtures/rmtree.sh`, whose cases are keyed by `TestRmtree` test names through the `tmp_path` fixture in `test/test_util.py`, and which that fixture runs in place for all but one of them. `rmfile` silently does nothing if the path is not a file, so the test would pass vacuously if the fixture made no file. It therefore first checks that the file exists and is not owner-writable, and only then that `rmfile` removes it. On Windows, `os.stat` reports the read-only attribute as the absence of write bits, so this also confirms that the fixture produced the condition the test is about, rather than assuming it. The test runs on every platform, but it is only meaningful on native Windows. Unlike the `rmtree` test, it is not skipped on Cygwin. That skip's reason, that Cygwin can't set the needed permissions, hasn't been checked for this test. On Cygwin `sys.platform` is `"cygwin"`, so `rmfile` uses no special handling there, and the test only checks that a read-only file can be removed, as on Unix. This adds coverage ahead of changing the mode `rmfile` passes to `os.chmod`: the test passes with the current `0o777`, and on Windows it would fail if the mode lacked the owner-write bit. Validation: on macOS with Python 3.12, the four `TestRmtree` and `TestRmfile` tests pass, the copied fixture file has mode `0o444`, and the precondition fails when the path does not exist. `shellcheck` passes on the new script. Neither Windows nor Cygwin was run locally. The `import-cygwin` workflow does not run the test suite, so Cygwin is not tested in CI either. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 38992bc commit d74c104

2 files changed

Lines changed: 20 additions & 0 deletions

File tree

‎test/test_util.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
is_cygwin_git,
4242
join_path,
4343
remove_password_if_present,
44+
rmfile,
4445
rmtree,
4546
stream_copy,
4647
)
@@ -213,6 +214,21 @@ def preen_dir2():
213214
assert old_mode == new_mode, f"Should stay {old_mode:#o}, became {new_mode:#o}."
214215

215216

217+
class TestRmfile:
218+
"""Tests for :func:`git.util.rmfile`."""
219+
220+
def test_deletes_readonly_file(self):
221+
# Automatically works on Unix, but requires special handling on Windows.
222+
with writable_fixture("rmfile.sh") as temporary:
223+
file = temporary.path().resolve() / "readonly"
224+
# rmfile ignores missing paths, so check that the fixture made a read-only file.
225+
assert file.is_file()
226+
assert not file.stat().st_mode & stat.S_IWUSR
227+
228+
rmfile(file)
229+
assert not file.exists()
230+
231+
216232
def _xfail_param(*values, **xfail_kwargs):
217233
"""Build a pytest.mark.parametrize parameter that carries an xfail mark."""
218234
return pytest.param(*values, marks=pytest.mark.xfail(**xfail_kwargs))

‎tests/fixtures/rmfile.sh‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#!/usr/bin/env bash
2+
set -eu
3+
touch readonly
4+
chmod a-w readonly

0 commit comments

Comments
 (0)