Delete orphaned snapshots via a generated manifest and prefire prune - #173
Delete orphaned snapshots via a generated manifest and prefire prune#173BarredEwe wants to merge 2 commits into
Conversation
Renaming or deleting a #Preview used to leave its recorded snapshot in __Snapshots__ forever. The generator knows the full expected set, so `prefire tests` now writes it to `prefire-snapshots.json` next to the generated tests: one entry per __Snapshots__/<TestFile> folder, resolved through `use_grouped_snapshots`, `split_snapshot_directories` and the `test_target_path` fallback, plus the snapshot names and their sanitized file-name prefixes. `prefire prune` reads the manifest and reports the snapshots nothing claims anymore. Deleting files is irreversible, so it only happens with an explicit --delete (--dry-run wins a conflict), only inside folders the manifest names, only for regular image files, and only for names that belong to no expected snapshot family — device suffixes, parameterized arguments and accessibility variants all keep their preview's snapshots. Folders that also receive PrefireProvider snapshots are skipped, since those names exist only at runtime. `delete_unused_snapshots: true` runs the same cleanup right after generation. Closes #85
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16c4c32844
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| var grouped: [String: [SnapshotManifest.Snapshot]] = [:] | ||
|
|
||
| for preview in result.previews { |
There was a problem hiding this comment.
Build the manifest from both the current result and previously managed directories. Since grouped is populated only while iterating current previews, deleting the last grouped preview produces an empty manifest, while in per-source ungrouped mode deleting a source drops that source's snapshot directory from the manifest. Neither prefire prune nor delete_unused_snapshots will ever scan those directories, so snapshots from exactly these deletion scenarios remain orphaned.
Useful? React with 👍 / 👎.
| SnapshotManifest.Directory( | ||
| path: path, | ||
| snapshots: snapshots.sorted(by: { $0.name < $1.name }), | ||
| complete: !result.hasPreviewProviders |
There was a problem hiding this comment.
Do not set complete solely from the presence of PrefireProvider when options.template is custom. A custom test template can emit image snapshots with additional or transformed test names, but the manifest still contains only parsed #Preview display names; prefire prune --delete or automatic cleanup will therefore classify those actively referenced custom snapshots as orphans and delete them. Custom templates need to supply equivalent manifest metadata or have their directories conservatively marked incomplete.
Useful? React with 👍 / 👎.
| testTarget: nil, | ||
| template: nil, | ||
| sources: [], | ||
| output: output, |
There was a problem hiding this comment.
Resolve the plugin-generated output before defaulting to the current directory. I checked both plugin paths: Plugins/PrefireTestsPlugin/Plugin.swift passes a Generated directory under the plugin work directory as --output (lines 10–20 and 55–66), whereas a standalone default prefire prune passes nil here and consequently searches ./prefire-snapshots.json. Thus the documented plain command cannot find the manifest produced by a normal SPM/Xcode plugin build unless users manually discover and pass the DerivedData/plugin-work path.
Useful? React with 👍 / 👎.
…d plugin paths Three gaps let `prune` either delete files the tests still use or miss the orphans it exists to remove. A custom `template_file_path` renders whatever it likes, including snapshots under names the parser never sees, while the manifest holds only parsed `#Preview` display names. Those folders are now marked incomplete, so a custom template turns pruning off instead of deleting live snapshots. The manifest was built only from the previews of the current run, so deleting the last preview of a source file — or every preview in the project — dropped its folder from the manifest and left its snapshots unreachable forever. Folders of the previous manifest that a run no longer produces are now carried over with nothing expected in them, and never gain completeness they did not already have. To keep that from turning a misconfiguration into a purge, a run that parsed no Swift file at all leaves the previous manifest untouched. The manifest also moves from the generated tests folder to `test_target_path`, next to the snapshots it describes: plugin builds generate into DerivedData, which a standalone `prefire prune` cannot guess. The generated tests folder remains a fallback for setups without `test_target_path` and for the SwiftPM plugin sandbox; both `tests` and `prune` search the same ordered candidates, a failed write is reported rather than fatal, and a stale copy in the other location is removed so it can never be read back.
1 participant
Closes #85
After a preview is renamed or deleted, its PNG stays in
__Snapshots__forever. The generator already knows the full set of expected snapshots, so it can now write it down and a new command can act on it.Manifest
prefire-snapshots.jsonis written next to the generated tests on everyprefire testsrun:{ "version": 1, "snapshotDevices": ["iPhone 15", "iPad"], "directories": [ { "path": "…/Tests/__Snapshots__/PreviewTests.generated", "complete": true, "snapshots": [{ "name": "Auth View", "stem": "Auth-View", "parameterized": false }] } ] }pathis computed exactly as SnapshotTesting computes it, accounting foruse_grouped_snapshots,split_snapshot_directories, and the case where notest_target_pathis set.stemgoes through the same sanitization as swift-snapshot-testing.completeisfalsewhen the sources containPrefireProvidertypes, whose names come frompreviewDisplayNameat runtime and cannot be known statically.Command
Without
--deleteit only reports.delete_unused_snapshots: trueintest_configurationruns the cleanup right after generation.Guards
Deleting a user's files is irreversible, so every step is narrow:
--delete, and--dry-runwins on conflict__Snapshots__attributesOfItem, hidden files and anything non-image are skippedSnapshotPruner.deletere-validates every path, so a stale or hand-edited list cannot reach outside a snapshot folderstem,stem.,stem-), which covers device suffixes,.accessibility, and parameterized previews whose argument is rendered withString(describing:)at runtimecomplete: falseare not cleaned at allThe bias is deliberate: a false negative (keeping a stale file) is acceptable, a false positive is not.
Tests
make test73/0,make test-cli17/0, plus an end-to-end run on a temporary project: generate → manifest → dry run → delete → automatic mode via config.