ci: shard package-local test suites - #6327
Conversation
![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: 9d0417ceaa
ℹ️ 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".
…ocal-tests # Conflicts: # CONTRIBUTING.md # nx.json # package.json # packages/api/package.json # packages/cli-test-helpers/package.json # packages/config/package.json # packages/process-compose/package.json
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@8e97e9c53154bb7b9e405e968b270efb0d1680d0Preview package for commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d99065a918
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
avallete
left a comment
avallete
left a comment
There was a problem hiding this comment.
LGTM — verified the risky spots and they all check out:
- The 8
@parcel/watcher-*platform packages land indevDependenciesonly; the publishedsupabasepackage still ships justeciesjs+josewithfiles: ["dist/supabase.js"], so no platform-mismatch risk for npm consumers (and this quietly fixes thepnpm buildbreak on missing watcher binaries, including win32 whichsupportedArchitecturesdoesn't cover). - Forwarding
--shard=N/3to every e2e package is safe: all three e2e-owning vitest configs setpassWithNoTests: true(packages/stack only has 4 e2e files). - Dropping the keyring-unlock step from test.yml is correct:
cli-go-ci.ymlkeeps the unlock and itsmerge_grouptrigger has no path filter, so Go tests still gate every merge even when a PR doesn't touchapps/cli-go/. - No test files orphaned by the removed vitest project stanzas (api/config have zero integration/e2e files, process-compose zero e2e), and every package with unit tests declares
test:unit:run. test-summarypreserves the branch-protection check name and mirrors the existingtest-e2e-summaryskipped-handling pattern with identicalifconditions.- No stale
test:core/nx affected/NX_BASEreferences remain.
Two non-blocking comments inline about leftover Nx test-target inference and the dual build orchestrators.
| | `types:check` | Type-check with `tsc --noEmit` _(inferred by Nx plugin)_ | | ||
|
|
||
| The inferred scripts (`test:unit`, `test:integration`, `test:e2e`, `types:check`) are not declared in `package.json` — they are injected by local Nx plugins in `tools/nx-plugins/`. They are fully cached and can be discovered via `nx show project <name>`. | ||
| The test scripts are declared in each package's `package.json`, so package-local test commands are directly discoverable and can be sharded independently. Type-checking remains an Nx-inferred target. |
There was a problem hiding this comment.
Non-blocking, but this line is ahead of the code: tools/nx-plugins/src/test.plugin.ts is still registered in nx.json and still infers test:unit/test:integration/test:e2e targets from every vitest config with cache: true (and go.plugin.ts still infers test:unit for cli-go; nx.json also keeps the test/test:e2e targetDefaults). So nx run-many -t test:unit remains a second, cached test path that invokes vitest directly and bypasses the Turbo orchestration — a stale-green footgun. Since nothing in CI uses those targets anymore, it'd be worth stripping the test-target inference (keeping types:check and project registration) here or in a fast follow-up.
| # every package-local e2e suite. | ||
| - name: Build CLI | ||
| if: steps.detect.outputs.cli_e2e == 'true' | ||
| run: pnpm exec nx run supabase:build |
There was a problem hiding this comment.
Nit: CI builds the CLI via nx run supabase:build while the local test:e2e scripts build via turbo run supabase#build → @supabase/cli-go#build. Both work, and I get that Turbo only owns tests for now, but worth converging on one build orchestrator eventually. Related: the Nx project is named cli-go (go.plugin default) while the new apps/cli-go/package.json names it @supabase/cli-go — two names for the same workspace invites confusion.
3 participants
Summary
test:corealias and document explicit unit and integration commands.Context
The first e2e shard previously ran every non-CLI-harness e2e suite unsharded, making it consistently much slower than the other shards. Propagating the same Vitest shard to each package distributes those suites uniformly.
Turbo currently provides only the test task graph and prerequisite ordering. Test caching remains disabled until its inputs and outputs are designed separately.