BE-319: Seed the backend integration test graph once per run#9001
Conversation
Previously every one of the 21 backend integration test files ran the full system-graph seed (system policies, system account, 28 ontology migrations, system entities) in its beforeAll and wiped the graph again in its teardown via resetGraph(), so the whole seed was paid once per file. - Add a vitest globalSetup which runs ensureSystemGraphIsInitialized once per run. The per-file ensureSystemGraphIsInitialized calls are kept: they now hit the migration-state fast path (already-applied migrations are skipped based on state stored on the HASH Instance entity) and act as cheap idempotent checks. - Remove the per-file resetGraph() teardowns, which destroyed the shared seed (and the stored migration state) between files. Per-file cleanup of Kratos identities is kept. - Order the destructive subgraph snapshot tests (which reset the graph and restore standalone snapshots) after all other test files via a custom test sequencer, so they cannot destroy the shared seed mid-run. - Scope the previously-unfiltered queryEntities call in entity.test.ts to the test org's web so it no longer iterates the whole graph. - Randomize the hardcoded emails in user.test.ts and delete the leftover Kratos identity in the shortname-casing test, so the file is rerun-safe against a persistent graph. The allow-listed charlie@example.com email is kept as the incomplete-user test depends on USER_EMAIL_ALLOW_LIST. - Delete the unreferenced src/tests/setup.ts helper.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Per-file Isolation fixes for a persistent graph: Reviewed by Cursor Bugbot for commit 4836ae5. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Pull request overview
This PR speeds up the @tests/hash-backend-integration suite by seeding the system graph once per run via a vitest globalSetup, instead of once per test file. The per-file ensureSystemGraphIsInitialized calls are kept but now hit the migration-state fast path, and the destructive subgraph/ tests are sorted to run last so they can't destroy the shared seed mid-run. Test files no longer wipe the graph in teardown, so several tests were hardened against a shared/persistent graph.
Changes:
- Add
globalSetup(global-setup.ts) that seeds the system graph inline in a separate process, plus aDestructiveTestsLastSequencerthat orderssubgraph/tests last. - Remove all per-file
resetGraph()teardowns (keeping Kratos-identity cleanup) and delete the unusedsetup.ts. - Harden isolation:
entity.test.tsscopes its multi-type read query to the test org's web, anduser.test.tsrandomizes previously-hardcoded emails and deletes the Kratos identity leaked by the shortname-casing test.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
vitest.config.ts |
Adds globalSetup and a sequencer sorting subgraph/ files last; updates the parallelism comment. |
src/tests/global-setup.ts |
New once-per-run seed; builds graph context inline with dynamic imports to guarantee env loads first. |
src/tests/setup.ts |
Deleted; the unreferenced recreateDbAndRunSchemaMigrations helper is removed. |
graph/knowledge/primitive/entity.test.ts |
Scopes the "read a multi-type entity" query to the test org's web instead of all: []. |
graph/knowledge/system-types/user.test.ts |
Randomizes hardcoded emails, adds Kratos cleanup for the casing test, fixes the allow-list comment, drops resetGraph teardowns. |
graph/knowledge/system-types/hash-instance.test.ts |
Removes the resetGraph teardown; stays in the main phase (add-then-remove admin mutations). |
Other graph/** test files (org, org-membership, org-invitation-email-casing, page, file, block, comment, comment-notification, mention-notification, ai, link-entity, policy, data/entity/property-type) |
Remove resetGraph import and teardown call; retain Kratos-identity cleanup where present. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9001 +/- ##
=======================================
Coverage 59.74% 59.74%
=======================================
Files 1370 1370
Lines 135170 135170
Branches 6067 6067
=======================================
Hits 80760 80760
Misses 53477 53477
Partials 933 933
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
thehabbos007
left a comment
There was a problem hiding this comment.
LGTM. I suppose if something starts being flaky on the integration tests, we should rever/reconsider.
| * rely on the shared seed – the next run's `globalSetup` re-seeds the graph | ||
| * from scratch. | ||
| */ | ||
| class DestructiveTestsLastSequencer extends BaseSequencer { |
There was a problem hiding this comment.
Only thing i am a little unsure about. It looks fine, and probably needs a better way to register destrutive ops than checking the path prefix, and we may have other destructive tests down the line that need to be sequenced. But i think our current integration tests are eventually going to look different as move move more into the graph
There was a problem hiding this comment.
Agreed on both counts — the path prefix is the minimal mechanism that works today, and if more destructive suites appear an explicit registration (e.g. a destructive tag/annotation the sequencer reads) would be the cleaner shape. Given the suite's likely restructuring as more moves into the graph, keeping it minimal for now seemed right.
Requested by Tim Diekmann · Slack thread
🌟 What is the purpose of this PR?
Speed up the backend integration test suite by seeding the system graph once per run instead of once per test file.
Before: every one of the 21 test files ran the full system-graph seed in its
beforeAll(system policies, system account, 28 ontology migrations over HTTP, system entities) and then wiped the entire graph again in its teardown viaresetGraph()— so the next file had to pay the full seed again. The migration runner already knows how to skip previously-applied migrations (state stored on the HASH Instance entity), but the per-file wipe destroyed that state every time.After: the graph is seeded once in a vitest
globalSetupbefore any test file runs. The per-fileensureSystemGraphIsInitializedcalls remain but now hit the migration-state fast path, becoming cheap idempotent checks. Files no longer wipe the graph; the destructive subgraph snapshot tests run last so they cannot destroy the shared seed mid-run, and the next run'sglobalSetupre-seeds from scratch.This is phase A of BE-319 — test files still run serially (
fileParallelism: falseis untouched); parallelising files is a possible follow-up.🔗 Related links
🔍 What does this change?
vitest.config.ts: addsglobalSetup(src/tests/global-setup.ts) and a customDestructiveTestsLastSequencerwhich sorts thesrc/tests/subgraph/files (theyresetGraph()+ restore standalone snapshots) after all other test files.src/tests/global-setup.ts(new): runsensureSystemGraphIsInitializedonce against the running graph. It executes in a separate process from the test files, so it builds its graph context inline (novimock available) and uses dynamic imports so that the environment module is guaranteed to load before any@apps/hash-apimodule reads env vars at module scope.resetGraph()calls insrc/tests/graph/**are removed (including the two inuser.test.ts). Kratos-identity cleanup in the same teardowns is kept. The per-fileensureSystemGraphIsInitializedbeforeAllcalls are deliberately kept — with the migration state intact they skip all 28 migrations and only perform idempotent existence checks, and they keep each file self-sufficient.entity.test.ts: the "can read a multi-type entity" query used an unfilteredall: []filter over the whole graph; it is now scoped to the test org's web.user.test.ts: hardcoded emails (test-user@,bob@,case-test-user@,kratos-profile-update@example.com) are randomized likecreateTestUserdoes, and the shortname-casing test now deletes the Kratos identity it creates (previously leaked and would collide on reruns against a persistent graph).charlie@example.comis kept because the incomplete-user test requires an allow-listed email (USER_EMAIL_ALLOW_LISTin.env.test); it is already deleted via the admin API in-file.hash-instance.test.tsstays in the main phase: its instance-admins mutations are add-then-remove within the file.src/tests/setup.ts(recreateDbAndRunSchemaMigrations).Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
globalSetupre-seeds from scratch, matching today's behaviour where every file re-seeded anyway.🐾 Next steps
🛡 What tests cover this?
The backend integration suite itself (
yarn test:integrationintests/hash-backend-integration) exercises all of this.Verification performed:
tsc --noEmitandeslintfor@tests/hash-backend-integration: the full dependency graph (wasm build, graph client codegen) cannot be built in my sandbox, so both were run on this branch and onmainwith identical unbuilt-deps state and the normalized outputs diffed — the error sets are identical (475 pre-existing resolution errors from unbuilt workspace deps on both; zero findings introduced by this change). The new/changed files themselves report no errors.oxfmt --checkpasses on all changed files.sequence.sequenceris instantiated and itssort()is applied to the spec list before execution (withfileParallelism: falsefiles then run serially in that order).libp2pandquay.io/Maven downloads are denied by the network proxy), so the graph binary and external services could not be brought up. CI on this PR is the real verification — please treat a green integration job as the gate.❓ How to test this?
docker compose --profile dev up -d,yarn start:graph, or rely on CI)yarn workspace @tests/hash-backend-integration test:integrationsubgraph/files run last.Generated by Claude Code
CI timing results
Before/after on genuine (uncached) executions of the
Integration (@tests/hash-backend-integration)job's "Run tests" step:≈ 8–9× faster. Note for anyone comparing against typical recent runs: most "Run tests" steps on main show 62–82s because turbo replays a cached
test:integrationresult (cache hit, replaying logs) — those are recordings, not executions, and are not a valid baseline.