Skip to content

Conversation

@alexluong
Copy link
Collaborator

@alexluong alexluong commented Nov 25, 2024

Description

This PR is an overhaul of our test setup, specifically regarding integration & e2e tests where test infrastructure is required (CH, LocalStack, RabbitMQ, etc.). This PR aims to improve the test speed as well the the resource consumption when running the full test suite.

The PR provides:

  • the mechanism to run a persistent test infra so that when running tests it won't have to spend 10s on spawning testcontainer
  • the ability to reuse spawned infra for every single suite instead of spawning new containers in every test suite

You can read more about how things work in the documentation (part of this PR). Short summary is to use TESTINFRA=1 environment variable to use the persistent test infra to speed up the test suite.

Examples

Time

Here's an example time difference when running with the persistent test infra vs spawning testcontainer

➜  outpost git:(test-infra) ✗ TEST=./internal/mqs make test
go test ./internal/mqs
ok  	github.com/hookdeck/outpost/internal/mqs	19.250s
➜  outpost git:(test-infra) TEST=./cmd/e2e make test
go test ./cmd/e2e
ok  	github.com/hookdeck/outpost/cmd/e2e	12.299s

vs with TESTINFRA=1

➜  outpost git:(test-infra) ✗ TESTINFRA=1 TEST=./internal/mqs make test
go test ./internal/mqs
ok  	github.com/hookdeck/outpost/internal/mqs	1.638s
➜  outpost git:(test-infra) TESTINFRA=1 TEST=./cmd/e2e make test
go test ./cmd/e2e
ok  	github.com/hookdeck/outpost/cmd/e2e	2.672s

Resource

Besides improving speed, the internal/util/testinfra pkg allows reusing spawned testcontainers for every tests instead of spawning a new testcontainer every test suite. This results in significant improvement in terms of resource consumption when running the full test suite.

This is an example of what happens before this PR:

CleanShot 2024-11-25 at 18 39 43

Every test suite that requires some infra will spawn its own testcontainers to perform the test. Sometimes, the CPU consumption goes all the way beyond the 800% CPU that I allocated for Docker. This specific run took over 2min and it ended up in some sort of testcontainer error. This issue will only get worse as we continue adding more tests as there's no upper bound to the number of testcontainer we may require.

With this PR, when running the full suite, we only spawn 3 testcontainers for CH, LocalStack, and RabbitMQ and reuse these for the whole test suite. Furthermore, we can also use TESTINFRA=1 flag to use the persistent containers to significantly improve test time.

@alexluong alexluong force-pushed the disable-destinations branch 2 times, most recently from 3896a12 to 7427a9f Compare November 25, 2024 14:37
@alexluong alexluong merged commit 5a3aa3c into disable-destinations Nov 25, 2024
@alexluong alexluong deleted the test-infra branch November 25, 2024 14:38
@alexluong alexluong restored the test-infra branch November 25, 2024 14:39
alexluong added a commit that referenced this pull request Nov 25, 2024
)

* test: Test infra with shared testcontainers & persistent test containers

* test: Refactor infra to support parallel tests

* test: Fix issue finding .env.test at project root dir

* test: RabbitMQ infra

* refactor: Confirm integration test before starting testinfra

* test: Apply testinfra to destination adapter integration tests

* test: Refactor internal/mqs integration tests

* test: Remove unused testutil code

* test: Fix flaky test

* docs: How to use test infra & integration test template

* chore: Remove no-longer-valid comment
@alexluong alexluong deleted the test-infra branch November 25, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants