-
Notifications
You must be signed in to change notification settings - Fork 32
feat: add cleanup routines for files in working directory #262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
nakul680
wants to merge
7
commits into
faccts:main
Choose a base branch
from
nakul680:feat/258-add-cleanup-functions
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+171
−1
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
afa8d1c
feat: add cleanup routines for files in working directory
nakul680 1da28e1
fix: add check for basename
nakul680 4653a12
fix:add to changelog
nakul680 f11bd77
fix: add truthiness checks for basename and checks for files to be de…
nakul680 c56df61
Merge branch 'main' into feat/258-add-cleanup-functions
nakul680 033a54b
fix: add safeguard against relative paths when specifying basenames f…
nakul680 ec8f2bb
fix: add unit tests for cleanup functions
nakul680 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| from opi.output.core import Output | ||
|
|
||
| """ | ||
| Unit tests for Output cleanup functions. | ||
|
|
||
| This module contains tests for the file-deletion helpers: | ||
| - `Output.cleanup_files()` | ||
| - `Output.cleanup_temp_files()` | ||
| - `Output._delete_files()` | ||
| """ | ||
|
|
||
|
|
||
| def _make_output(basename: str, working_dir: Path) -> Output: | ||
| output = Output(basename, working_dir=working_dir, version_check=False) | ||
| return output | ||
|
|
||
|
|
||
| def _create_file(directory: Path, *names: str) -> None: | ||
| for name in names: | ||
| (directory / name).touch() | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| @pytest.mark.output | ||
| def test_cleanup_files_deletes_matching_basename_files(tmp_path: Path): | ||
| """`cleanup_files()` deletes all files matching the basename, including files whose | ||
| name only contains the basename as a prefix, and leaves unrelated files untouched.""" | ||
| _create_file(tmp_path, "job.out", "job.gbw", "job_1.gbw", "unrelated.txt") | ||
| output = _make_output("job", tmp_path) | ||
|
|
||
| output.cleanup_files() | ||
|
|
||
| remaining = {p.name for p in tmp_path.iterdir()} | ||
| assert remaining == {"unrelated.txt"} | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| @pytest.mark.output | ||
| def test_cleanup_files_explicit_basename_overrides_self_basename(tmp_path: Path): | ||
| """`cleanup_files(basename=...)` deletes files for the given basename, not `self.basename`.""" | ||
| _create_file(tmp_path, "job_1.out", "job_2.out") | ||
| output = _make_output("job_1", tmp_path) | ||
|
|
||
| output.cleanup_files(basename="job_2") | ||
|
|
||
| remaining = {p.name for p in tmp_path.iterdir()} | ||
| assert remaining == {"job_1.out"} | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| @pytest.mark.output | ||
| def test_cleanup_files_raises_without_basename(tmp_path: Path): | ||
| """`cleanup_files()` raises `ValueError` rather than deleting everything in `working_dir`.""" | ||
| _create_file(tmp_path, "unrelated.txt") | ||
| output = _make_output("", tmp_path) | ||
|
|
||
| with pytest.raises(ValueError, match="No basename specified"): | ||
| output.cleanup_files() | ||
|
|
||
| assert (tmp_path / "unrelated.txt").exists() | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| @pytest.mark.output | ||
| @pytest.mark.parametrize("basename", ["../secret", "sub/job", "sub\\job"]) | ||
| def test_cleanup_files_rejects_path_separators_in_basename(tmp_path: Path, basename: str): | ||
| """A basename containing a path separator must be rejected rather than letting the | ||
| glob pattern escape `working_dir`.""" | ||
| working_dir = tmp_path / "workdir" | ||
| working_dir.mkdir() | ||
| _create_file(tmp_path, "secret_file.txt") | ||
| output = _make_output(basename, working_dir) | ||
|
|
||
| with pytest.raises(ValueError, match="path separators"): | ||
| output.cleanup_files() | ||
|
|
||
| assert (tmp_path / "secret_file.txt").exists() | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| @pytest.mark.output | ||
| def test_cleanup_temp_files_deletes_only_temp_suffixes(tmp_path: Path): | ||
| """`cleanup_temp_files()` only deletes `.tmp.*`/`.proc.*` files, not other job files.""" | ||
| _create_file(tmp_path, "job.out", "job.gbw", "job.tmp", "job.proc", "job_scan.tmp") | ||
| output = _make_output("job", tmp_path) | ||
|
|
||
| output.cleanup_temp_files() | ||
|
|
||
| remaining = {p.name for p in tmp_path.iterdir()} | ||
| assert remaining == {"job.out", "job.gbw"} | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| @pytest.mark.output | ||
| def test_delete_files_empty_suffixes_deletes_all(tmp_path: Path): | ||
| """An empty `suffixes` sequence behaves like no filter at all: all files matching | ||
| the basename are deleted, same as omitting `suffixes`.""" | ||
| _create_file(tmp_path, "job.out", "job.gbw", "unrelated.txt") | ||
| output = _make_output("job", tmp_path) | ||
|
|
||
| output._delete_files(None, suffixes=()) | ||
|
|
||
| remaining = {p.name for p in tmp_path.iterdir()} | ||
| assert remaining == {"unrelated.txt"} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.