Anvil/initiative edit test email schema cleanup#5
Merged
Conversation
Remove the .env.telemetry pattern that baked DYNATRACE_LOG_INGEST_URL and DYNATRACE_API_TOKEN into the Docker image at build time. Secrets were extractable from the final image layer — a security vulnerability. Now: - Dockerfile has zero secrets; CMD runs bun directly - CI workflow no longer passes secrets at build time - docker-compose.prod.yml injects Dynatrace vars as runtime env vars - No app code changes needed (logging module already reads process.env) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…untime" This reverts commit 590d8b6.
…urce - Add edit route and form for draft initiatives (projects/[id]/initiatives/[initiativeId]/edit) - Add updateInitiativeEmail remote function with draft-only guard - Add getInitiative query for loading initiative data - Enhance test email with 3 modes: my-email, lead's email, custom email - Show lead selector in all modes for variable replacement - Remove emailSource column from leads table (schema, types, forms, remote functions) - Generate migration: ALTER TABLE lead DROP COLUMN email_source - Add Edit button to initiative detail panel for draft initiatives Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…onents Decompose initiative-email-form and initiative-edit-form by extracting: - InitiativeAiDraftPanel: encapsulates AI generation state (prompt, loading, error) - InitiativeTestEmailForm: encapsulates test email state (mode, lead selection, feedback) Merges testSendSuccessMessage + testSendErrorMessage into single feedback state. Eliminates ~130 lines of duplication between new/edit forms. Reduces parent component state from 12 vars to 3 each. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- AI draft panel: replace manual `generating` $state with $derived(generateInitiativeEmail.pending > 0) - Test email form: derive success from sendInitiativeTestEmail.result instead of manual feedback state, keep only errorMessage as $state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Set up vitest workspace with testcontainers for real PostgreSQL testing. Infrastructure: - Install vitest, testcontainers, @testcontainers/postgresql, postgres (postgres.js) - Root vitest.config.ts workspace config for packages/db, packages/logging, apps/web - Per-package vitest configs with appropriate timeouts - Turbo 'test' task (cache: false) and per-package test scripts - Testcontainers PostgreSQL helper (packages/db/src/__tests__/setup.ts) using postgres.js driver (vitest workers run Node.js, not Bun) - Bun module stub for apps/web (avoids transitive bun-sql import) - Separate unit/integration configs: `test` (no Docker) vs `test:integration` Tests (172 total): - packages/db: 62 ID generation/validation + 4 RLS integration tests with real PostgreSQL via testcontainers (org isolation, cross-org prevention) - packages/logging: 10 Dynatrace sink tests (batching, flushing, error handling) - apps/web schemas: 52 valibot schema tests (leads, projects, initiatives, settings) - apps/web utils: 27 utility tests (normalize, parseTypes, buildFallbackQueries) - apps/web server: 17 lead utils tests (keyword extraction, JSON parsing) Production changes (minimal): - Export AppDatabase type from @leader/db Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Switch packages/db and packages/logging from vitest to bun:test - Replace postgres.js driver with bun:sql in testcontainers setup - Remove postgres package dependency (no longer needed) - Delete vitest configs for packages/db and packages/logging - Update root vitest workspace to only reference apps/web - Adapt dynatrace-sink tests: vi.* → jest.*/spyOn + flushPromises helper - apps/web remains on vitest (needs SvelteKit vite plugin for $lib aliases) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Migrate apps/web tests from vitest to bun:test (7 test files) - Remove vitest.config.ts from root and apps/web - Remove bun mock stub (apps/web/src/__mocks__/bun.ts) - no longer needed - Remove vitest from root devDependencies - Update apps/web test script: vitest run → bun test - All 168 tests pass on bun:test (62 db + 10 logging + 96 web) - Test suite 22x faster: 427ms vs 9.3s Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 76 new tests across 5 test files for previously untested code: - email.ts: 37 tests for normalizeWebsiteUrl, extractEmails, isLikelyBusinessEmail, pickBestEmail, extractCandidateLinks - openrouter.ts: 12 tests for getContentText pure helper - packages/auth/email.ts: 6 tests for sendEmail with mocked nodemailer - packages/db/schema.ts: 21 tests for schema integrity and relations - request-logging.ts: 5 tests for addRequestLogContext with mocked $app/server Export private pure functions from email.ts and openrouter.ts for testability. Add test script to @leader/auth package.json. Total test count: 244 (was 168). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add svelte-test-setup.ts preload with Bun plugin for .svelte and .svelte.js/ts compilation via happy-dom + @testing-library/svelte - Resolve compiler per-directory to handle monorepo svelte version diffs - Enable experimental.async compiler flag for $derived(await ...) pages - Align svelte version to 5.53.6 across apps/web and packages/ui UI component tests (packages/ui): - Button (8), Card (4), Input (8), Badge (7), Breadcrumbs (5), Field (10) - 43 tests across 6 files Page and form tests (apps/web): - Login page: 5 tests (form rendering, inputs, labels) - Leads page: 8 tests (lead list, empty state, contact details) - Project page: 5 tests (heading, description, initiatives, breadcrumbs) - Initiative test email form: 8 tests (radio buttons, submit, lead dropdown) Infrastructure: - Shared SvelteKit mock helpers (sveltekit-mocks.ts) for form/query/command - bunfig.toml preload for packages/ui and apps/web - Exclude test files from tsconfig in all packages (bun:test types) - afterEach DOM cleanup in preload to prevent test leakage Total: 313 tests passing across 5 packages (6+10+83+43+171) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- 26 E2E tests covering auth, navigation, projects, leads, settings - Docker-compose E2E infrastructure (PostgreSQL on port 5433) - Page Object Model fixtures matching SCP pattern - Auth setup project with user/admin/guest storage states - Programmatic DB seeding via bun subprocess (Playwright/Node.js compat) - Production build serving on port 3000 - Restricted bun test root to src/ to avoid Playwright file conflicts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Create docs/testing.md with comprehensive testing guide covering unit, component, integration, and E2E test architecture - Update .github/workflows/ci.yml to run 4 parallel jobs: lint/typecheck/build, unit & component tests, integration tests (testcontainers), E2E tests (Playwright) - Upload Playwright report as artifact on E2E job - Update docs/agents/commands.md with test commands - Add Testing section to README.md linking to full guide Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fold googleMapsUri and businessStatus into the Text Search field mask (both are Basic-tier fields, no billing tier change) and remove the per-result googlePlaceDetails call loop entirely. Before: 1 Text Search + N Place Details calls per query batch. After: 1 Text Search per query batch. ~90% API cost reduction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace `as any` with proper Id<> branded types in db integration tests - Replace `Function` type with explicit function signatures in test mocks - Remove unused imports (spyOn, fireEvent, AppDatabase, STORAGE_STATE_ADMIN) - Remove unused variables and parameters in test files - Add eslint-disable for unavoidable `as any` in Svelte test render calls - Use proper type import for Playwright Page type Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove test file excludes from tsconfigs so the IDE can resolve bun:test types. Add missing @types/bun to packages/auth and packages/ui. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…very Rename Playwright .spec.ts files to .e2e.ts so bun's test runner doesn't pick them up (bun auto-discovers *.spec.ts). Update Playwright config with testMatch for the new .e2e.ts convention. Rename integration test from .integration.test.ts to .integration-test.ts so it's excluded from bun's *.test.ts discovery pattern. The db package's test:integration script is updated to match the new name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add turbo tasks and root package.json scripts for test:integration and test:e2e so they can be run from the repo root. Update CI to use these root-level commands instead of --cwd/working-directory overrides. - turbo run test:integration → targets @leader/db - turbo run test:e2e → targets @leader/web Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename client.integration-test.ts back to client.integration.test.ts since bun test requires '.test.' in filenames to recognize test files. Gate integration tests behind INTEGRATION=1 env var so they're skipped during 'bun test' from root but run when explicitly invoked via 'bun run test:integration'. The test:integration script sets the env var. Use 'find' command in test:integration script for reliable file discovery (consistent with the existing test script pattern). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three issues prevented test:integration from running on VM-based Docker runtimes (Rancher Desktop, Colima) with Bun's test runner: 1. Docker socket discovery: testcontainers doesn't check ~/.rd/docker.sock. Added auto-detection for Rancher Desktop and Colima socket paths when DOCKER_HOST is not already set. 2. Ryuk log streaming: the Ryuk sidecar's log-based wait strategy is incompatible with Bun's streaming implementation on non-default Docker runtimes. Disabled Ryuk by default (cleanup still handled by teardown). 3. Container readiness: the default forListeningPorts() wait strategy includes an internal-port exec check that hangs under Bun + VM-based Docker. Replaced with forLogMessage (×2) + forHealthCheck. Added connection retry loop for host port forwarding lag. 4. Drizzle schema init: the beta bun-sql adapter drops connections when initialised with a full relations schema before tables exist. Migrations now run via raw SQL client; typed drizzle instance created after. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add postbuild script to auto-copy drizzle migrations into build output, removing the manual cp step from the e2e script - Extract test orchestration into tests/run-e2e.sh (proper shell script with trap-based cleanup, set -euo pipefail, and passthrough args to playwright) - Extract TEST_USER/TEST_ADMIN credentials to tests/fixtures/credentials.ts to eliminate duplication between seed.ts and fixtures/index.ts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Playwright 1.58 added a .esm.preflight dynamic import in requireOrImport() that relies on Node's Module._resolveFilename hook to intercept resolution. Bun uses native module resolution and doesn't support this hook, causing: Cannot find module 'playwright.config.ts.esm.preflight' The patch skips the preflight import when globalThis.Bun is defined. Bun natively handles TypeScript and ESM, so the preflight (which registers Node-specific transform hooks) is unnecessary. Also updates run-e2e.sh to explicitly use 'bun run playwright test' to ensure Bun is the runtime for the test runner. Ref: oven-sh/bun#8222 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move Docker lifecycle into Playwright's global.setup.ts (start) and global.teardown.ts (stop), with CI detection to skip Docker when a CI service provides the database. Extract hardcoded env vars from playwright.config.ts into .env.e2e, loaded via bun --env-file. This keeps config clean and makes it easy to override values per environment. Simplify test:e2e to a single command: PW_DISABLE_TS_ESM=1 bun --bun --env-file=.env.e2e playwright test The --bun flag ensures Bun is the runtime (required for the esm.preflight patch to work). PW_DISABLE_TS_ESM=1 disables Playwright's Node ESM loader. Local workflow: run dev server, then bun run test:e2e. CI workflow: database service pre-provisioned, build step separate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Playwright starts webServer BEFORE project-based setup, causing a race condition where the app tries to connect to a DB that isn't running yet. Split Docker lifecycle into config-level globalSetup/globalTeardown (runs before/after webServer) and keep project-based setup for migrations and seeding only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Playwright 1.58 changed task ordering so plugins (including webServer) run before globalSetup. This meant docker.setup.ts never executed before the webServer tried to connect to the database, causing 'PostgresError: Connection closed' on every local e2e run. Move Docker lifecycle startup from globalSetup into playwright.config.ts top-level code, which runs at config-load time (before any tasks). Keep globalTeardown for cleanup since teardown ordering is unaffected. Guard with TEST_WORKER_INDEX to prevent worker processes from re-running Docker setup when they load the config file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The test-e2e CI job failed because build/index.js didn't exist — the Playwright webServer requires the built SvelteKit app. CI: Upload build output from lint-and-check job as an artifact, download it in test-e2e (which now depends on lint-and-check via needs:). This avoids a redundant second build. Local: playwright.config.ts now auto-builds when not in CI and build/index.js is missing, matching the existing Docker pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simpros
force-pushed
the
anvil/initiative-edit-test-email-schema-cleanup
branch
from
1614519 to
4d5440b
Compare
simpros
force-pushed
the
anvil/initiative-edit-test-email-schema-cleanup
branch
3 times, most recently
from
ab5177d to
a09a143
Compare
Run the E2E job inside the Playwright container image so the postgres service is reachable via hostname alias. Install Bun manually (matching GitLab CI service pattern) since the Playwright image does not ship with it. - container: mcr.microsoft.com/playwright:v1.58.2-jammy - services: postgres:18-alpine with health check - DATABASE_URL uses postgres hostname (container networking) - POSTGRES_HOST_AUTH_METHOD=trust for reliable auth - Manual Bun install via curl (apt-get unzip + bun installer) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simpros
force-pushed
the
anvil/initiative-edit-test-email-schema-cleanup
branch
from
a09a143 to
f73e640
Compare
Include playwright.config.ts in apps/web/tsconfig.json so VS Code resolves it in the configured project without per-file type references. Also remove the temporary triple-slash Bun reference from playwright.config.ts now that config-level inclusion handles editor typing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use kit.typescript.config to mutate generated .svelte-kit/tsconfig.json with ../playwright.config.ts (path relative to generated file), and simplify apps/web/tsconfig.json back to default generated include behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
No description provided.