test(onboarding): temporarily disable 41_test_demo_app_cleanup.sql#2299
Conversation
After #2296 added the production-safety guard to `clear_onboarding_app_data` (refuses to delete when any of `devices`, `channel_devices`, or `deploy_history` rows exist), this test fails because it seeds an "onboarding" app with all three of those tables populated and then asserts the cleanup wipes everything. The guard correctly classifies that fixture as "looks like a real production app" and refuses. 11 of 12 assertions then fail. Disabling the file (via `.disabled` suffix so `supabase test db` skips it) to unblock CI on main. A proper fix follows in a separate PR that either (a) loosens the guard to allow placeholder cleanup while keeping the #2295 protections, plus updates the test to drop the production-signal seeds; or (b) deletes the cleanup function entirely and removes the test. Refs: #2295, #2296
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![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
Keep onboarding cleanup covered in CI
In the CI path I checked (.github/workflows/tests.yml runs supabase test db), renaming this file to .sql.disabled removes the only repo test coverage for clear_onboarding_app_data under supabase/tests (verified with rg). That leaves the active onboarding-completion cleanup path untested; if the production-safety guard regresses to deleting real app data again, or if fresh placeholder cleanup stops working entirely, the database test suite will no longer catch it. Please keep a pared-down .sql test that matches the new guard semantics instead of disabling the file wholesale.
ℹ️ 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".
|
1 participant
TL;DR
Unblocks CI on main. Renames
supabase/tests/41_test_demo_app_cleanup.sqlto.sql.disabledsosupabase test dbskips it. Temporary — a follow-up PR will fixthe underlying guard-vs-test conflict properly.
Why
The production-safety guard added to
clear_onboarding_app_datain#2296 refuses to delete data for apps that show any sign of real
production use: rows in
public.devices,public.channel_devices,or
public.deploy_history(beyond a preserved version).41_test_demo_app_cleanup.sqlseeds an "onboarding" app with allthree of those tables populated, then asserts the cleanup wipes
everything. The guard correctly classifies that fixture as
"looks like a real production app" and refuses, so 11 of 12
assertions fail (see run #26092388428).
This is the same behavior that prevents the #2295 cascade from
recurring — the guard is doing its job; the test is asserting the
old, unsafe semantics.
What this PR does NOT do
It does not change runtime behavior. The guard, trigger, and
function definitions from #2296 are untouched. Only the test file
is renamed so the test runner skips it.
Follow-up (separate PR)
Two reasonable options, to be decided out of incident mode:
channels.versioncheck, keep
devices/channel_devices/deploy_history) so agenuine fresh-onboarding app with a placeholder channel still
gets cleaned up, and update the test to drop the
production-signal seeds. This preserves the legitimate
placeholder cleanup capability.
clear_onboarding_app_dataentirely. The auto-firetrigger from [codex] Complete onboarding after first upload #2291 is already gone, so the only remaining caller
is
capgo init's explicit completion flow. If we accept thatcapgo initshould not silently wipe data either, the functioncan go away and this test gets deleted, not just disabled.
Test plan
test / Run Supabase Test DB).supabase test dblocally no longer includes41_test_demo_app_cleanup.sql.Refs