test(coderd/database): share one database across TestGetUserStatusCounts subtests - #29464
Draft
ibetitsmike wants to merge 1 commit into
Draft
ibetitsmike wants to merge 1 commit into
ibetitsmike wants to merge 1 commit into
Conversation
…nts subtests Each of the 756 leaf subtests cloned its own Postgres database. They only need query isolation, so run every leaf inside a rolled-back transaction on one shared database via the new dbtestutil.StartRolledBackTx helper. The deleted-user scenarios set deleted_at through the Store instead of raw SQL, which the transaction-bound Store cannot run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TestGetUserStatusCountscloned a Postgres database for each of its 756 leaf subtests (6 timezones x 7 report dates x 18 scenarios). The leaves only need query isolation, so they now run inside a rolled-back transaction on one shared database via a newdbtestutil.StartRolledBackTxhelper (the rollback counterpart of the existingStartTx). Fixtures, assertions, subtest names and parallelism are unchanged.One database per scenario, as first sketched, does not work here: every fixture derives from the case's
reportUntiland the query aggregates over all users, so users from different dates cannot coexist in one database without changing every expected count. Per-transaction isolation keeps the fixtures byte-identical. The two deleted-user scenarios setdeleted_atthrough the Store (UpdateUserStatuswith the intendedupdated_at, which the deletion trigger copies intouser_deleted) instead of raw SQL, because a transaction-bound Store cannot run raw statements; the resultinguser_status_changesanduser_deletedrows are the same as before.Before / After
Local,
go test -count=1 -tags=testsmallbatch -parallel=8:dbtestutil.NewDBcalls in the test./coderd/database/ -run '^TestGetUserStatusCounts$'ok 63.2sok 28.2s./coderd/database/ok 57.2sok 33.0s=== RUN)jit = offin dbtestutil)ok 0.7s, packageok 9.7sThe remaining 28s is the query itself being JIT-compiled per call, which #29465 removes; the two changes compose.
Validation
go test -count=1 -tags=testsmallbatch -parallel=8 ./coderd/database/ -run '^TestGetUserStatusCounts$' -vpasses with 925 subtestsgo test -count=2 -shuffle=on -race -run '^TestGetUserStatusCounts$' ./coderd/database/passes (shared-fixture leakage check)golangci-lint runoncoderd/database/andcoderd/database/dbtestutil/: 0 issuesPart of the test-suite speedup series (see #29457).
Remote validation
Remote run on a dogfood workspace (separate compute), chat https://dogfood.cdr.dev/agents/5bbe73d0-866a-4841-9cc3-c5e5b4d97e44, tested head
16c8d556c4dagainst base9d973372774on the same machine:./coderd/database/ -run '^TestGetUserStatusCounts$'46.97s to 15.05s (925 subtests, 925 pass on both);./coderd/database/56.43s to 22.66s;-race -count=2 -shuffle=onon the test: pass;-parallel=1:ok 23.3s(the rolled-back transactions do not deadlock when serialized). With #29465 cherry-picked on top: testok 0.58s, packageok 7.49s. Endorsed verdict: PASS.