fix(test): keep the suites off a developer's real database

bun loads a plain `.env` into process.env before anything runs, and
tests/helpers/env.ts read that as a deliberate export, so it beat `.env.test`
outright. `scripts/test.sh api` therefore pointed the api suites at whatever
deployment `.env` describes: `prisma db push --accept-data-loss` for the
schema, then a truncate of every table between tests. The e2e half was worse,
because playwright.config.ts built and started the app with that DATABASE_URL
and those R2 credentials, then wrote fixtures into it. CI never saw any of
this: a runner has no `.env`.

Three changes, in order of what each one catches:

- helpers/dev-env.ts drops the values bun copied out of a development env file,
  leaving `.env.test` to fill them. Only values that match the file
  character-for-character go, so a real export still wins and the per-suite
  databases of a parallel api run keep working.
- helpers/test-database.ts refuses a DATABASE_URL whose database name is not
  marked as a test one, at the single point every path into the setup passes
  through. This is the backstop, not the fix.
- playwright.config.ts blanks the variables a development env file defines and
  the config does not. Dropping them from process.env is not enough there:
  `next build` and `next start` run @next/env themselves and read the files
  again. That is also why a local e2e run could not build at all (a set
  DISABLE_RATE_LIMIT throws in lib/rate-limit.ts under NODE_ENV=production) and
  why auth.spec.ts failed on a machine with SMTP configured.

