Skip to content

Commit 405c0c5

Browse files
committed
Full schema insertion + schema testing
Here, introduce a full version of schema injection as initially started in #798, and exposing a new `Schema` option to make it configurable even outside the test suite. This involved fixing a lot of bugs from #798, and the only way to make it possible to root them all out was to make full use of schemas across the entire test suite. The original "test DB manager" system has been replaced with a new `riverschematest.TestSchema` helper that generates a schema for use with a test case or prefers an existing idle one that was already generated for the same test run. `TestSchema` runs migrations for generated schemas which also means we don't need to use `testdbman` anymore, with tests capable of bootstrapping themselves at run time. We reuse schemas to avoid this extra work when possible, but migrating a new schema is also surprisingly fast, taking up to 50ms, but getting more down to a stable ~10ms once things are warmed up. Here's a series of sample timings: $ go test ./rivermigrate -run TestMigrator/MigrateUpDefault -test.v -count 50 | grep 'migrations ran in' river_migrate_test.go:492: migrations ran in 45.571209ms river_migrate_test.go:492: migrations ran in 24.642458ms river_migrate_test.go:492: migrations ran in 16.749708ms river_migrate_test.go:492: migrations ran in 22.970375ms river_migrate_test.go:492: migrations ran in 16.201375ms river_migrate_test.go:492: migrations ran in 15.727625ms river_migrate_test.go:492: migrations ran in 13.291333ms river_migrate_test.go:492: migrations ran in 13.680708ms river_migrate_test.go:492: migrations ran in 14.867416ms river_migrate_test.go:492: migrations ran in 15.631916ms river_migrate_test.go:492: migrations ran in 13.873791ms river_migrate_test.go:492: migrations ran in 14.8645ms river_migrate_test.go:492: migrations ran in 14.92575ms river_migrate_test.go:492: migrations ran in 12.541834ms river_migrate_test.go:492: migrations ran in 14.753875ms river_migrate_test.go:492: migrations ran in 12.694334ms river_migrate_test.go:492: migrations ran in 13.955917ms river_migrate_test.go:492: migrations ran in 12.126458ms river_migrate_test.go:492: migrations ran in 14.095958ms river_migrate_test.go:492: migrations ran in 13.273375ms river_migrate_test.go:492: migrations ran in 13.988917ms river_migrate_test.go:492: migrations ran in 13.141459ms river_migrate_test.go:492: migrations ran in 12.394417ms river_migrate_test.go:492: migrations ran in 11.539208ms river_migrate_test.go:492: migrations ran in 11.577834ms river_migrate_test.go:492: migrations ran in 10.883375ms river_migrate_test.go:492: migrations ran in 10.547417ms river_migrate_test.go:492: migrations ran in 12.330375ms river_migrate_test.go:492: migrations ran in 11.54575ms river_migrate_test.go:492: migrations ran in 11.437458ms river_migrate_test.go:492: migrations ran in 10.957ms river_migrate_test.go:492: migrations ran in 10.589083ms river_migrate_test.go:492: migrations ran in 9.758583ms Removal of the "test DB manager" system also means that we can ungate test from `-p 1` because they're all able to run in parallel now. The limiting factor I ran in is that we need to keep max pool connections within each package's tests to a relatively modest number (I found 15 seemed to maximum success) so parallel packages don't exceed the default Postgres configuration of 100 connections. Something that can be kind of annoying is that in case a schema isn't used properly somewhere in a test case (i.e. `TestSchema` is run, but then not used), inserts/operations will go the default schema, which will leave debris there, and that will interfere with test cases using `TestTx` (with test DB manager, all debris would go to a different database so you wouldn't notice). To remediate this, I've added a cleanup hook to `TestSchema` that looks for leftovers that may have been added to the default schema. This isn't perfect because those leftovers may have come from another test case running in parallel or which ran previously, but it helps to zero in on the original source of the issue.
1 parent cc5f0e0 commit 405c0c5

