Skip to content

Commit

Permalink
chore: Actually improve tests reliability (#39)
Browse files Browse the repository at this point in the history
Running the tests in parallel with testcontainers is very unreliable
right now so i'm reverting that. It's slow but reliable. Additionally
now we are waiting for the db to actually be up with the schama that we
want
  • Loading branch information
kevinrobayna authored Oct 1, 2023
1 parent f49381d commit 22d3de3
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 17 deletions.
33 changes: 23 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,18 @@ on:
- cron: '0 10 * * 1' # run "At 10:00 on Monday"
workflow_call:
jobs:
test:
name: Test
check:
name: Check
runs-on: ubuntu-latest
timeout-minutes: 5
strategy:
fail-fast: true
matrix:
go: [ '1.20', '1.21' ]
steps:
- name: Check out code
uses: actions/checkout@v4
- name: Install Go
uses: actions/setup-go@v4
with:
go-version: ${{ matrix.go }}
go-version: 'stable'
check-latest: true
- run: go version
- name: Checkout code
uses: actions/checkout@v4
- name: Go Format
run: gofmt -s -w . && git diff --exit-code
- name: Go Tidy
Expand All @@ -52,6 +47,24 @@ jobs:
git diff --exit-code
- name: Go Build
run: make build
test:
name: Test
runs-on: ubuntu-latest
strategy:
fail-fast: true
matrix:
go: [ '1.20', '1.21' ]
steps:
- name: Check out code
uses: actions/checkout@v4
- name: Install Go
uses: actions/setup-go@v4
with:
go-version: ${{ matrix.go }}
check-latest: true
- run: go version
- name: Go Build
run: make build
- name: Go Test
run: make test
- name: Upload Coverage
Expand Down
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ generate:

.PHONY: test
test:
go run github.com/onsi/ginkgo/v2/ginkgo -r -p \
# TODO This could run in parallel but it becomes very flaky -procs=N -p
go run github.com/onsi/ginkgo/v2/ginkgo -r \
-randomize-all \
-randomize-suites \
-race \
Expand Down
21 changes: 18 additions & 3 deletions internal/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const (
defaultUser = "rotabot"
defaultPassword = "password"
defaultPostgresImage = "docker.io/postgres:14-alpine"
defaultQuery = "SELECT ID FROM ROTAS LIMIT 1"
)

type PostgresContainer struct {
Expand Down Expand Up @@ -59,9 +60,6 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize
},
ExposedPorts: []string{"5432/tcp"},
Cmd: []string{"postgres", "-c", "fsync=off"},
WaitingFor: wait.ForSQL("5432/tcp", "postgres", func(host string, port nat.Port) string {
return fmt.Sprintf("postgres://%s:%s@%s/%s?%s", "rotabot", "password", net.JoinHostPort(host, port.Port()), "rotabot", "sslmode=disable")
}).WithStartupTimeout(time.Second * 5),
}

genericContainerReq := testcontainers.GenericContainerRequest{
Expand All @@ -84,3 +82,20 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize

return &PostgresContainer{Container: container, dbName: dbName, password: password, user: user}, nil
}

func WaitStrategyWithQuery(query string) wait.Strategy {
return wait.ForSQL("5432/tcp", "postgres", connectionString()).
WithStartupTimeout(time.Second * 5).
WithQuery(query).
WithPollInterval(time.Millisecond * 10)
}

func DefaultWaitStrategy() wait.Strategy {
return WaitStrategyWithQuery(defaultQuery)
}

func connectionString() func(host string, port nat.Port) string {
return func(host string, port nat.Port) string {
return fmt.Sprintf("postgres://%s:%s@%s/%s?%s", "rotabot", "password", net.JoinHostPort(host, port.Port()), "rotabot", "sslmode=disable")
}
}
5 changes: 4 additions & 1 deletion lib/db/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/rotabot-io/rotabot/internal"
"github.com/testcontainers/testcontainers-go"
)

var _ = Describe("Migration", func() {
Expand All @@ -19,7 +20,9 @@ var _ = Describe("Migration", func() {
var err error
ctx = context.Background()

container, err = internal.RunContainer(ctx)
container, err = internal.RunContainer(ctx,
testcontainers.WithWaitStrategy(internal.WaitStrategyWithQuery("SELECT 1")),
)
Expect(err).ToNot(HaveOccurred())

connString, err = container.ConnectionString(ctx, "sslmode=disable")
Expand Down
6 changes: 4 additions & 2 deletions lib/db/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"path/filepath"

"github.com/testcontainers/testcontainers-go"

"github.com/jackc/pgx/v5"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -15,14 +17,14 @@ var _ = Describe("Repository", func() {
var ctx context.Context
var connString string
var conn *pgx.Conn
var container *internal.PostgresContainer

BeforeEach(func() {
var err error
ctx = context.Background()

container, err = internal.RunContainer(ctx,
container, err := internal.RunContainer(ctx,
postgres.WithInitScripts(filepath.Join("..", "..", "assets", "structure.sql")),
testcontainers.WithWaitStrategy(internal.DefaultWaitStrategy()),
)
Expect(err).ToNot(HaveOccurred())

Expand Down
3 changes: 3 additions & 0 deletions lib/db/rotas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"errors"
"path/filepath"

"github.com/testcontainers/testcontainers-go"

"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgconn"
. "github.com/onsi/ginkgo/v2"
Expand All @@ -22,6 +24,7 @@ var _ = Describe("Rotas", func() {

container, err := internal.RunContainer(ctx,
postgres.WithInitScripts(filepath.Join("..", "..", "assets", "structure.sql")),
testcontainers.WithWaitStrategy(internal.DefaultWaitStrategy()),
)
Expect(err).ToNot(HaveOccurred())

Expand Down
3 changes: 3 additions & 0 deletions slack/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"path/filepath"

"github.com/testcontainers/testcontainers-go"

"github.com/rotabot-io/rotabot/internal"
"github.com/rotabot-io/rotabot/slack/slackclient"

Expand Down Expand Up @@ -39,6 +41,7 @@ var _ = Describe("Service", func() {

container, err := internal.RunContainer(ctx,
postgres.WithInitScripts(filepath.Join("..", "assets", "structure.sql")),
testcontainers.WithWaitStrategy(internal.DefaultWaitStrategy()),
)
Expect(err).ToNot(HaveOccurred())

Expand Down
3 changes: 3 additions & 0 deletions slack/views/home_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"path/filepath"

"github.com/testcontainers/testcontainers-go"

"github.com/jackc/pgx/v5"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -30,6 +32,7 @@ var _ = Describe("Home", func() {

container, err := internal.RunContainer(ctx,
postgres.WithInitScripts(filepath.Join("..", "..", "assets", "structure.sql")),
testcontainers.WithWaitStrategy(internal.DefaultWaitStrategy()),
)
Expect(err).ToNot(HaveOccurred())

Expand Down
3 changes: 3 additions & 0 deletions slack/views/save_rota_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"path/filepath"

"github.com/testcontainers/testcontainers-go"

"github.com/rotabot-io/rotabot/internal"

"github.com/rotabot-io/rotabot/slack/slackclient"
Expand Down Expand Up @@ -36,6 +38,7 @@ var _ = Describe("SaveRota", func() {

container, err := internal.RunContainer(ctx,
postgres.WithInitScripts(filepath.Join("..", "..", "assets", "structure.sql")),
testcontainers.WithWaitStrategy(internal.DefaultWaitStrategy()),
)
Expect(err).ToNot(HaveOccurred())

Expand Down

0 comments on commit 22d3de3

Please sign in to comment.