Repository navigation
Cover the code that deletes, and make the suite runnable - #5
Merged
Merged
Conversation
Reclaimer is the only place in the app that removes anything, and it was the only service without a test. These pin the behaviour the UI promises: emptyDirectories keeps the directory itself, prune keeps files newer than the cutoff and removes only the directories it emptied, steps stops at the first failure instead of carrying on down the list, and a delete of a path the user already removed by hand is a no-op rather than an error that would stop a batch halfway. Everything happens inside a temporary directory the suite creates and removes; the whole suite runs in 6 ms.
itemsAreWellFormed and jsonInventoryIsWellFormed each walk $HOME, /Library, /private/var, /opt, /usr/local and /Users/Shared and measure every file under them. On a developer Mac that is over 20 minutes for the pair, with no output until they finish, so `swift test` is not something anyone runs before a commit. Put them behind SYSDATA_SCAN_TESTS and give them a time limit. The remaining suite is 9 tests in 8 ms; the inventory is still one variable away, and a CI runner starts empty enough to enable it there.
Jarvis322
added a commit
that referenced
this pull request
Sep 6, 2026
Making the scan opt-in in #5 was right — 21 tests in ten milliseconds is a suite people will actually run — but it left nothing running the scan automatically. The release script is the one place it has to happen, and the comment saying CI enables them was describing a workflow that does not.
Owner
|
Merged. 21 tests in ten milliseconds is a suite people will actually run before committing, which the old one was not. One thing your change left open, fixed in a follow-up: nothing ran the disk-walking tests automatically any more. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Two changes to the test target.
Reclaimerhad no test. It is the only code in the app that removes anything, so this pins the behaviour the UI promises:emptyDirectorieskeeps the directory itself,prunekeeps files newer than the cutoff and removes only the directories it emptied,stepsstops at the first failure instead of carrying on down the list, and deleting a path the user already removed by hand is a no-op rather than an error that would stop a batch halfway. Everything happens inside a temporary directory the suite creates and removes.The machine-wide scan is now opt-in.
itemsAreWellFormedandjsonInventoryIsWellFormedeach walk the whole disk and measure every file. On a developer Mac that is over 20 minutes for the pair, with no output until they finish, which is not something anyone runs before a commit — I could not get either to complete while working on this.They are worth keeping, so they stay one variable away:
SYSDATA_SCAN_TESTS=1 swift testand they now carry a time limit. What is left runs in 8 ms, so
swift testis worth running again.Happy to split these if you would rather take one.