refactor(supabase_common): share the local stack test configuration - #1640
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change centralizes local Supabase stack URLs, credentials, mail settings, and database settings in ChangesShared local-stack configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Tr00d
approved these changes
Aug 4, 2026
Vinzent03
approved these changes
Aug 4, 2026
Vinzent03
left a comment
Vinzent03
left a comment
Collaborator
There was a problem hiding this comment.
I like that most of them should be usable by normal package users as well for their own testing suites. Unsure if that should/could be documented somewhere such that users are aware of this.
Contributor
Author
Right now the package is marked as internal, but later we can for sure document it and open it up. :) |
The URL, ports and API keys of the local Supabase CLI stack were duplicated across the test suites, the service role key alone in five places. Move them into a testing library of supabase_common, annotated with @VisibleForTesting so they cannot leak into runtime code.
The per package aliases only pointed at the shared constants, so the call sites now reference those. Only the values that are actually derived from them stay local.
spydon
force-pushed
the
refactor/shared-local-stack-config
branch
from
August 5, 2026 07:21
53ce320 to
3c7b3ce
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/gotrue/test/admin_test.dart (1)
14-20: 🗄️ Data Integrity & Integration | 🔵 TrivialRun the GoTrue integration tests sequentially.
These tests reset and mutate shared authentication state. Run
dart test -j 1frompackages/gotrueagainst the local Supabase stack. Runmelos analyzeandmelos formatbefore commit.As per coding guidelines, backend-dependent GoTrue tests must run sequentially with
dart test -j 1, and Dart changes requiremelos analyzeandmelos format.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/gotrue/test/admin_test.dart` around lines 14 - 20, Update the GoTrue integration test configuration around the shared client setup and tests in packages/gotrue so backend-dependent tests are explicitly run sequentially with dart test -j 1. Before committing, run melos analyze and melos format to validate and format the Dart changes.Source: Coding guidelines
packages/postgrest/test/basic_test.dart (1)
17-27: 🗄️ Data Integrity & Integration | 🔵 TrivialRun PostgREST tests sequentially against the shared stack.
These tests mutate shared database tables. Run
dart test -j 1frompackages/postgrestagainst the local Supabase stack.As per coding guidelines, backend-dependent PostgREST tests must run sequentially with
dart test -j 1.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/postgrest/test/basic_test.dart` around lines 17 - 27, Configure the PostgREST test setup around the shared-stack tests in basic_test.dart to run sequentially, ensuring the package test command uses dart test -j 1. Preserve the existing setup and teardown behavior while applying the single-worker requirement to these backend-dependent tests.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/gotrue/test/client_test.dart`:
- Line 17: Update the email-change PKCE test flow in client_test so
_pkceCodeFromEmailChange uses the effective gotrueUrl instead of
localStackAuthUrl. Pass gotrueUrl into that helper and use the same value in the
RegExp.escape prefix so verification link parsing matches the URL used by the
PKCE client and avoids the firstMatch(... )! failure when GOTRUE_URL is set.
In `@packages/storage_client/test/client_test.dart`:
- Line 282: Wrap the long public transformed URL string assertion in the client
test into adjacent string literals so the source stays within the 80-character
limit. Preserve the exact URL value, then run dart format on the Dart file.
In `@packages/supabase_common/lib/src/testing/local_stack.dart`:
- Around line 64-72: In
packages/supabase_common/lib/src/testing/local_stack.dart lines 64-72, split
both JWT constants, localStackAnonKey and localStackServiceRoleKey, into
adjacent string literals so every Dart source line stays within 80 characters.
In packages/realtime_client/test/utils/realtime_test_utils.dart line 23, wrap
the documentation comment to the same limit. Run dart format afterward.
---
Nitpick comments:
In `@packages/gotrue/test/admin_test.dart`:
- Around line 14-20: Update the GoTrue integration test configuration around the
shared client setup and tests in packages/gotrue so backend-dependent tests are
explicitly run sequentially with dart test -j 1. Before committing, run melos
analyze and melos format to validate and format the Dart changes.
In `@packages/postgrest/test/basic_test.dart`:
- Around line 17-27: Configure the PostgREST test setup around the shared-stack
tests in basic_test.dart to run sequentially, ensuring the package test command
uses dart test -j 1. Preserve the existing setup and teardown behavior while
applying the single-worker requirement to these backend-dependent tests.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a6a6194a-81f7-46d7-8ee7-b502a79029d8
📒 Files selected for processing (28)
packages/gotrue/test/admin_test.dartpackages/gotrue/test/client_test.dartpackages/gotrue/test/custom_oauth_provider_test.dartpackages/gotrue/test/get_claims_test.dartpackages/gotrue/test/provider_test.dartpackages/gotrue/test/src/gotrue_admin_custom_providers_api_test.dartpackages/gotrue/test/src/gotrue_admin_mfa_api_test.dartpackages/gotrue/test/src/gotrue_admin_oauth_api_test.dartpackages/gotrue/test/src/gotrue_mfa_api_test.dartpackages/gotrue/test/src/gotrue_oauth_api_test.dartpackages/gotrue/test/utils.dartpackages/gotrue/test/web3_auth_integration_test.dartpackages/postgrest/test/basic_test.dartpackages/postgrest/test/filter_test.dartpackages/postgrest/test/prefer_header_test.dartpackages/postgrest/test/resource_embedding_test.dartpackages/postgrest/test/test_utils.dartpackages/postgrest/test/transforms_test.dartpackages/postgrest/test/upsert_test.dartpackages/realtime_client/test/realtime_integration_test.dartpackages/realtime_client/test/utils/realtime_test_utils.dartpackages/storage_client/test/client_test.dartpackages/storage_client/test/vector_integration_test.dartpackages/supabase/test/stream_filter_test.dartpackages/supabase/test/stream_integration_test.dartpackages/supabase/test/utils/local_stack.dartpackages/supabase_common/lib/src/testing/local_stack.dartpackages/supabase_common/lib/testing.dart
💤 Files with no reviewable changes (1)
- packages/supabase/test/utils/local_stack.dart
spydon
added a commit
that referenced
this pull request
Aug 5, 2026
Stacked on #1640, since it touches the same file. The storage suite created buckets and never removed them, while `List buckets` asserted the total bucket count. A second run against the same stack therefore failed twice over: the listing saw the leftovers, and `Create new public bucket` hit a 409 because it uses a fixed name. Only CI got away with it, because every job starts a fresh stack. - Sweeps the buckets the suite creates before and after the run, so leftovers of an interrupted run cannot break the next one either. Only names the suite itself uses are removed, never the seeded buckets or those of another suite. - `List buckets` now asserts that the seeded buckets are listed instead of counting every bucket on the stack, so a suite running in parallel cannot fail it. Verified by running the suite twice in a row against a stack that had 20 leftover buckets: both runs pass and leave exactly the four seeded buckets. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Tests** - Improved storage client test reliability by documenting shared-state behavior and using a consistent temporary bucket namespace. - Added comprehensive cleanup for temporary buckets, including emptying buckets before removal and safely handling concurrent cleanup races. - Updated bucket listing coverage to verify expected seeded buckets are present without relying on a fixed total count. - Added safeguards to preserve seeded data and tolerate cleanup timing differences. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
3 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.
Follow-up to #1639.
The URL, ports and API keys of the local Supabase CLI stack were copied into every test suite that talks to it. The service role JWT alone existed in five places, port 54421 in more than twenty.
package:supabase_common/testing.dart, holding the host, ports, per-service URLs, the mail server address, the anon and service role keys and the JWT secret. Every package already depends onsupabase_common, so no pubspec changes.@visibleForTesting, so they are usable from any package's test directory but rejected in library code, which CI catches throughdart analyze --fatal-infos.apiHeaders,resetAuthDataUrland the Postgres endpoint record.packages/supabase/test/utils/local_stack.dart, which refactor(supabase): rename stream regex filters and cover stream filters with tests #1639 had added.No hardcoded stack address or key is left outside the shared library.
Summary by CodeRabbit