Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

Run the integration tests in parallel: 168s of tests becomes 16s

44530e746def72216bf02e88716f121936576ab7 · Keenan Nemetz · 17d ago · parent 6c03e461

Run the integration tests in parallel: 168s of tests becomes 16s

The suite was serial, and it is the wrong shape to be serial. 95 tests in
internal/server each start a server, push over SSH and shell out to git, so
they spend nearly all their time waiting on child processes and almost none
on CPU: the package took 148s wall for 23s of user time. Sixteen cores were
sitting idle while tests waited for git.

Every test now calls t.Parallel(). Nothing else about the fixtures had to
change, because they were already isolated the way parallelism needs: each
gets t.TempDir(), binds 127.0.0.1:0, and passes its environment through
cmd.Env rather than the process's. binPath is built once by TestMain and is
read-only.

Measured on 16 cores: internal/server 148.4s -> 12.8s, the whole suite
168s -> 16s, and `scripts/ci.sh` (gofmt, vet, build, link check, then the
whole suite under -race) 19.5s end to end. Verified stable across six
repeated runs, two `-shuffle=on` runs and two `-race` runs, with no data
races reported, so the parallelism is exposing isolation the fixtures
already had rather than luck.

The critical path is now one test, TestKilledRunnerIsRequeued, because it is
genuinely a timing test: it waits for a lease to expire and for the requeue
sweep. Its fixture workflow held the step for `sleep 8`, which also runs on
the retry, so eight of its fourteen seconds were the fixture waiting for
itself. Three seconds keeps the property it is testing — the runner must
still be working when it is killed, and the kill happens as soon as the lease
is visible — and takes the test to 9.7s.

What is left is fixture setup, and most of it is SQLite schema creation:
runs.Open is ~89ms and index.Open ~58ms on a *fresh* file. Measured, that is
not the eleven ALTER TABLE statements that are expected to fail on "duplicate
column" — on a warm connection those cost 2us for all eleven. It is the
CREATE TABLE / CREATE INDEX work itself, and modernc's pure-Go engine is much
slower at DDL than C SQLite. Reopening an already-created database is 408us,
so this is a cost paid once per database file, not once per open: in
production it is `vole init` and nowhere else. Tests pay it 95 times because
every fixture starts from an empty data directory.

Left alone deliberately, for two reasons. It is ~1s of wall time across the
suite, and the only fixes available are test-side (build the empty databases
once in TestMain and copy the files per fixture) or would mean changing
storage engines — which is the wrong conclusion from 150ms, and is not what
those 89ms are about.

The invariant is written down at the top of internal/server's test file, with
the rules a new test has to keep: no t.Setenv, no fixed paths, no shared
state beyond binPath, no ordering assumptions, and the shortest sleep a
fixture can get away with — because a test's own wait sets the floor for the
whole package and parallelism cannot hide it.

internal/gitx and cmd/vole keep `t.Setenv` tests, which cannot be parallel,
so those packages are unchanged. They take 0.2s and 0.5s.

internal/identity/identity_test.go +14

