Conversation
Enhanced model filtering logic in e2e tests by integrating `TEST_MODELS` variable directly into the primary filter chain. Removed redundant filtering steps, improving code clarity and efficiency. Updated `getTestOptions` to account for models with the test-only flag or `TEST_MODELS`.
WalkthroughAdds environment-driven gating for E2E tests. In keys-provider tests, getTestOptions now inspects models/providers, setting retries on CI and skipping locally based on TEST_MODELS or test-only flags. In gateway E2E, shifts TEST_MODELS filtering from per-test-case to a model-level provider/model-pair filter. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Env as Env (CI, TEST_MODELS)
participant Models as Models/Providers
participant KeysSpec as keys-provider.e2e.ts
participant Vitest as Vitest Runner
Env->>KeysSpec: Read CI, TEST_MODELS
KeysSpec->>Models: Scan for provider with test === "only"
Models-->>KeysSpec: hasTestOnly flag
KeysSpec->>Vitest: getTestOptions()\n- CI: { retry: 3 }\n- Local: { skip: hasTestOnly || !!TEST_MODELS }
Vitest-->>KeysSpec: Apply describe options
sequenceDiagram
autonumber
participant Env as Env (TEST_MODELS)
participant GWSpec as gateway api.e2e.ts
participant ModelList as All Models × Providers
participant Final as Final Test Models
Env-->>GWSpec: specifiedModels (provider/model ids) or empty
GWSpec->>ModelList: Apply existing filters\n(auto/custom exclusion, deactivated, free-mode)
GWSpec->>ModelList: Apply TEST_MODELS filter at model level\nmatch providerId/model.id
ModelList-->>Final: Filtered models
Note right of GWSpec: Per-test-case TEST_MODELS filters removed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/gateway/src/api.e2e.ts (1)
65-74: Prevent “phantom” inclusions when TEST_MODELS targets skipped providersIf TEST_MODELS includes a provider/model that’s marked test: "skip", the model passes this filter but yields zero test cases later. Filter them out here to keep counts and logs consistent.
- return model.providers.some((provider: ProviderModelMapping) => { - const providerModelId = `${provider.providerId}/${model.id}`; - return specifiedModels.includes(providerModelId); - }); + return model.providers.some((provider: ProviderModelMapping) => { + if (provider.test === "skip") return false; + const providerModelId = `${provider.providerId}/${model.id}`; + return specifiedModels.includes(providerModelId); + });apps/api/src/routes/keys-provider.e2e.ts (1)
21-30: Confirm intent: skipping all provider-key tests when TEST_MODELS is setCoupling provider-keys tests to TEST_MODELS may hide regressions when developers narrow model E2E locally. Verify this behavior is desired; alternatively, gate only on hasTestOnly.
- return process.env.CI - ? { retry: 3 } - : { skip: hasTestOnly || !!process.env.TEST_MODELS }; + return process.env.CI ? { retry: 3 } : { skip: hasTestOnly };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/api/src/routes/keys-provider.e2e.ts(1 hunks)apps/gateway/src/api.e2e.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use localStorage instead of cookies for client-side data persistence
Files:
apps/gateway/src/api.e2e.tsapps/api/src/routes/keys-provider.e2e.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts}: Use drizzle with the latest object syntax for database operations
For read queries, always usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/gateway/src/api.e2e.tsapps/api/src/routes/keys-provider.e2e.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use
as anyor: anyin TypeScript files.
Files:
apps/gateway/src/api.e2e.tsapps/api/src/routes/keys-provider.e2e.ts
**/*.e2e.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Name end-to-end test files with the .e2e.ts suffix
Files:
apps/gateway/src/api.e2e.tsapps/api/src/routes/keys-provider.e2e.ts
apps/{api,gateway}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/{api,gateway}/**/*.{ts,tsx}: Use Zod schemas for validation in backend services
Use Hono as the web framework for backend routes and middleware.findMany() or db().query.
apps/{api,gateway}/**/*.{ts,tsx}: Use Drizzle with the latest object syntax for database operations
For read queries, use db().query..findFirst() Files:
apps/gateway/src/api.e2e.tsapps/api/src/routes/keys-provider.e2e.tsapps/gateway/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
In apps/gateway (Hono), always use Hono + Zod + OpenAPI for validation and typesafety
Files:
apps/gateway/src/api.e2e.tsapps/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
In apps/api (Hono), always use Hono + Zod + OpenAPI for validation and typesafety
Files:
apps/api/src/routes/keys-provider.e2e.ts🧬 Code graph analysis (2)
apps/gateway/src/api.e2e.ts (1)
packages/models/src/models.ts (2)
ModelDefinition(89-130)ProviderModelMapping(22-87)apps/api/src/routes/keys-provider.e2e.ts (1)
packages/models/src/models.ts (1)
ProviderModelMapping(22-87)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e / run
- GitHub Check: build / run
🔇 Additional comments (2)
apps/api/src/routes/keys-provider.e2e.ts (2)
2-6: LGTM: models/providers imports are appropriate for gating logicThe additional imports are minimal and used correctly.
8-15: LGTM: Importing TestOptions improves typing of getTestOptionsGood move to make the return type explicit.
| .filter((model) => !model.deactivatedAt || new Date() <= model.deactivatedAt) | ||
| // Filter out free models if not in full mode | ||
| .filter((model) => fullMode || !(model as ModelDefinition).free); | ||
| .filter((model) => fullMode || !(model as ModelDefinition).free) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Parse FULL_MODE as a real boolean to avoid accidental truthiness
process.env.FULL_MODE is a string; "false"/"0" are truthy and will include free models unintentionally. Normalize once at definition.
Outside this hunk, update the declaration:
// before
const fullMode = process.env.FULL_MODE;
// after
const fullMode = /^1|true|yes$/i.test(process.env.FULL_MODE ?? "");🤖 Prompt for AI Agents
In apps/gateway/src/api.e2e.ts around line 63, process.env.FULL_MODE is being
treated as a truthy string which causes values like "false" or "0" to be
considered true; change the environment parsing at its declaration to convert
FULL_MODE into a real boolean (e.g., test the env value against /^1|true|yes$/i
and default to false), then keep the existing .filter(...) but rely on the new
boolean fullMode so free models are excluded as intended.
1 participant
Enhanced model filtering logic in e2e tests by integrating
TEST_MODELSvariable directly into the primary filter chain. Removed redundant filtering steps, improving code clarity and efficiency. UpdatedgetTestOptionsto account for models with the test-only flag orTEST_MODELS.Summary by CodeRabbit