test : DB結合テスト基盤の導入とrunInTransactionテストの移行#129
test : DB結合テスト基盤の導入とrunInTransactionテストの移行#129Mel-906 wants to merge 5 commits intofeature/uow-repository-transactionfrom
Conversation
KinjiKawaguchi
left a comment
There was a problem hiding this comment.
PR Titleを test:hogehoge
にしたほうが良さそう。
There was a problem hiding this comment.
DB接続情報がテストコード、Docker Compose、CIの3箇所に散在しているのが気になります。
.env.test に POSTGRES_* を一元管理し、Docker Composeとテストコードの両方がそこから読むようにすると解消できます:
.env.test:
POSTGRES_USER=core_test
POSTGRES_PASSWORD=core_test
POSTGRES_DB=core_test
POSTGRES_PORT=5433
docker-compose.test.yml:
services:
test-db:
image: postgres:17-alpine
env_file: .env.test
ports:
- "${POSTGRES_PORT:-5433}:5432"
# ...tests/helpers/db.ts:
import "dotenv/config"; // .env.testをロード
const user = process.env.POSTGRES_USER;
const password = process.env.POSTGRES_PASSWORD;
const db = process.env.POSTGRES_DB;
const port = process.env.POSTGRES_PORT ?? "5433";
export const TEST_DATABASE_URL = `postgresql://${user}:${password}@localhost:${port}/${db}`;のようにすることで接続情報のソースが .env.test の1箇所に集約されると思います。
これはアドバイスなんですが、process.env は型なし・グローバル・ミュータブルな外部依存です。プロダクションコードでDB接続をRepository境界に閉じ込めるのと同じ考え方で、process.env への直接アクセスも境界層に閉じ込めて、内側のコードには型のついた値として渡すのが望ましいです。
具体的には、process.env を読む専用の設定モジュールを用意します:
tests/helpers/config.ts:
import "dotenv/config";
export const testConfig = {
db: {
user: process.env.POSTGRES_USER ?? "core_test",
password: process.env.POSTGRES_PASSWORD ?? "core_test",
name: process.env.POSTGRES_DB ?? "core_test",
port: Number(process.env.POSTGRES_PORT ?? 5433),
},
} as const;
export const TEST_DATABASE_URL =
`postgresql://${testConfig.db.user}:${testConfig.db.password}@localhost:${testConfig.db.port}/${testConfig.db.name}`;db.ts や他のヘルパーはこの config.ts から import するだけで、process.env に直接触りません。
環境変数の読み取りと解釈が1箇所に集約される、値が as const で型付けされ、typoや型ミスマッチがコンパイル時に検出できる設定値の出どころを追うときに見る場所が1ファイルで済むというメリットがあります
There was a problem hiding this comment.
本番環境がpostgres15なのでそっちを使えると良い
There was a problem hiding this comment.
posgtre15 に下げて検証回し、問題なく通ったのでコミット、プッシュしました!
There was a problem hiding this comment.
@Mel-906 imageがpostgres:17-alphaになっているみたいだけど、私が何か間違えている?
|
対応遅くなってしまいすみません。
ローカルでも |
| if (dbName !== testConfig.db.name) { | ||
| throw new Error( | ||
| `テスト用DBではないDBに接続しています: "${dbName}"。接続先が core_test であることを確認してください。`, | ||
| `テスト用DBではないDBに接続しています: "${dbName}"。接続先が ${testConfig.db.name} であることを確認してください。`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🚩 Safety guard now compares against configurable DB name instead of hardcoded value
The old code at tests/helpers/db.ts:50 hardcoded dbName !== "core_test" as a safety check to prevent running tests against a production database. The new code compares dbName !== testConfig.db.name, where testConfig.db.name comes from .env.test. This means the safety guard is now tautological — it will always pass as long as the connection succeeds, because the configured name and actual name will always match when the connection string is derived from the same config. If someone accidentally configures .env.test to point at a production database, this guard won't catch it. The old hardcoded check provided a stronger safety net. Whether this matters depends on the team's risk tolerance for test database misuse.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Repo上に本番DBの情報直書きして防ぐみたいなのを思いましたがさすがにダメですよね...
DB接続先情報の一部のみハードに実装しますか?
There was a problem hiding this comment.
localhostじゃなかったら警告するとかがいいかもしれない?
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Docker Compose + Vitest設定分離 + テストヘルパーによる結合テスト基盤を構築し、 既存のrunInTransactionテストを新基盤に移行した。 CIにintegrationテストジョブを追加。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dfb4694 to
7bd75f1
Compare
b74198a to
44cd950
Compare
There was a problem hiding this comment.
@Mel-906 imageがpostgres:17-alphaになっているみたいだけど、私が何か間違えている?
| if (dbName !== testConfig.db.name) { | ||
| throw new Error( | ||
| `テスト用DBではないDBに接続しています: "${dbName}"。接続先が core_test であることを確認してください。`, | ||
| `テスト用DBではないDBに接続しています: "${dbName}"。接続先が ${testConfig.db.name} であることを確認してください。`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
localhostじゃなかったら警告するとかがいいかもしれない?
| import { beforeEach } from "vite-plus/test"; | ||
| import "./config"; | ||
| import { cleanDatabase } from "./db"; | ||
|
|
||
| beforeEach(async () => { | ||
| await cleanDatabase(); | ||
| }); |
There was a problem hiding this comment.
📝 Info: Module import ordering ensures correct DATABASE_URL before pool creation
There's a subtle dependency: the production client.ts lazily reads process.env.DATABASE_URL only when getPool() is first called. The setupFiles config ensures setupIntegration.ts runs before test files, and it imports ./config which sets process.env.DATABASE_URL as a module-level side effect (tests/helpers/config.ts:43). Since the production pool is lazy (src/infrastructure/drizzle/client.ts:12-21), the env var is guaranteed to be set before the pool reads it. This works correctly but is fragile — if anyone imports the production client at module scope in a setupFile that runs before config, the pool would get an empty DATABASE_URL. A comment noting this ordering dependency would help future maintainers.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
runInTransaction.test.tsを新基盤に移行背景
プロジェクト初のDB結合テスト基盤です。
今後、他のメンバーがRepositoryやUseCaseの結合テストを追加する際の土台になることを意識しています。
具体的には、テストファイルを
*.integration.test.tsで作成するだけで、DB接続・スキーマ同期・テストごとのクリーンアップが自動で行われる構成にしています。変更内容
1. テスト用DB(Docker Compose)
docker-compose.test.ymlを追加--waitに対応2. テストヘルパー(
tests/helpers/)db.ts: テスト用DB接続・Drizzleクライアント・スキーマからの動的テーブル列挙によるTRUNCATE CASCADE・マイグレーション(実行前にDB名ガード付き)globalSetup.ts: テストスイート開始時にDBスキーマを同期(drizzle-kit push)setupIntegration.ts: 各テスト前にDATABASE_URL設定 + 全テーブルTRUNCATE3. Vitest設定分離
vite.config.integration.tsを追加(*.integration.test.tsのみ対象)vite.config.tsにexcludeを追加し、unitテストにintegrationが混入しない構成4. npmスクリプト
test:integration— 結合テスト実行(DB起動済み前提)test:integration:up— DB起動 → テスト実行(ワンコマンド)test:integration:down— テスト用DBコンテナ停止5. CI
test-integrationジョブを追加(GitHub Actions servicesでPostgreSQLを起動)6. テスト移行
runInTransaction.test.ts→runInTransaction.integration.test.tsテストで確認している事柄(runInTransaction)
runInTransactionでINSERT成功後、外側で例外を投げ、内側の書き込みも含めて全て巻き戻ることを確認(ネストが新しいトランザクションを開始せず、外側に合流する動作の検証)結合テストの追加方法(今後のために)
*.integration.test.tsでファイルを作るimport { getTestClient } from "../../helpers/db"で取得npm run test:integration:upTest plan
npm run test:integration:upで3テスト全パスnpm testで既存127テスト全パス(integrationが混入していないことを確認)test-integrationジョブの動作確認🤖 Generated with Claude Code