@@ -42,6 +42,7 @@ func pubLine(t *testing.T) (string, string) {
42 42 }
43 43
44 44 func TestEmptyRosterLoads(t *testing.T) {
45 t.Parallel()
45 46 s := newStore(t)
46 47 if got := s.Snapshot().Count; got != 0 {
47 48 t.Fatalf("count = %d, want 0", got)
@@ -49,12 +50,14 @@ func TestEmptyRosterLoads(t *testing.T) {
49 50 }
50 51
51 52 func TestMissingDatabaseIsAnError(t *testing.T) {
53 t.Parallel()
52 54 if _, err := Open(filepath.Join(t.TempDir(), "nope.db")); err == nil {
53 55 t.Fatal("Open succeeded on a missing database")
54 56 }
55 57 }
56 58
57 59 func TestCreateAccountAndAddKey(t *testing.T) {
60 t.Parallel()
58 61 s := newStore(t)
59 62 a, err := s.Create("keenan", "test")
60 63 if err != nil {
@@ -81,6 +84,7 @@ func TestCreateAccountAndAddKey(t *testing.T) {
81 84 }
82 85
83 86 func TestMultipleKeysPerAccount(t *testing.T) {
87 t.Parallel()
84 88 s := newStore(t)
85 89 if _, err := s.Create("keenan", "test"); err != nil {
86 90 t.Fatal(err)
@@ -112,6 +116,7 @@ func TestMultipleKeysPerAccount(t *testing.T) {
112 116 }
113 117
114 118 func TestDuplicateKeyAcrossAccountsRejected(t *testing.T) {
119 t.Parallel()
115 120 s := newStore(t)
116 121 for _, n := range []string{"alice", "bob"} {
117 122 if _, err := s.Create(n, "test"); err != nil {
@@ -133,6 +138,7 @@ func TestDuplicateKeyAcrossAccountsRejected(t *testing.T) {
133 138 // parse-time check: user_keys is keyed by fingerprint, so a hand-written second
134 139 // row cannot exist.
135 140 func TestOneKeyBelongsToOneAccount(t *testing.T) {
141 t.Parallel()
136 142 s := newStore(t)
137 143 for _, n := range []string{"alice", "bob"} {
138 144 if _, err := s.Create(n, "test"); err != nil {
@@ -162,6 +168,7 @@ func TestOneKeyBelongsToOneAccount(t *testing.T) {
162 168 // load, and a running store must keep its previous snapshot rather than boot
163 169 // with a partial roster.
164 170 func TestCorruptRosterFailsStartup(t *testing.T) {
171 t.Parallel()
165 172 s := newStore(t)
166 173 if _, err := s.Create("alice", "test"); err != nil {
167 174 t.Fatal(err)
@@ -186,6 +193,7 @@ func TestCorruptRosterFailsStartup(t *testing.T) {
186 193 }
187 194
188 195 func TestRemoveKey(t *testing.T) {
196 t.Parallel()
189 197 s := newStore(t)
190 198 if _, err := s.Create("keenan", "test"); err != nil {
191 199 t.Fatal(err)
@@ -211,6 +219,7 @@ func TestRemoveKey(t *testing.T) {
211 219 // TestRetireStopsAuthenticationAndReservesTheName is the property that stops a
212 220 // new person inheriting a departed one's access.
213 221 func TestRetireStopsAuthenticationAndReservesTheName(t *testing.T) {
222 t.Parallel()
214 223 s := newStore(t)
215 224 a, err := s.Create("wilbur", "test")
216 225 if err != nil {
@@ -243,6 +252,7 @@ func TestRetireStopsAuthenticationAndReservesTheName(t *testing.T) {
243 252 // TestRenameKeepsTheIdAndReservesBothNames: the id is what ACLs and approvals
244 253 // hold, so a rename must not disturb it, and neither name may be reused.
245 254 func TestRenameKeepsTheIdAndReservesBothNames(t *testing.T) {
255 t.Parallel()
246 256 s := newStore(t)
247 257 a, err := s.Create("wilbur", "test")
248 258 if err != nil {
@@ -286,6 +296,7 @@ func TestRenameKeepsTheIdAndReservesBothNames(t *testing.T) {
286 296 }
287 297
288 298 func TestInvalidNamesRefused(t *testing.T) {
299 t.Parallel()
289 300 s := newStore(t)
290 301 for _, name := range []string{"Keenan", "has space", "a", "with/slash", "trailing."} {
291 302 if _, err := s.Create(name, "test"); err == nil {
@@ -297,6 +308,7 @@ func TestInvalidNamesRefused(t *testing.T) {
297 308 // TestConcurrentAddsAllLand: key adds from concurrent goroutines each commit their
298 309 // own row, so none may be lost to a read-modify-write race.
299 310 func TestConcurrentAddsAllLand(t *testing.T) {
311 t.Parallel()
300 312 s := newStore(t)
301 313 if _, err := s.Create("keenan", "test"); err != nil {
302 314 t.Fatal(err)
@@ -330,6 +342,7 @@ func TestConcurrentAddsAllLand(t *testing.T) {
330 342 }
331 343
332 344 func TestAuditLogRecordsChanges(t *testing.T) {
345 t.Parallel()
333 346 s := newStore(t)
334 347 if _, err := s.Create("keenan", "test"); err != nil {
335 348 t.Fatal(err)
@@ -360,6 +373,7 @@ func TestAuditLogRecordsChanges(t *testing.T) {
360 373 // SQLite file, and Backup takes a consistent copy of it without a system repo or
361 374 // a directory of key files beside it.
362 375 func TestRosterIsOneDatabaseFile(t *testing.T) {
376 t.Parallel()
363 377 s := newStore(t)
364 378 a, err := s.Create("keenan", "test")
365 379 if err != nil {