Removes stale paywall template screenshots if necessary#3595
Conversation
The screenshot upload lane was add-only, so when a paywall is removed, renamed, or its directory name changes, the old android.png lingered forever in paywall-rendering-validation. Track the offering IDs rendered in each run and delete any android.png whose directory is no longer produced, then remove directories that become empty. Only android.png is touched, so screenshots owned by other platforms (ios.png, mac-*.png) are never affected. A guard refuses to prune if nothing was produced, to avoid wiping everything on a failed render. Co-authored-by: joop <joop@revenuecat.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3595 +/- ##
=======================================
Coverage 80.26% 80.26%
=======================================
Files 378 378
Lines 15448 15448
Branches 2143 2143
=======================================
Hits 12400 12400
Misses 2189 2189
Partials 859 859 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AlvaroBrey
left a comment
There was a problem hiding this comment.
Left a comment, but approved anyway!
| target_repository_path = options[:target_repository_path] || UI.user_error!("No target_repository_path provided") | ||
| produced_offering_ids = options[:produced_offering_ids] || UI.user_error!("No produced_offering_ids provided") | ||
|
|
||
| UI.user_error!("No Android screenshots were produced; refusing to prune stale screenshots.") if produced_offering_ids.empty? |
There was a problem hiding this comment.
Hmm, the empty check will not prevent deletion of screenshots if there is a partial failure, but I'm not sure that partial failures are possible in our current setup.
What i mean by this is, if a screenshot fails to generate silently, we would delete the baseline too.
The ideal way to fix this would be to not rely on the generated files, but on the tests themselves.
There was a problem hiding this comment.
Hmm that's a good point... TBH, I don't think partial failures would be possible, as in, if there is an issue running the screenshots at some point, it would fail the whole process, but could be an interesting improvement nonetheless.
There was a problem hiding this comment.
Good point indeed, although the only alternative I can come up with is parsing the Offering IDs from the JSON files before generating the screenshots, to know which IDs we should expect. I'm not sure that's worth the complexity. The output of this lane will be a PR in paywall-rendering-validation, so any deleted screenshots should already be checked there during review.
tonidero
left a comment
There was a problem hiding this comment.
Good point in the previous comment... But yeah, I would say it shouldn't happen so probably ok to merge for now.
| target_repository_path = options[:target_repository_path] || UI.user_error!("No target_repository_path provided") | ||
| produced_offering_ids = options[:produced_offering_ids] || UI.user_error!("No produced_offering_ids provided") | ||
|
|
||
| UI.user_error!("No Android screenshots were produced; refusing to prune stale screenshots.") if produced_offering_ids.empty? |
There was a problem hiding this comment.
Hmm that's a good point... TBH, I don't think partial failures would be possible, as in, if there is an issue running the screenshots at some point, it would fail the whole process, but could be an interesting improvement nonetheless.
Description
We would only ever add new paywall screenshots to paywall-rendering-validation, never remove ones that we deleted from paywall-preview-resources. This PR implements just that.
iOS equivalent: RevenueCat/purchases-ios#6984
Note
Medium Risk
Automated deletion in an external repo could remove valid screenshots if offering IDs fail to parse or the recorder returns an incomplete set; the empty-list guard mitigates total wipe on zero output.
Overview
The automated paywall template screenshot flow now removes stale Android assets from the validation repo when templates disappear from preview resources, matching the iOS behavior.
While moving Paparazzi PNGs into
screenshots/templates/{offering_id}/, the lane collectsproduced_offering_idsand then runsremove_stale_android_template_screenshots, which deletesandroid.png(only) for offering folders not in that set and drops empty directories. If the run produces no screenshots, pruning is skipped so a failed recording cannot wipe the repo.Reviewed by Cursor Bugbot for commit a47a2fb. Bugbot is set up for automated code reviews on this repo. Configure here.