From d52803a914ce97cd0f060ccf3c633b9e65f28a52 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Mon, 28 Oct 2024 15:23:54 -0600 Subject: [PATCH] remove cleanup db util --- .github/workflows/deploy.yml | 2 +- docs/decisions/038-remove-cleanup-db.md | 45 ++++++++++++++++++ package.json | 2 +- prisma/seed.ts | 5 -- tests/db-utils.ts | 62 ------------------------- 5 files changed, 47 insertions(+), 69 deletions(-) create mode 100644 docs/decisions/038-remove-cleanup-db.md diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 4b9c766c..8495418a 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -118,7 +118,7 @@ jobs: - name: ๐ŸŒฑ Seed Database if: steps.db-cache.outputs.cache-hit != 'true' - run: npx prisma db seed + run: npx prisma migrate reset - name: ๐Ÿ— Build run: npm run build diff --git a/docs/decisions/038-remove-cleanup-db.md b/docs/decisions/038-remove-cleanup-db.md new file mode 100644 index 00000000..070f4782 --- /dev/null +++ b/docs/decisions/038-remove-cleanup-db.md @@ -0,0 +1,45 @@ +# Remove Cleanup DB + +Date: 2024-10-28 + +Status: accepted + +## Context + +We have a utility called `cleanupDb` that removes all tables from the database +except for prisma migration tables. The reference to prisma migration tables is +unfortunate because those are an implementation detail that we should not have +to think about. + +The goal of `cleanupDb` was to make it easy for tests to reset the database +without having to run `prisma migrate reset` which is too slow for lower level +tests. + +We also used `cleanupDb` in the seed file to reset the database before seeding +it. + +However, after a lot of work on the tests, we found a much simpler solution to +resetting the database between tests: simply copy/paste the `base.db` file +(which is a fresh database) to `test.db` before each test. We were already doing +this before all the tests. It takes nanoseconds and is much simpler. + +For the seed script, it's nice to have the database be completely reset when +running `prisma db seed` (in fact, our seed expects the database to be empty), +but you can get the same behavior as our current `seed` with a fresh database by +running `prisma migrate reset` (which runs the seed script after resetting the +database). + +It would be nice to ditch the implementation detail of prisma's tables, so we'd +like to remove this utility. + +## Decision + +Remove the `cleanupDb` utility and update our CI to run `prisma migrate reset` +instead of `prisma db seed`. + +## Consequences + +Running `prisma db seed` will fail because the seed script expects the database +to be empty. We could address this by using upsert or something, but really +people should just run `prisma migrate reset` to seed the database (which is +effectively what we used to do before removing `cleanupDb`). diff --git a/package.json b/package.json index df2d674b..fd39d5b3 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "prisma:studio": "prisma studio", "format": "prettier --write .", "lint": "eslint .", - "setup": "npm run build && prisma generate && prisma migrate deploy && prisma db seed && playwright install", + "setup": "npm run build && prisma generate && prisma migrate reset && playwright install", "start": "cross-env NODE_ENV=production node .", "start:mocks": "cross-env NODE_ENV=production MOCKS=true tsx .", "test": "vitest", diff --git a/prisma/seed.ts b/prisma/seed.ts index f38edef3..21bca626 100644 --- a/prisma/seed.ts +++ b/prisma/seed.ts @@ -3,7 +3,6 @@ import { promiseHash } from 'remix-utils/promise' import { prisma } from '#app/utils/db.server.ts' import { MOCK_CODE_GITHUB } from '#app/utils/providers/constants' import { - cleanupDb, createPassword, createUser, getNoteImages, @@ -16,10 +15,6 @@ async function seed() { console.log('๐ŸŒฑ Seeding...') console.time(`๐ŸŒฑ Database has been seeded`) - console.time('๐Ÿงน Cleaned up the database...') - await cleanupDb() - console.timeEnd('๐Ÿงน Cleaned up the database...') - const totalUsers = 5 console.time(`๐Ÿ‘ค Created ${totalUsers} users...`) const noteImages = await getNoteImages() diff --git a/tests/db-utils.ts b/tests/db-utils.ts index 5264125e..32fb1d40 100644 --- a/tests/db-utils.ts +++ b/tests/db-utils.ts @@ -1,7 +1,6 @@ import fs from 'node:fs' import { faker } from '@faker-js/faker' import bcrypt from 'bcryptjs' -import Database from 'better-sqlite3' import { UniqueEnforcer } from 'enforce-unique' const uniqueUsernameEnforcer = new UniqueEnforcer() @@ -114,64 +113,3 @@ export async function img({ blob: await fs.promises.readFile(filepath), } } - -let _migrationSqls: Array> | undefined -async function getMigrationSqls() { - if (_migrationSqls) return _migrationSqls - - const migrationSqls: Array> = [] - const migrationPaths = (await fs.promises.readdir('prisma/migrations')) - .filter((dir) => dir !== 'migration_lock.toml') - .map((dir) => `prisma/migrations/${dir}/migration.sql`) - - for (const path of migrationPaths) { - const sql = await fs.promises.readFile(path, 'utf8') - const statements = sql - .split(';') - .map((statement) => statement.trim()) - .filter(Boolean) - migrationSqls.push(statements) - } - - _migrationSqls = migrationSqls - - return migrationSqls -} - -export async function cleanupDb() { - const db = new Database(process.env.DATABASE_URL!.replace('file:', '')) - - try { - // Disable FK constraints to avoid relation conflicts during deletion - db.exec('PRAGMA foreign_keys = OFF') - - // Get all table names - const tables = db - .prepare( - ` - SELECT name FROM sqlite_master - WHERE type='table' AND name NOT LIKE 'sqlite_%' AND name NOT LIKE '_prisma_migrations' - `, - ) - .all() as { name: string }[] - - // Delete tables except the ones that are excluded above - for (const { name } of tables) { - db.exec(`DROP TABLE IF EXISTS "${name}"`) - } - - // Get migration SQLs and run each migration - const migrationSqls = await getMigrationSqls() - for (const statements of migrationSqls) { - // Run each sql statement in the migration - db.transaction(() => { - for (const statement of statements) { - db.exec(statement) - } - })() - } - } finally { - db.exec('PRAGMA foreign_keys = ON') - db.close() - } -}