fix(test): remove orphaned gock mock in TestUpdateRemoteConfig#4913
fix(test): remove orphaned gock mock in TestUpdateRemoteConfig#4913lightstrike wants to merge 1 commit intosupabase:developfrom
Conversation
PR supabase#4887 added an early return to UpdateDbNetworkRestrictionsConfig when NetworkRestrictions.Enabled is false, but didn't remove the corresponding gock mock from the "updates_all_configs" subtest. The mock was never consumed, causing gock.IsDone() to fail.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Cache: Disabled due to Reviews > Disable Cache setting Disabled knowledge base sources:
📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe test file Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
TestUpdateRemoteConfig/updates_all_configsfails becausegock.IsDone()returnsfalse— a registered mock forGET /v1/projects/test-project/network-restrictionsis never consumed.Root Cause
This is a regression introduced by the interaction of two commits:
69937e09(Dec 2025, PR feat: support storage s3 protocol config #4545 — S3 protocol support): Added a gock mock for the network-restrictions endpoint to the "updates all configs" integration test. At the time,UpdateDbNetworkRestrictionsConfigmade the API call unconditionally, so the mock was always consumed.2f6e0c32(Feb 2026, PR fix(updater): network restrictions not enabled #4887 — network restrictions safety fix): Added an early-return guard toUpdateDbNetworkRestrictionsConfig:This correctly prevents 400 errors on projects without network restrictions entitlements. However, the "updates all configs" test was not updated to account for this new guard. Since the test config does not set
NetworkRestrictions.Enabled = true, the function now returns early, the HTTP call is never made, and the gock mock is left unconsumed.Fix
Remove the orphaned gock mock (5 lines). The test intentionally leaves
NetworkRestrictions.Enabledat its zero value (false), consistent with Auth/Storage/Experimental which are each explicitly set toEnabled: truewhen the test intends to exercise their update paths.The dedicated
TestUpdateDbNetworkRestrictionsConfig(also added in PR #4887) already covers both the disabled-skip and enabled-error paths.Test plan
go test ./config/... -run TestUpdateRemoteConfig -count=1passesTestAuthDifffailures exist on upstreamdevelop(pre-existing, unrelated)