mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 06:23:01 +00:00
* test: add withEnv helper + canonical PGLite block JSDoc withEnv(overrides, fn) saves prior values, runs the callback, restores via try/finally — including on throw. Handles delete via undefined override. Nested calls compose. Cross-test safe; explicitly NOT intra-file concurrent-safe (process.env is process-global). 7 unit cases covering sync, async, delete-key, delete-when-prior-unset, restore-on-throw, nested compose, multi-key atomic restore. reset-pglite.ts JSDoc extended with the canonical 4-line PGLite block (beforeAll create + afterAll disconnect + beforeEach reset). The lint script in the next commit enforces this exact shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: add check-test-isolation lint script + wire into verify Grep-based lint enforcing 4 rules on non-serial unit test files: R1: no process.env mutations (use withEnv() or rename to *.serial.test.ts) R2: no mock.module() (rename to *.serial.test.ts) R3: new PGLiteEngine( only inside beforeAll() context R4: PGLiteEngine creators must pair with afterAll{disconnect} Wired into 'bun run verify' and 'bun run check:all' (NOT 'bun run test' which is the parallel runner script with no pre-check chain). Matches the existing scripts/check-*.sh family shape (jsonb, progress, etc). 51 baseline violators captured in scripts/check-test-isolation.allowlist. List MUST shrink over time — entries removed by v0.26.8 (env sweep) and v0.26.9 (PGLite sweep). New files cannot be added. CLAUDE.md ## Testing section extended with R1-R4 rules table, the canonical 4-line PGLite block, withEnv pattern, and when-to-quarantine guidance. 16 fixture-driven test cases for the lint: clean, R1 (5 patterns + 1 negative), R2, R3 (top-level vs in-beforeAll), R4 (missing disconnect), *.serial.test.ts skip, test/e2e/ skip, allowlist (3 cases). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: quarantine cycle and embed mock.module test files Both files use mock.module(...) at top level — leaks across files in the same shard process. The check-test-isolation lint (R2) bans this pattern in non-serial files; quarantine is the escape hatch. Per v0.26.7 plan D5: prefer quarantine over DI on runCycle/runEmbed. Production signatures stay frozen; tests run at --max-concurrency=1 in the serial post-pass (the existing pattern shipped in v0.26.4 for brain-registry and reconcile-links). Quarantine count: 2 → 4. Cap raised to 10 informational per D15. Renames: test/core/cycle.test.ts → test/core/cycle.serial.test.ts test/embed.test.ts → test/embed.serial.test.ts Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v0.26.7) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: post-ship documentation sync for v0.26.7 - README.md "Contributing" line: point to bun run test + bun run verify (parallel fast loop) - CONTRIBUTING.md "Running tests": rewrite for the v0.26.4/v0.26.7 test surface (parallel runner, verify, slow/serial/e2e tiers) - CONTRIBUTING.md adds "Writing tests that survive the parallel loop" section: R1-R4 lint, canonical PGLite block, withEnv pattern, when to quarantine - llms-full.txt regenerated to pick up the README + CONTRIBUTING changes Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
142 lines
5.5 KiB
Bash
Executable File
142 lines
5.5 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# CI guard: fail if any non-serial unit test file violates intra-process
|
|
# isolation rules. The v0.26.4 parallel runner loads multiple test files
|
|
# into one bun process per shard; module-level state (env vars, PGLite
|
|
# engines, mock.module overrides) leaks across files in that process and
|
|
# silently flakes other tests.
|
|
#
|
|
# Rules enforced (non-serial unit test files only):
|
|
# R1: no `process.env.X = ...`, `process.env['X'] = ...`,
|
|
# `delete process.env.X`, `Object.assign(process.env, ...)`,
|
|
# `Reflect.set(process.env, ...)` mutations. Use withEnv() helper or
|
|
# rename the file to `*.serial.test.ts`.
|
|
# R2: no `mock.module(...)` anywhere. Top-level module mocks affect every
|
|
# other file in the same shard process. Rename to `*.serial.test.ts`.
|
|
# R3: `new PGLiteEngine(` may only appear within ~50 lines following a
|
|
# `beforeAll(` line. Engines created at module scope (or in describe
|
|
# bodies) leak across files in the shard process.
|
|
# R4: any file that creates `new PGLiteEngine(` must call `.disconnect(`
|
|
# inside an `afterAll(` block. Without disconnect, engines leak across
|
|
# file boundaries within a shard process.
|
|
#
|
|
# Scope:
|
|
# - Recursively scans `test/**/*.test.ts`.
|
|
# - Skips `*.serial.test.ts` entirely (the quarantine escape hatch).
|
|
# - Skips `test/e2e/**` (E2E runs sequentially in its own runner; not in
|
|
# the parallel pool).
|
|
#
|
|
# Allow-list:
|
|
# Files in `scripts/check-test-isolation.allowlist` (one filename per
|
|
# line, # comments allowed) are skipped. This exists because v0.26.7
|
|
# ships the lint as a foundation; v0.26.8 (env sweep) and v0.26.9
|
|
# (PGLite sweep) remove entries as files get fixed. New files MUST NOT
|
|
# be added — the allow-list shrinks over time, never grows.
|
|
#
|
|
# Usage: scripts/check-test-isolation.sh [TARGET_DIR]
|
|
# Exit: 0 when clean, 1 when un-allow-listed violations found.
|
|
|
|
set -euo pipefail
|
|
|
|
ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)"
|
|
cd "$ROOT"
|
|
|
|
TARGET_DIR="${1:-test}"
|
|
ALLOWLIST_FILE="$ROOT/scripts/check-test-isolation.allowlist"
|
|
|
|
# Read allowlist (one filename per line, # comments allowed). Empty file
|
|
# is fine — every violation will fail.
|
|
ALLOWLIST=""
|
|
if [ -f "$ALLOWLIST_FILE" ]; then
|
|
ALLOWLIST="$(grep -v '^[[:space:]]*#' "$ALLOWLIST_FILE" | grep -v '^[[:space:]]*$' || true)"
|
|
fi
|
|
|
|
is_allowlisted() {
|
|
local f="$1"
|
|
[ -z "$ALLOWLIST" ] && return 1
|
|
echo "$ALLOWLIST" | grep -qxF "$f"
|
|
}
|
|
|
|
# Find non-serial unit test files (excluding test/e2e). Portable across
|
|
# bash 3.2 (macOS default) and bash 4+; no mapfile.
|
|
FILE_LIST="$(find "$TARGET_DIR" -name '*.test.ts' \
|
|
-not -name '*.serial.test.ts' \
|
|
-not -path "*/e2e/*" \
|
|
-type f 2>/dev/null | sort)"
|
|
|
|
violations=0
|
|
file_count=0
|
|
|
|
emit_violation() {
|
|
local f="$1" rule="$2" detail="$3" lines="$4"
|
|
if is_allowlisted "$f"; then
|
|
return
|
|
fi
|
|
echo "ERROR: $f"
|
|
echo " rule $rule: $detail"
|
|
if [ -n "$lines" ]; then
|
|
echo "$lines" | head -3 | sed 's/^/ /'
|
|
fi
|
|
violations=$((violations + 1))
|
|
}
|
|
|
|
# Read newline-separated file list; OK on macOS bash 3.2.
|
|
while IFS= read -r f; do
|
|
[ -z "$f" ] && continue
|
|
file_count=$((file_count + 1))
|
|
# R1: env mutations.
|
|
env_lines=$(grep -nE 'process\.env\.[A-Za-z_][A-Za-z_0-9]*[[:space:]]*=[^=]|process\.env\[[^]]+\][[:space:]]*=[^=]|delete[[:space:]]+process\.env\.|delete[[:space:]]+process\.env\[|Object\.assign[[:space:]]*\([[:space:]]*process\.env|Reflect\.set[[:space:]]*\([[:space:]]*process\.env' "$f" 2>/dev/null || true)
|
|
if [ -n "$env_lines" ]; then
|
|
emit_violation "$f" "R1" "process.env mutation; use withEnv() or rename to *.serial.test.ts" "$env_lines"
|
|
fi
|
|
|
|
# R2: mock.module() anywhere.
|
|
mock_lines=$(grep -nE 'mock\.module[[:space:]]*\(' "$f" 2>/dev/null || true)
|
|
if [ -n "$mock_lines" ]; then
|
|
emit_violation "$f" "R2" "mock.module() leaks across files in the shard process; rename to *.serial.test.ts" "$mock_lines"
|
|
fi
|
|
|
|
# R3: PGLiteEngine outside ~50 lines after a beforeAll(.
|
|
if grep -qE 'new PGLiteEngine[[:space:]]*\(' "$f" 2>/dev/null; then
|
|
bad=$(awk '
|
|
BEGIN { last_before_all = -1000 }
|
|
/beforeAll[[:space:]]*\(/ { last_before_all = NR }
|
|
/new PGLiteEngine[[:space:]]*\(/ {
|
|
if (NR - last_before_all > 50) {
|
|
printf "%d:%s\n", NR, $0
|
|
}
|
|
}
|
|
' "$f" 2>/dev/null)
|
|
if [ -n "$bad" ]; then
|
|
emit_violation "$f" "R3" "new PGLiteEngine(...) outside beforeAll() context (>50 lines); move into beforeAll" "$bad"
|
|
fi
|
|
fi
|
|
|
|
# R4: PGLiteEngine creation requires afterAll{disconnect}.
|
|
if grep -qE 'new PGLiteEngine[[:space:]]*\(' "$f" 2>/dev/null; then
|
|
if ! grep -qE 'afterAll[[:space:]]*\(' "$f" 2>/dev/null \
|
|
|| ! grep -qE '\.disconnect[[:space:]]*\(' "$f" 2>/dev/null; then
|
|
emit_violation "$f" "R4" "creates PGLiteEngine but missing afterAll(() => engine.disconnect()); engine leaks across files in the shard process" ""
|
|
fi
|
|
fi
|
|
done <<EOF
|
|
$FILE_LIST
|
|
EOF
|
|
|
|
if [ $violations -gt 0 ]; then
|
|
echo
|
|
echo "check-test-isolation: FAIL ($violations violation(s))"
|
|
echo
|
|
echo "Fix:"
|
|
echo " - For env mutations, use withEnv() from test/helpers/with-env.ts"
|
|
echo " - For mock.module(), rename to *.serial.test.ts (quarantine)"
|
|
echo " - For PGLiteEngine, follow the canonical pattern in"
|
|
echo " test/helpers/reset-pglite.ts JSDoc and CLAUDE.md."
|
|
echo
|
|
echo "Or, if this is a baseline file from before the lint shipped,"
|
|
echo "add it to scripts/check-test-isolation.allowlist (with a TODO"
|
|
echo "comment naming the sweep PR that will remove it)."
|
|
exit 1
|
|
fi
|
|
|
|
echo "check-test-isolation: OK ($file_count non-serial unit files scanned)"
|