`scripts/test.sh` now creates `.env.test` from the committed example instead of
asking for a one-line copy, so the guard above is something nobody has to meet.
This commit is contained in:
yusufipk
2026-07-26 15:49:19 +07:00
parent e3fcbf30bf
commit 6136817f75
7 changed files with 327 additions and 53 deletions
+120
View File
@@ -0,0 +1,120 @@
// Locating the checkout, and undoing bun's automatic `.env` load.
//
// Split out of helpers/env.ts so it can be imported without side effects.
// Importing that module loads all of `.env.test` into process.env, which
// playwright.config.ts must not do: the file carries DISABLE_RATE_LIMIT for the
// api suites, and `next build` runs in production mode, where lib/rate-limit.ts
// refuses to start with that set.
import fs from 'node:fs';
import path from 'node:path';
import { parse as parseDotenv } from 'dotenv';
/**
* Walks up from the working directory until it finds the checkout.
*
* This deliberately avoids `import.meta.url`, which would be the obvious way to
* resolve a path relative to this file: Playwright transpiles TypeScript to
* CommonJS unless package.json declares `"type": "module"`, and in CommonJS
* `import.meta` is a *syntax* error, so the e2e suite could not import this
* module at all. `__dirname` has the mirror-image problem under Vitest's ESM.
*
* The marker is prisma/schema.prisma as well as package.json, so a stray
* package.json inside node_modules cannot be mistaken for the checkout.
*/
function findRepoRoot(): string {
let current = path.resolve(process.cwd());
for (;;) {
if (
fs.existsSync(path.join(current, 'package.json')) &&
fs.existsSync(path.join(current, 'prisma', 'schema.prisma'))
) {
return current;
}
const parent = path.dirname(current);
if (parent === current) {
throw new Error(
`Could not locate the OpenFrame checkout from ${process.cwd()}: no ancestor ` +
'directory holds both package.json and prisma/schema.prisma. Run the test ' +
'suites from the repository root.'
);
}
current = parent;
}
}
export const REPO_ROOT = findRepoRoot();
export const TEST_ENV_PATH = path.join(REPO_ROOT, '.env.test');
/**
* The env files a developer machine has and CI does not.
*
* bun autoloads these, and so does @next/env inside `next build` and
* `next start`, which is why both halves of the problem below need the same
* list. `.env.test` is deliberately absent: that one is the test configuration.
*/
const DEV_ENV_FILES = ['.env', '.env.local', '.env.production', '.env.production.local'];
function readDevEnv(): Record<string, string> {
const merged: Record<string, string> = {};
for (const file of DEV_ENV_FILES) {
const filePath = path.join(REPO_ROOT, file);
if (!fs.existsSync(filePath)) continue;
Object.assign(merged, parseDotenv(fs.readFileSync(filePath)));
}
return merged;
}
/**
* Removes the values bun copied out of a plain `.env` on start-up.
*
* bun reads `.env` into process.env before the first line of a script runs, and
* nothing downstream can tell that apart from `DATABASE_URL=… bun run test:api`.
* Under the "an already-exported variable wins" contract the development `.env`
* therefore beat `.env.test` outright, which pointed the api suites at whatever
* deployment `.env` describes: `prisma db push --accept-data-loss` for the
* schema, then a truncate of every table between tests. The same values reached
* the app the e2e suite builds, R2 credentials included.
*
* Only entries whose current value is character-for-character what `.env` holds
* are dropped, so a real export still wins, which is what the per-suite
* databases of a parallel api run rely on. CI has no `.env`, so this is a no-op
* there.
*/
export function forgetAutoloadedDotenv(): void {
for (const [key, value] of Object.entries(readDevEnv())) {
if (process.env[key] === value) {
delete process.env[key];
}
}
}
/**
* Every variable a development env file defines.
*
* Deleting them from process.env only gets you half way for the e2e suite:
* `next build` and `next start` run @next/env themselves, which reads the same
* files again and fills in whatever is undefined. playwright.config.ts uses this
* list to blank the ones it does not set, which is the state CI is already in.
*/
export function developmentEnvKeys(): string[] {
return Object.keys(readDevEnv());
}
/**
* Reads a single variable out of `.env.test` without loading the rest of it.
*
* For playwright.config.ts, which needs DATABASE_URL to agree with the suite
* that seeds the database but must keep the rest of that file away from a
* production `next build`.
*/
export function readTestEnvValue(key: string): string | undefined {
if (!fs.existsSync(TEST_ENV_PATH)) return undefined;
return parseDotenv(fs.readFileSync(TEST_ENV_PATH))[key];
}
+13 -38
View File
@@ -13,49 +13,18 @@
// Contract: an already-exported variable always wins. `.env.test` fills the
// gaps. That is what lets CI export DATABASE_URL for a service container
// without needing a `.env.test` file at all.
//
// With one correction, see forgetAutoloadedDotenv in helpers/dev-env.ts: bun
// populates process.env from a plain `.env` before any of this runs, which the
// contract above would otherwise read as a deliberate export.
import fs from 'node:fs';
import path from 'node:path';
import { config as loadDotenv } from 'dotenv';
/**
* Walks up from the working directory until it finds the checkout.
*
* This deliberately avoids `import.meta.url`, which would be the obvious way to
* resolve a path relative to this file: Playwright transpiles TypeScript to
* CommonJS unless package.json declares `"type": "module"`, and in CommonJS
* `import.meta` is a *syntax* error, so the e2e suite could not import this
* module at all. `__dirname` has the mirror-image problem under Vitest's ESM.
*
* The marker is prisma/schema.prisma as well as package.json, so a stray
* package.json inside node_modules cannot be mistaken for the checkout.
*/
function findRepoRoot(): string {
let current = path.resolve(process.cwd());
import { forgetAutoloadedDotenv, REPO_ROOT, TEST_ENV_PATH } from './dev-env';
import { assertTestDatabase } from './test-database';
for (;;) {
if (
fs.existsSync(path.join(current, 'package.json')) &&
fs.existsSync(path.join(current, 'prisma', 'schema.prisma'))
) {
return current;
}
const parent = path.dirname(current);
if (parent === current) {
throw new Error(
`Could not locate the OpenFrame checkout from ${process.cwd()}: no ancestor ` +
'directory holds both package.json and prisma/schema.prisma. Run the test ' +
'suites from the repository root.'
);
}
current = parent;
}
}
export const REPO_ROOT = findRepoRoot();
export const TEST_ENV_PATH = path.join(REPO_ROOT, '.env.test');
export { REPO_ROOT, TEST_ENV_PATH };
let loaded = false;
@@ -63,6 +32,8 @@ export function loadTestEnv(): void {
if (loaded) return;
loaded = true;
forgetAutoloadedDotenv();
if (fs.existsSync(TEST_ENV_PATH)) {
loadDotenv({ path: TEST_ENV_PATH, quiet: true });
}
@@ -75,6 +46,10 @@ export function loadTestEnv(): void {
);
}
// Deliberately after the file load and before anything opens a pool: this is
// the one place every path into the test setup goes through.
assertTestDatabase(process.env.DATABASE_URL);
// Vitest sets this already, but db-global.ts also spawns the Prisma CLI and
// lib/rate-limit.ts throws when DISABLE_RATE_LIMIT is set in production.
// @types/node declares NODE_ENV as read-only, hence the cast.
+58
View File
@@ -0,0 +1,58 @@
// Guard that keeps the test suites off a real database.
//
// This is deliberately a separate, side-effect-free module rather than part of
// helpers/env.ts: importing that one loads .env.test and throws when
// DATABASE_URL is missing, which a unit test cannot exercise.
/**
* A database name that identifies a disposable test database.
*
* `test` has to be its own `_`/`-` delimited segment, so `openframe_test` and
* `openframe_test_api` (the per-suite databases the parallel api runs use) are
* accepted while `openframe` is not.
*/
const TEST_DATABASE_NAME = /(^|[_-])test([_-]|$)/i;
/**
* Throws unless `url` names a test database.
*
* The reason this exists: bun loads a plain `.env` into `process.env` on its
* own, so anything that reaches the test setup outside Vitest, such as
* `bun run test:db:bootstrap`, inherits the DATABASE_URL of whatever deployment
* `.env` happens to describe when `.env.test` is absent. tests/setup/db-global.ts
* then builds the schema with `prisma db push --accept-data-loss`, and the api
* suites truncate every table between tests. Neither is something you want
* pointed at a database holding real rows, and the failure is silent: the
* bootstrap prints its usual success line either way.
*
* CI is unaffected because it exports DATABASE_URL for a service container
* named openframe_test.
*/
export function assertTestDatabase(url: string): void {
let parsed: URL;
try {
parsed = new URL(url);
} catch {
throw new Error(
'DATABASE_URL is not a valid connection string, so there is no way to ' +
'tell whether it points at a test database. Refusing to continue.'
);
}
const name = decodeURIComponent(parsed.pathname).replace(/^\//, '');
if (TEST_DATABASE_NAME.test(name)) return;
throw new Error(
`Refusing to run the test setup against database "${name}" on ` +
`${parsed.hostname}: the name does not mark it as a test database.\n\n` +
'The setup builds the schema with `prisma db push --accept-data-loss` ' +
'and the api suites truncate every table, so this would destroy real ' +
'data.\n\n' +
'A test database is one whose name carries a `test` segment, for ' +
'example openframe_test or openframe_test_api.\n\n' +
'The usual cause is a missing .env.test, which leaves DATABASE_URL to be ' +
'inherited from .env: cp .env.test.example .env.test'
);
}
+57
View File
@@ -0,0 +1,57 @@
import { describe, expect, it } from 'vitest';
import { assertTestDatabase } from '../../helpers/test-database';
const CREDENTIALS = 'openframe:hunter2';
function url(database: string, host = 'localhost:5432'): string {
return `postgresql://${CREDENTIALS}@${host}/${database}?schema=public`;
}
describe('assertTestDatabase', () => {
it.each([
['the name the compose file and CI both use', 'openframe_test'],
['a per-suite database from a parallel api run', 'openframe_test_api'],
['a dash instead of an underscore', 'openframe-test'],
['a leading test segment', 'test_openframe'],
['nothing but the word itself', 'test'],
['an upper-case spelling', 'openframe_TEST'],
])('accepts %s', (_label, database) => {
expect(() => assertTestDatabase(url(database))).not.toThrow();
});
it.each([
['the production database', 'openframe'],
['a name that merely starts with the letters', 'testimonials'],
['a name that merely ends with them', 'latest'],
['an empty database name', ''],
])('rejects %s', (_label, database) => {
expect(() => assertTestDatabase(url(database))).toThrow(/Refusing to run the test setup/);
});
it('rejects a remote host just the same when the name is not a test one', () => {
expect(() => assertTestDatabase(url('openframe', '157.90.147.190:3799'))).toThrow(
/database "openframe" on 157\.90\.147\.190/
);
});
it('points at the missing .env.test, which is what actually causes this', () => {
expect(() => assertTestDatabase(url('openframe'))).toThrow(
/cp \.env\.test\.example \.env\.test/
);
});
it('keeps the password out of the message, which ends up in logs', () => {
expect(() => assertTestDatabase(url('openframe'))).toThrow(
expect.objectContaining({ message: expect.not.stringContaining('hunter2') })
);
});
it('decodes a percent-encoded database name before judging it', () => {
expect(() => assertTestDatabase(url('openframe%5Ftest'))).not.toThrow();
});
it('refuses a connection string it cannot parse rather than assuming the best', () => {
expect(() => assertTestDatabase('not-a-url')).toThrow(/not a valid connection string/);
});
});