85 files changed

Lines changed: 1996 additions & 2241 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/ci.yaml

Lines changed: 10 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ env:
88
DATABASE_URL: postgres://postgres:postgres@127.0.0.1:5432/river_dev?sslmode=disable
99

1010
# Test database.
11-
TEST_DATABASE_URL: postgres://postgres:postgres@127.0.0.1:5432/river_test?sslmode=disable
11+
TEST_DATABASE_URL: postgres://postgres:postgres@127.0.0.1:5432/river_test?pool_max_conns=15&sslmode=disable
1212

1313
on:
1414
push:
@@ -42,6 +42,7 @@ jobs:
4242
postgres:
4343
image: postgres:${{ matrix.postgres-version }}
4444
env:
45+
# POSTGRES_INITDB_ARGS: "-c max_connections=1500"
4546
POSTGRES_PASSWORD: postgres
4647
options: >-
4748
--health-cmd pg_isready
@@ -62,42 +63,16 @@ jobs:
6263
- name: Display Go version
6364
run: go version
6465

65-
- name: Set up test DBs
66-
run: go run ./internal/cmd/testdbman create
67-
env:
68-
PGHOST: 127.0.0.1
69-
PGPORT: 5432
70-
PGUSER: postgres
71-
PGPASSWORD: postgres
72-
PGSSLMODE: disable
73-
74-
- name: Test
75-
working-directory: .
76-
run: go test -p 1 -race ./... -timeout 2m
77-
78-
- name: Test cmd/river
79-
working-directory: ./cmd/river
80-
run: go test -race ./... -timeout 2m
81-
82-
- name: Test riverdriver
83-
working-directory: ./riverdriver
84-
run: go test -race ./... -timeout 2m
85-
86-
- name: Test riverdriver/riverdatabasesql
87-
working-directory: ./riverdriver/riverdatabasesql
88-
run: go test -race ./... -timeout 2m
89-
90-
- name: Test riverdriver/riverpgxv5
91-
working-directory: ./riverdriver/riverpgxv5
92-
run: go test -race ./... -timeout 2m
66+
- name: Set up database
67+
run: |
68+
psql -c "CREATE DATABASE river_test" postgres://postgres:postgres@localhost:5432
69+
go run github.com/riverqueue/river/cmd/river@latest migrate-up --database-url "$TEST_DATABASE_URL"
9370
94-
- name: Test rivershared
95-
working-directory: ./rivershared
96-
run: go test -race ./... -timeout 2m
71+
- name: Temporary verbose jobcompleter run TODO remove
72+
run: go test ./internal/jobcompleter -test.v
9773

98-
- name: Test rivertype
99-
working-directory: ./rivertype
100-
run: go test -race ./... -timeout 2m
74+
- name: Test
75+
run: make test/race
10176

10277
cli:
10378
strategy:

.golangci.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ linters:
8888
- r
8989
- sb # common convention for string builder
9090
- t
91+
- tb
9192
- tt # common convention for table tests
9293
- tx
9394
- w

Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,14 @@ $(foreach mod,$(submodules),$(eval $(call lint-target,$(mod))))
5353
.PHONY: test
5454
test:: ## Run test suite for all submodules
5555
define test-target
56-
test:: ; cd $1 && go test ./... -p 1
56+
test:: ; cd $1 && go test ./...
5757
endef
5858
$(foreach mod,$(submodules),$(eval $(call test-target,$(mod))))
5959

6060
.PHONY: test/race
6161
test/race:: ## Run test suite for all submodules with race detector
6262
define test-race-target
63-
test/race:: ; cd $1 && go test ./... -p 1 -race
63+
test/race:: ; cd $1 && go test ./... -race
6464
endef
6565
$(foreach mod,$(submodules),$(eval $(call test-race-target,$(mod))))
6666

0 commit comments

Comments
 (0)