feat: add web testing foundation with infra-backed suites#349
feat: add web testing foundation with infra-backed suites#349
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a comprehensive testing suite and infrastructure for the web app: a GitHub Actions workflow to run tests on PRs and main, a Docker Compose file with PostgreSQL 16 and Redis 7 for local CI, Vitest base and specialized configs (unit, trpc, api, integration, default), test setup files and environment example, integration helpers and test factories, numerous unit/integration/API/tRPC test modules, new test-related npm scripts in root and apps/web package.json, and small runtime tweaks (Redis reconnection logic and a renamed usage utility). 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Deploying usesend with
|
| Latest commit: |
7a78a4e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d56e6c38.usesend.pages.dev |
| Branch Preview URL: | https://feat-web-testing-foundation.usesend.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/test-web.yml:
- Around line 12-23: Add NEXTAUTH_SECRET to the test environment variables so
unsubscribe hash verification succeeds: set NEXTAUTH_SECRET: "test-secret" in
the env block currently defining NODE_ENV/NEXTAUTH_URL/... in the CI workflow
(the .github workflow env) and also add NEXTAUTH_SECRET: "test-secret" to the
defaultEnv object in the test setup (the apps/web test/setup/setup-env.ts file)
so functions createUnsubUrl, createOneClickUnsubUrl and
unsubscribeContactFromLink have the secret during integration tests.
In `@apps/web/src/lib/usage.unit.test.ts`:
- Line 7: Rename the misspelled function getUsageUinits to getUsageUnits in the
implementation and update both call sites accordingly (the test referencing
getUsageUinits in this test file and the usage-job.ts call). Search for the
symbol getUsageUinits, rename the function declaration to getUsageUnits, and
replace all references (including imports and tests) so imports, calls, and
exports use getUsageUnits consistently.
In `@apps/web/src/test/setup/setup-env.ts`:
- Line 9: The default value for NEXT_PUBLIC_IS_CLOUD in setup-env.ts is "true"
but .env.test.example uses "false", causing inconsistent test defaults; update
the NEXT_PUBLIC_IS_CLOUD entry in apps/web/src/test/setup/setup-env.ts to match
the canonical value ("false") used in apps/web/.env.test.example so tests and
example envs align, ensuring the exported/default env object that contains
NEXT_PUBLIC_IS_CLOUD uses "false".
In `@apps/web/vitest.config.ts`:
- Around line 4-27: The test config in defineConfig (the test block in
vitest.config.ts) lacks an explicit include/exclude so bare "vitest run" will
pick up integration tests; update the test block (where test.environment,
globals, setupFiles, etc. are defined) to add either an include array that only
matches unit tests (e.g., "src/**/*.test.{ts,tsx}" and/or
"src/**/*.spec.{ts,tsx}") or add an exclude pattern for integration tests (e.g.,
"**/*.integration.test.{ts,tsx}") so integration tests are not run by default
without infrastructure.
🧹 Nitpick comments (6)
apps/web/src/test/setup/setup-env.ts (1)
14-17: Empty-string env vars are silently overwritten.
!process.env[key]is falsy for"", so an explicitly exportedFOO=""would be replaced by the default. Usingprocess.env[key] === undefined(or!(key in process.env)) preserves intentional empty overrides.Proposed fix
for (const [key, value] of Object.entries(defaultEnv)) { - if (!process.env[key]) { + if (process.env[key] === undefined) { process.env[key] = value; } }apps/web/src/server/api/trpc.integration.test.ts (2)
119-122: Avoid callingcaller.teamPing()twice — each invocation re-executes the procedure.Both assertions independently invoke the procedure, doubling the DB round-trips and risking divergent results if the first call has side effects. Capture the rejection once:
Proposed fix
- await expect(caller.teamPing()).rejects.toBeInstanceOf(TRPCError); - await expect(caller.teamPing()).rejects.toMatchObject({ - code: "NOT_FOUND", - }); + await expect(caller.teamPing()).rejects.toSatisfy((err: unknown) => { + expect(err).toBeInstanceOf(TRPCError); + expect(err).toMatchObject({ code: "NOT_FOUND" }); + return true; + });
2-3: Consolidate vitest imports into a single statement.
vican be imported alongside the other vitest exports on line 2.Proposed fix
-import { afterAll, beforeEach, describe, expect, it } from "vitest"; -import { vi } from "vitest"; +import { afterAll, beforeEach, describe, expect, it, vi } from "vitest";apps/web/src/server/service/api-service.integration.test.ts (1)
25-29: Consider using thecreateTeamfactory for consistency with other integration tests.Other integration tests (e.g.,
trpc.integration.test.ts) use the factories from~/test/factories/core. Usingdb.team.createdirectly here works but diverges from the established pattern.Also applies to: 46-50
apps/web/src/test/setup/setup-tests.ts (1)
3-6:vi.clearAllMocks()is redundant when followed byvi.restoreAllMocks().
restoreAllMocksalready clears all mock state (calls, instances, results) and restores original implementations. TheclearAllMockscall can be removed.Proposed fix
afterEach(() => { - vi.clearAllMocks(); vi.restoreAllMocks(); });.github/workflows/test-web.yml (1)
3-7: Consider adding path filters to avoid running tests on unrelated changes.This workflow triggers on every pull request regardless of which files changed. Adding path filters would save CI minutes for docs-only or non-web changes.
Example
on: pull_request: + paths: + - "apps/web/**" + - "packages/**" + - "docker/testing/**" + - ".github/workflows/test-web.yml" push: branches: - main
| env: | ||
| NODE_ENV: test | ||
| NEXTAUTH_URL: http://localhost:3000 | ||
| DATABASE_URL: postgresql://usesend:password@127.0.0.1:5432/usesend_test | ||
| REDIS_URL: redis://127.0.0.1:6379/15 | ||
| AWS_ACCESS_KEY: test-access-key | ||
| AWS_SECRET_KEY: test-secret-key | ||
| AWS_DEFAULT_REGION: us-east-1 | ||
| NEXT_PUBLIC_IS_CLOUD: "true" | ||
| API_RATE_LIMIT: "2" | ||
| AUTH_EMAIL_RATE_LIMIT: "5" | ||
| RUN_INTEGRATION: "true" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if NEXTAUTH_SECRET is referenced in test-adjacent or server code
rg -n "NEXTAUTH_SECRET" --type=ts -g '!node_modules/**' apps/web/src/Repository: usesend/useSend
Length of output: 452
🏁 Script executed:
#!/bin/bash
# Also check the workflow file itself to understand the full context
cat -n .github/workflows/test-web.yml | head -50Repository: usesend/useSend
Length of output: 1615
🏁 Script executed:
#!/bin/bash
# Check for NEXTAUTH_SECRET in configuration files and test setup
rg -n "NEXTAUTH_SECRET" --type-add ts --type-list | head -5
rg "NEXTAUTH_SECRET" -t js -t ts -t jsx -t tsx --max-count=20Repository: usesend/useSend
Length of output: 476
🏁 Script executed:
#!/bin/bash
# Check for tests that use campaign-service
find apps/web -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "campaign" 2>/dev/null || echo "No campaign tests found"Repository: usesend/useSend
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Check the campaign-service file to see context of NEXTAUTH_SECRET usage
sed -n '535,570p' apps/web/src/server/service/campaign-service.ts
sed -n '625,635p' apps/web/src/server/service/campaign-service.tsRepository: usesend/useSend
Length of output: 1381
🏁 Script executed:
#!/bin/bash
# Check if there's any test configuration that might set NEXTAUTH_SECRET
find apps/web -name "jest.config.*" -o -name "vitest.config.*" -o -name "setup.*test*"
cat apps/web/jest.config.* 2>/dev/null || cat apps/web/vitest.config.* 2>/dev/null || echo "No test config found"Repository: usesend/useSend
Length of output: 731
🏁 Script executed:
#!/bin/bash
# Check the setup files for NEXTAUTH_SECRET configuration
cat apps/web/src/test/setup/setup-env.tsRepository: usesend/useSend
Length of output: 602
🏁 Script executed:
#!/bin/bash
cat apps/web/src/test/setup/setup-tests.tsRepository: usesend/useSend
Length of output: 167
🏁 Script executed:
#!/bin/bash
# Check if there are any integration or server tests that might call these functions
find apps/web/src -name "*.test.ts" -o -name "*.spec.ts" | head -20Repository: usesend/useSend
Length of output: 674
🏁 Script executed:
#!/bin/bash
# Check the integration tests to see if they might call campaign functions
grep -l "campaign\|unsubscribe" apps/web/src/server/api/trpc.integration.test.ts apps/web/src/server/service/*.integration.test.ts 2>/dev/null || echo "No matches found in integration tests"Repository: usesend/useSend
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Check if campaign service is imported or used anywhere in tests
rg "campaign-service|createUnsubUrl|unsubscribeContactFromLink" --type ts apps/web/srcRepository: usesend/useSend
Length of output: 1767
🏁 Script executed:
#!/bin/bash
# Check if there are any route handlers that use the campaign functions
rg "createUnsubUrl|createOneClickUnsubUrl|unsubscribeContactFromLink" --type ts apps/web/src/appRepository: usesend/useSend
Length of output: 522
🏁 Script executed:
#!/bin/bash
# Check if env.NEXTAUTH_SECRET is accessed conditionally or if it always fails
grep -B3 -A3 "env.NEXTAUTH_SECRET" apps/web/src/server/service/campaign-service.ts | head -30Repository: usesend/useSend
Length of output: 845
🏁 Script executed:
#!/bin/bash
# Check the unsubscribe page to see if it's tested or how it handles missing env vars
cat apps/web/src/app/unsubscribe/page.tsxRepository: usesend/useSend
Length of output: 1430
Add NEXTAUTH_SECRET to test environment variables.
NEXTAUTH_SECRET is required at runtime by the campaign unsubscribe functions (createUnsubUrl, createOneClickUnsubUrl, unsubscribeContactFromLink) and is called by server routes in apps/web/src/app/unsubscribe/page.tsx and apps/web/src/app/api/unsubscribe-oneclick/route.ts. Without it defined in the CI environment, any integration test exercising these routes will fail with "Invalid unsubscribe link" errors because hash verification will fail.
Add NEXTAUTH_SECRET: "test-secret" to the env block in .github/workflows/test-web.yml and to the defaultEnv object in apps/web/src/test/setup/setup-env.ts.
🧰 Tools
🪛 Checkov (3.2.334)
[medium] 15-16: Basic Auth Credentials
(CKV_SECRET_4)
🤖 Prompt for AI Agents
In @.github/workflows/test-web.yml around lines 12 - 23, Add NEXTAUTH_SECRET to
the test environment variables so unsubscribe hash verification succeeds: set
NEXTAUTH_SECRET: "test-secret" in the env block currently defining
NODE_ENV/NEXTAUTH_URL/... in the CI workflow (the .github workflow env) and also
add NEXTAUTH_SECRET: "test-secret" to the defaultEnv object in the test setup
(the apps/web test/setup/setup-env.ts file) so functions createUnsubUrl,
createOneClickUnsubUrl and unsubscribeContactFromLink have the secret during
integration tests.
apps/web/src/lib/usage.unit.test.ts
Outdated
| getCost, | ||
| getUsageDate, | ||
| getUsageTimestamp, | ||
| getUsageUinits, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'getUsageUinits' --type=tsRepository: usesend/useSend
Length of output: 473
🏁 Script executed:
sed -n '55,65p' apps/web/src/lib/usage.tsRepository: usesend/useSend
Length of output: 317
Rename misspelled function: getUsageUinits → getUsageUnits.
The function definition in ~/lib/usage.ts contains a typo (Uinits instead of Units). Update the function name and its two call sites in usage-job.ts and this test file.
🤖 Prompt for AI Agents
In `@apps/web/src/lib/usage.unit.test.ts` at line 7, Rename the misspelled
function getUsageUinits to getUsageUnits in the implementation and update both
call sites accordingly (the test referencing getUsageUinits in this test file
and the usage-job.ts call). Search for the symbol getUsageUinits, rename the
function declaration to getUsageUnits, and replace all references (including
imports and tests) so imports, calls, and exports use getUsageUnits
consistently.
| AWS_ACCESS_KEY: "test-access-key", | ||
| AWS_SECRET_KEY: "test-secret-key", | ||
| AWS_DEFAULT_REGION: "us-east-1", | ||
| NEXT_PUBLIC_IS_CLOUD: "true", |
There was a problem hiding this comment.
NEXT_PUBLIC_IS_CLOUD value inconsistent with .env.test.example.
This file defaults NEXT_PUBLIC_IS_CLOUD to "true", but apps/web/.env.test.example sets it to "false". Since setup-env.ts is the runtime bootstrap that applies when no env file is loaded, tests will behave as cloud-mode by default, which may surprise contributors who expect the example file to reflect the actual defaults.
Pick one canonical value and align both files.
🤖 Prompt for AI Agents
In `@apps/web/src/test/setup/setup-env.ts` at line 9, The default value for
NEXT_PUBLIC_IS_CLOUD in setup-env.ts is "true" but .env.test.example uses
"false", causing inconsistent test defaults; update the NEXT_PUBLIC_IS_CLOUD
entry in apps/web/src/test/setup/setup-env.ts to match the canonical value
("false") used in apps/web/.env.test.example so tests and example envs align,
ensuring the exported/default env object that contains NEXT_PUBLIC_IS_CLOUD uses
"false".
| export default defineConfig({ | ||
| plugins: [tsconfigPaths()], | ||
| test: { | ||
| environment: "node", | ||
| globals: true, | ||
| setupFiles: [ | ||
| "./src/test/setup/setup-env.ts", | ||
| "./src/test/setup/setup-tests.ts", | ||
| ], | ||
| clearMocks: true, | ||
| restoreMocks: true, | ||
| mockReset: true, | ||
| coverage: { | ||
| provider: "v8", | ||
| reporter: ["text", "html"], | ||
| include: ["src/**/*.{ts,tsx}"], | ||
| exclude: [ | ||
| "src/**/*.test.{ts,tsx}", | ||
| "src/**/*.spec.{ts,tsx}", | ||
| "src/test/**", | ||
| "src/env.js", | ||
| ], | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Base config has no include filter — bare vitest run will match all test files.
The test script (vitest run) uses this base config, which has no include pattern. Vitest's default glob will match *.integration.test.ts files too. If integration tests don't guard themselves with an early RUN_INTEGRATION check, they'll run (and likely fail) without infrastructure.
Consider adding an explicit include that excludes integration tests, or an exclude for integration patterns, so that the bare test command is safe to run without Docker services:
Suggested change
test: {
environment: "node",
globals: true,
+ include: ["src/**/*.test.ts"],
+ exclude: ["src/**/*.integration.test.ts"],
setupFiles: [🤖 Prompt for AI Agents
In `@apps/web/vitest.config.ts` around lines 4 - 27, The test config in
defineConfig (the test block in vitest.config.ts) lacks an explicit
include/exclude so bare "vitest run" will pick up integration tests; update the
test block (where test.environment, globals, setupFiles, etc. are defined) to
add either an include array that only matches unit tests (e.g.,
"src/**/*.test.{ts,tsx}" and/or "src/**/*.spec.{ts,tsx}") or add an exclude
pattern for integration tests (e.g., "**/*.integration.test.{ts,tsx}") so
integration tests are not run by default without infrastructure.
Greptile OverviewGreptile SummaryIntroduces comprehensive Vitest-based testing infrastructure for Key changes:
Observations:
Confidence Score: 4/5
Last reviewed commit: 9b752f3 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/package.json`:
- Around line 11-13: The base Vitest config is missing an explicit include, so
the "test" script ("vitest run") picks up all suites and duplicates what
"test:all" runs; update the base vitest.config.ts (the exported config object)
to add an explicit include array targeting only the intended baseline tests
(e.g., ["src/**/*.test.{ts,tsx,js}"] or another project-specific glob) and
ensure suite-specific configs (vitest.integration.config.ts) keep their own
include (e.g., "**/*.integration.test.*") and pool: "forks" overrides; keep the
package.json scripts ("test" and "test:all") as-is once the config includes are
tightened so each script runs the intended subset.
apps/web/package.json
Outdated
| "test": "vitest run", | ||
| "test:watch": "vitest", | ||
| "test:all": "pnpm test:unit && pnpm test:trpc && pnpm test:api && pnpm test:integration:full", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, list all vitest config files in the apps/web directory
echo "=== Looking for vitest config files ==="
find apps/web -maxdepth 1 -name "vitest*.ts" -o -name "vitest*.config.ts" | sort
# Get the package.json test scripts
echo ""
echo "=== Test scripts from package.json ==="
cat apps/web/package.json | grep -A 5 '"test"'
# Read the base vitest.config.ts
echo ""
echo "=== Base vitest.config.ts ==="
cat apps/web/vitest.config.ts 2>/dev/null | head -50
echo ""
echo "=== Checking for unit config ==="
cat apps/web/vitest.unit.config.ts 2>/dev/null | head -50
echo ""
echo "=== Checking for api config ==="
cat apps/web/vitest.api.config.ts 2>/dev/null | head -50
echo ""
echo "=== Checking for trpc config ==="
cat apps/web/vitest.trpc.config.ts 2>/dev/null | head -50
echo ""
echo "=== Checking for integration config ==="
cat apps/web/vitest.integration.config.ts 2>/dev/null | head -50Repository: usesend/useSend
Length of output: 2470
The default test script will overlap with test:all because the base config lacks an explicit include pattern.
The base vitest.config.ts doesn't specify an include pattern, so vitest run uses Vitest's default glob (**/*.test.ts, **/*.spec.ts), which matches all test files including *.unit.test.ts, *.api.test.ts, *.trpc.test.ts, and *.integration.test.ts. This means test and test:all will run the same tests. Additionally, integration tests should use pool: "forks" (specified in vitest.integration.config.ts), but the default test script runs them with the standard pool.
Add an explicit include pattern to the base config (e.g., src/**/*.test.ts only, excluding suite-specific patterns) or document what the default test script is intended to run.
🤖 Prompt for AI Agents
In `@apps/web/package.json` around lines 11 - 13, The base Vitest config is
missing an explicit include, so the "test" script ("vitest run") picks up all
suites and duplicates what "test:all" runs; update the base vitest.config.ts
(the exported config object) to add an explicit include array targeting only the
intended baseline tests (e.g., ["src/**/*.test.{ts,tsx,js}"] or another
project-specific glob) and ensure suite-specific configs
(vitest.integration.config.ts) keep their own include (e.g.,
"**/*.integration.test.*") and pool: "forks" overrides; keep the package.json
scripts ("test" and "test:all") as-is once the config includes are tightened so
each script runs the intended subset.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/test-web.yml:
- Around line 52-55: Update the pnpm setup step in the GitHub Actions workflow
to match the packageManager declared in package.json: change the
pnpm/action-setup "version: 8.9.0" entry to "version: 8.9.2" so the workflow's
pnpm version aligns with the apps/web packageManager ("pnpm@8.9.2") and avoids
corepack/version mismatch warnings.
🧹 Nitpick comments (2)
.github/workflows/test-web.yml (1)
72-82: Consider running test suites in parallel or combining into a single Vitest workspace run.Each test step (
test:unit,test:trpc,test:api,test:integration) spawns a separate Vitest process, paying the startup/transform cost each time. If CI time becomes a concern, a Vitest workspace config or a singletest:allinvocation could reduce overhead.apps/web/.env.test.example (1)
1-14: Consider addingRUN_INTEGRATIONto the example file.The integration test config gates execution on
RUN_INTEGRATION=true(also set in the CI workflow at Line 24). Including it here (commented out or set tofalse) would help developers discover the flag when setting up locally.Suggested addition
API_RATE_LIMIT=2 AUTH_EMAIL_RATE_LIMIT=5 + +# Set to "true" to run integration tests (requires infra services) +# RUN_INTEGRATION=false
| - name: Setup pnpm | ||
| uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 8.9.0 |
There was a problem hiding this comment.
pnpm version mismatch between workflow and packageManager field.
The workflow installs pnpm 8.9.0 but apps/web/package.json declares "packageManager": "pnpm@8.9.2". This can cause a version mismatch warning or failure (depending on corepack / strict engine settings). Align to 8.9.2.
Proposed fix
- name: Setup pnpm
uses: pnpm/action-setup@v4
with:
- version: 8.9.0
+ version: 8.9.2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Setup pnpm | |
| uses: pnpm/action-setup@v4 | |
| with: | |
| version: 8.9.0 | |
| - name: Setup pnpm | |
| uses: pnpm/action-setup@v4 | |
| with: | |
| version: 8.9.2 |
🤖 Prompt for AI Agents
In @.github/workflows/test-web.yml around lines 52 - 55, Update the pnpm setup
step in the GitHub Actions workflow to match the packageManager declared in
package.json: change the pnpm/action-setup "version: 8.9.0" entry to "version:
8.9.2" so the workflow's pnpm version aligns with the apps/web packageManager
("pnpm@8.9.2") and avoids corepack/version mismatch warnings.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/test-web.yml (2)
3-11: Path filter may miss regressions from shared package changes.The workflow only triggers on
apps/web/**changes. If a shared package underpackages/*is modified in a PR without touchingapps/web/, these tests won't run, potentially missing regressions. Consider adding paths for critical shared packages (e.g.,packages/sdk/**) or the root lockfile.Example path additions
on: pull_request: paths: - "apps/web/**" + - "packages/**" + - "pnpm-lock.yaml" push: branches: - main paths: - "apps/web/**" + - "packages/**" + - "pnpm-lock.yaml"
14-86: Consider adding a job-leveltimeout-minutesto prevent runaway CI.If a test hangs or infrastructure fails to respond, this job could run indefinitely. A timeout (e.g., 15–20 minutes) provides a safety net.
Proposed fix
web-tests: runs-on: ubuntu-latest + timeout-minutes: 20 env:
* feat: add web test framework with infra-backed suites * fix: honor DATABASE_URL env in integration prepare script * fix: apply web test review feedback * fix: streamline web test infra lifecycle and workflow scope
Summary
apps/webwith dedicated unit, tRPC, API, and integration suite configs plus root/app test scripts.env.test.example, and a GitHub Actions workflow that runs unit + tRPC + API + integration suitesVerification
pnpm --filter=web test:unitpnpm --filter=web test:trpcpnpm --filter=web test:apipnpm test:infra:uppnpm --filter=web test:integration:fullpnpm test:infra:downpnpm --filter=web testSummary by cubic
Adds a Vitest testing foundation for apps/web with unit, tRPC, API, and infra-backed integration suites. Includes Dockerized Postgres/Redis, root/app scripts, a scoped CI workflow, and broad test coverage across core modules and the public API.
New Features
Bug Fixes
Written for commit 7a78a4e. Summary will update on new commits.
Summary by CodeRabbit