Remove .env.test file #835
Conversation
WalkthroughThis pull request involves the removal of the Changes
Possibly related PRs
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
projects/app/src/server/schemaUtil.ts (1)
Line range hint
56-56: Update environment check to useisRunningTests.For consistency with the changes above, this check should also use the new
isRunningTestsconstant.Apply this diff:
- if (`NODE_TEST_CONTEXT` in process.env || __DEV__) { + if (isRunningTests || __DEV__) {
🧹 Nitpick comments (1)
projects/app/src/server/lib/inngest.ts (1)
79-80: Consider removing redundant invariant check.The token is already validated at the top level when
failFastIfMissingEnvVarsis true, making this check redundant in production. In development/test environments, the top-level check is skipped but we still want validation before usage.Consider this pattern instead:
- invariant(POSTMARK_SERVER_TOKEN != null); + if (!failFastIfMissingEnvVars) { + // Only validate in dev/test since production is already validated + invariant(POSTMARK_SERVER_TOKEN != null, "POSTMARK_SERVER_TOKEN is required"); + } const client = new postmark.ServerClient(POSTMARK_SERVER_TOKEN);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/ci-daily.yml(0 hunks).github/workflows/pr.yml(0 hunks)projects/app/.env.test(0 hunks)projects/app/src/client/ui/ReplicacheContext.tsx(3 hunks)projects/app/src/env.ts(0 hunks)projects/app/src/server/lib/inngest.ts(2 hunks)projects/app/src/server/schemaUtil.ts(2 hunks)projects/app/src/util/env.ts(1 hunks)
💤 Files with no reviewable changes (4)
- projects/app/src/env.ts
- projects/app/.env.test
- .github/workflows/pr.yml
- .github/workflows/ci-daily.yml
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: moon-ci (1)
- GitHub Check: moon-ci (0)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
projects/app/src/client/ui/ReplicacheContext.tsx (3)
5-5: LGTM! Import changes align with PR objectives.The addition of
failFastIfMissingEnvVarsandTEST_LICENSE_KEYimports supports the new approach for handling missing environment variables.Also applies to: 20-25
32-34: LGTM! Robust environment variable validation.The fail-fast check ensures that
EXPO_PUBLIC_REPLICACHE_LICENSE_KEYis defined when required, aligning with the PR's objective of improving environment variable handling.
53-54: LGTM! Graceful fallback for license key.The configuration gracefully handles missing environment variables by falling back to
TEST_LICENSE_KEY, which aligns with the PR's objective of maintaining functionality in development/testing environments.projects/app/src/util/env.ts (2)
1-1: LGTM! Clear and effective test environment detection.The use of the
inoperator is a reliable way to check for environment variable presence. The comment clearly explains the purpose.
3-3: LGTM! Robust environment-aware validation strategy.This implements a fail-fast approach for production while being more lenient in development and test environments, which aligns well with the PR objectives.
projects/app/src/server/lib/inngest.ts (1)
8-12: LGTM! Robust environment variable validation.The implementation correctly follows the new pattern for handling missing environment variables:
- Early destructuring improves readability
- Fail-fast check only in production environments
- Clear error message in the invariant
projects/app/src/server/schemaUtil.ts (1)
41-41: LGTM! Consistent use of the new environment detection utility.The change correctly uses the new
isRunningTestsconstant instead of directly checkingprocess.env.
Replace it with the convention of gracefully handling missing environment variables by disabling functionality "just in time", and having a fail fast check when running outside dev or tests.
- remove non-null assertion - fix english typos
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
projects/app/src/util/env.ts (2)
1-3: Clarity in Environment Variable Check for Test ContextThe new constant
isRunningTestsis implemented correctly by checking if"NODE_TEST_CONTEXT"exists inprocess.env. Consider using single quotes instead of backticks for consistency and clarity, as template literals aren’t needed here.-export const isRunningTests = `NODE_TEST_CONTEXT` in process.env; +export const isRunningTests = 'NODE_TEST_CONTEXT' in process.env;
4-6: Consistent String Literal Usage for CI CheckThe constant
isRunningCicorrectly determines the CI context by checking for"CI"inprocess.env. Similar to the previous comment, replacing the backticks with single quotes may improve readability.-export const isRunningCi = `CI` in process.env; +export const isRunningCi = 'CI' in process.env;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/ci-daily.yml(0 hunks).github/workflows/pr.yml(0 hunks)projects/app/.env.test(0 hunks)projects/app/drizzle.config.ts(0 hunks)projects/app/moon.yml(0 hunks)projects/app/src/client/ui/ReplicacheContext.tsx(3 hunks)projects/app/src/client/ui/auth.ts(1 hunks)projects/app/src/client/ui/replicacheOptions.web.ts(1 hunks)projects/app/src/env.ts(0 hunks)projects/app/src/server/lib/inngest.ts(2 hunks)projects/app/src/server/schemaUtil.ts(2 hunks)projects/app/src/typings/node.d.ts(0 hunks)projects/app/src/util/env.ts(1 hunks)
💤 Files with no reviewable changes (7)
- projects/app/.env.test
- projects/app/drizzle.config.ts
- projects/app/src/env.ts
- .github/workflows/ci-daily.yml
- .github/workflows/pr.yml
- projects/app/moon.yml
- projects/app/src/typings/node.d.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- projects/app/src/client/ui/replicacheOptions.web.ts
- projects/app/src/client/ui/auth.ts
- projects/app/src/server/schemaUtil.ts
- projects/app/src/server/lib/inngest.ts
- projects/app/src/client/ui/ReplicacheContext.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: moon-ci (1)
- GitHub Check: moon-ci (0)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
projects/app/src/util/env.ts (1)
8-9: Robust failFastIfMissingEnvVars FlagThe
failFastIfMissingEnvVarsflag is defined to betrueonly when not running tests, not running in CI, and not in a development environment. This approach aligns well with the fail-fast philosophy outside specified environments. Ensure that the global variable__DEV__is reliably defined in your environment.
Replace it with the convention of gracefully handling missing environment variables by disabling functionality "just in time", and having a fail fast check when running outside dev or tests.
Summary by CodeRabbit
Environment Configuration
.env.testfrom testing tasksCode Refactoring
NEON_DATABASE_URLin environment configuration