-
Notifications
You must be signed in to change notification settings - Fork 372
chore: replace ory/dockertest with testcontainers #2782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,20 @@ | ||
| //go:build docker && image | ||
| // //go:build docker && image | ||
|
|
||
| package integration_test | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "context" | ||
| "fmt" | ||
| "io" | ||
| "slices" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/google/uuid" | ||
| "github.com/ory/dockertest/v3" | ||
| "github.com/ory/dockertest/v3/docker" | ||
| "github.com/stretchr/testify/require" | ||
| "github.com/testcontainers/testcontainers-go" | ||
| "github.com/testcontainers/testcontainers-go/network" | ||
|
|
||
| testdatastore "github.com/authzed/spicedb/internal/testserver/datastore" | ||
| "github.com/authzed/spicedb/pkg/datastore" | ||
|
|
@@ -20,18 +23,14 @@ import ( | |
| var toSkip = []string{"memory"} | ||
|
|
||
| func TestMigrate(t *testing.T) { | ||
| ctx := context.Background() | ||
| bridgeNetworkName := fmt.Sprintf("bridge-%s", uuid.New().String()) | ||
|
|
||
| pool, err := dockertest.NewPool("") | ||
| require.NoError(t, err) | ||
|
|
||
| // Create a bridge network for testing. | ||
| network, err := pool.Client.CreateNetwork(docker.CreateNetworkOptions{ | ||
| Name: bridgeNetworkName, | ||
| }) | ||
| net, err := network.New(ctx, network.WithDriver("bridge"), network.WithLabels(map[string]string{"name": bridgeNetworkName})) | ||
| require.NoError(t, err) | ||
| t.Cleanup(func() { | ||
| _ = pool.Client.RemoveNetwork(network.ID) | ||
| _ = net.Remove(ctx) | ||
| }) | ||
|
|
||
| for _, engineKey := range datastore.Engines { | ||
|
|
@@ -42,46 +41,46 @@ func TestMigrate(t *testing.T) { | |
| t.Run(engineKey, func(t *testing.T) { | ||
| engineKey := engineKey | ||
|
|
||
| r := testdatastore.RunDatastoreEngineWithBridge(t, engineKey, bridgeNetworkName) | ||
| r := testdatastore.RunDatastoreEngine(t, engineKey) | ||
| db := r.NewDatabase(t) | ||
|
|
||
| envVars := []string{} | ||
| envVars := map[string]string{} | ||
| if wev, ok := r.(testdatastore.RunningEngineForTestWithEnvVars); ok { | ||
| envVars = wev.ExternalEnvVars() | ||
| for _, env := range wev.ExternalEnvVars() { | ||
| parts := strings.SplitN(env, "=", 2) | ||
| if len(parts) == 2 { | ||
| envVars[parts[0]] = parts[1] | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Run the migrate command and wait for it to complete. | ||
| resource, err := pool.RunWithOptions(&dockertest.RunOptions{ | ||
| Repository: "authzed/spicedb", | ||
| Tag: "ci", | ||
| Cmd: []string{"migrate", "head", "--datastore-engine", engineKey, "--datastore-conn-uri", db}, | ||
| NetworkID: bridgeNetworkName, | ||
| Env: envVars, | ||
| }, func(config *docker.HostConfig) { | ||
| config.RestartPolicy = docker.RestartPolicy{ | ||
| Name: "no", | ||
| } | ||
| // TODO: | ||
| container, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: why not using the module? https://github.com/Mariscal6/testcontainers-spicedb-go If it does not provide the APIs need by these tests, we can contribute them to the upstream. I'm pretty sure @Mariscal6 will be happy to review that |
||
| ContainerRequest: testcontainers.ContainerRequest{ | ||
| Image: "authzed/spicedb:ci", | ||
| Cmd: []string{"migrate", "head", "--datastore-engine", engineKey, "--datastore-conn-uri", db}, | ||
| Networks: []string{bridgeNetworkName}, | ||
| Env: envVars, | ||
| }, | ||
| Started: true, | ||
| }) | ||
| require.NoError(t, err) | ||
| t.Cleanup(func() { | ||
| _ = pool.Purge(resource) | ||
| }) | ||
| testcontainers.CleanupContainer(t, container) | ||
|
|
||
| // Ensure the command completed successfully. | ||
| status, err := pool.Client.WaitContainerWithContext(resource.Container.ID, t.Context()) | ||
| state, err := container.State(ctx) | ||
| require.NoError(t, err) | ||
|
|
||
| if status != 0 { | ||
| if state.ExitCode != 0 { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: you could define a log consumer at container creation, and print it here. Please see https://golang.testcontainers.org/features/follow_logs/#following-container-logs |
||
| stream := new(bytes.Buffer) | ||
|
|
||
| lerr := pool.Client.Logs(docker.LogsOptions{ | ||
| Context: t.Context(), | ||
| OutputStream: stream, | ||
| ErrorStream: stream, | ||
| Stdout: true, | ||
| Stderr: true, | ||
| Container: resource.Container.ID, | ||
| }) | ||
| // TODO: use logs | ||
| logReader, lerr := container.Logs(ctx) | ||
| require.NoError(t, lerr) | ||
| defer logReader.Close() | ||
|
|
||
| _, lerr = io.Copy(stream, logReader) | ||
| require.NoError(t, lerr) | ||
|
|
||
| require.Fail(t, "Got non-zero exit code", stream.String()) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,20 @@ | ||
| //go:build docker && image | ||
| // //go:build docker && image | ||
|
|
||
| package integration_test | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "io" | ||
| "strings" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/google/uuid" | ||
| "github.com/ory/dockertest/v3" | ||
| "github.com/ory/dockertest/v3/docker" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| "github.com/testcontainers/testcontainers-go" | ||
| "github.com/testcontainers/testcontainers-go/network" | ||
|
|
||
| testdatastore "github.com/authzed/spicedb/internal/testserver/datastore" | ||
| "github.com/authzed/spicedb/pkg/datastore" | ||
|
|
@@ -33,95 +35,86 @@ func TestSchemaWatch(t *testing.T) { | |
| } | ||
|
|
||
| t.Run(driverName, func(t *testing.T) { | ||
| ctx := context.Background() | ||
| bridgeNetworkName := fmt.Sprintf("bridge-%s", uuid.New().String()) | ||
|
|
||
| pool, err := dockertest.NewPool("") | ||
| require.NoError(t, err) | ||
|
|
||
| // Create a bridge network for testing. | ||
| network, err := pool.Client.CreateNetwork(docker.CreateNetworkOptions{ | ||
| Name: bridgeNetworkName, | ||
| }) | ||
| net, err := network.New(ctx, network.WithDriver("bridge"), network.WithLabels(map[string]string{"name": bridgeNetworkName})) | ||
| require.NoError(t, err) | ||
| t.Cleanup(func() { | ||
| _ = pool.Client.RemoveNetwork(network.ID) | ||
| _ = net.Remove(ctx) | ||
| }) | ||
|
|
||
| engine := testdatastore.RunDatastoreEngineWithBridge(t, driverName, bridgeNetworkName) | ||
| engine := testdatastore.RunDatastoreEngine(t, driverName) | ||
|
|
||
| envVars := []string{} | ||
| envVars := map[string]string{} | ||
| if wev, ok := engine.(testdatastore.RunningEngineForTestWithEnvVars); ok { | ||
| envVars = wev.ExternalEnvVars() | ||
| for _, env := range wev.ExternalEnvVars() { | ||
| parts := strings.SplitN(env, "=", 2) | ||
| if len(parts) == 2 { | ||
| envVars[parts[0]] = parts[1] | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Run the migrate command and wait for it to complete. | ||
| db := engine.NewDatabase(t) | ||
| migrateResource, err := pool.RunWithOptions(&dockertest.RunOptions{ | ||
| Repository: "authzed/spicedb", | ||
| Tag: "ci", | ||
| Cmd: []string{"migrate", "head", "--datastore-engine", driverName, "--datastore-conn-uri", db}, | ||
| NetworkID: bridgeNetworkName, | ||
| Env: envVars, | ||
| }, func(config *docker.HostConfig) { | ||
| config.RestartPolicy = docker.RestartPolicy{ | ||
| Name: "no", | ||
| } | ||
| migrateContainer, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: given there are a few of this, you could abstract the container creation building your own test "framework", something like this: // pseudocode
func NewTestSpiceDB(ctx Context, opts ...ContainerOpts) {...} |
||
| ContainerRequest: testcontainers.ContainerRequest{ | ||
| Image: "authzed/spicedb:ci", | ||
| Cmd: []string{"migrate", "head", "--datastore-engine", driverName, "--datastore-conn-uri", db}, | ||
| Networks: []string{bridgeNetworkName}, | ||
| Env: envVars, | ||
| }, | ||
| Started: true, | ||
| }) | ||
| require.NoError(t, err) | ||
| t.Cleanup(func() { | ||
| _ = pool.Purge(migrateResource) | ||
| _ = migrateContainer.Terminate(ctx) | ||
| }) | ||
|
|
||
| // Ensure the command completed successfully. | ||
| status, err := pool.Client.WaitContainerWithContext(migrateResource.Container.ID, t.Context()) | ||
| exitCode, err := migrateContainer.State(ctx) | ||
| require.NoError(t, err) | ||
| require.Equal(t, 0, status) | ||
| require.Equal(t, 0, exitCode.ExitCode) | ||
|
|
||
| // Run a serve and immediately close, ensuring it shuts down gracefully. | ||
| serveResource, err := pool.RunWithOptions(&dockertest.RunOptions{ | ||
| Repository: "authzed/spicedb", | ||
| Tag: "ci", | ||
| Cmd: []string{"serve", "--grpc-preshared-key", "firstkey", "--datastore-engine", driverName, "--datastore-conn-uri", db, "--datastore-gc-interval", "1s", "--telemetry-endpoint", "", "--log-level", "trace", "--enable-experimental-watchable-schema-cache"}, | ||
| NetworkID: bridgeNetworkName, | ||
| Env: envVars, | ||
| }, func(config *docker.HostConfig) { | ||
| config.RestartPolicy = docker.RestartPolicy{ | ||
| Name: "no", | ||
| } | ||
| // TODO; | ||
| serveContainer, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{ | ||
| ContainerRequest: testcontainers.ContainerRequest{ | ||
| Image: "authzed/spicedb:ci", | ||
| Cmd: []string{"serve", "--grpc-preshared-key", "firstkey", "--datastore-engine", driverName, "--datastore-conn-uri", db, "--datastore-gc-interval", "1s", "--telemetry-endpoint", "", "--log-level", "trace", "--enable-experimental-watchable-schema-cache"}, | ||
| Networks: []string{bridgeNetworkName}, | ||
| Env: envVars, | ||
| }, | ||
| Started: true, | ||
| }) | ||
| require.NoError(t, err) | ||
| t.Cleanup(func() { | ||
| _ = pool.Purge(serveResource) | ||
| }) | ||
| testcontainers.CleanupContainer(t, serveContainer) | ||
|
|
||
| ww := &watchingWriter{make(chan bool, 1), "starting watching cache"} | ||
|
|
||
| // Grab logs and ensure GC has run before starting a graceful shutdown. | ||
| opts := docker.LogsOptions{ | ||
| Context: context.Background(), | ||
| Stderr: true, | ||
| Stdout: true, | ||
| Follow: true, | ||
| Timestamps: true, | ||
| RawTerminal: true, | ||
| Container: serveResource.Container.ID, | ||
| OutputStream: ww, | ||
| } | ||
|
|
||
| // Grab logs and ensure schema watch has started before graceful shutdown. | ||
| go (func() { | ||
| err = pool.Client.Logs(opts) | ||
| // TODO: do logging directly | ||
| logReader, err := serveContainer.Logs(ctx) | ||
| if err != nil { | ||
| assert.NoError(t, err) | ||
| return | ||
| } | ||
| defer logReader.Close() | ||
| _, err = io.Copy(ww, logReader) | ||
| assert.NoError(t, err) | ||
| })() | ||
|
|
||
| // TODO: what? | ||
| select { | ||
| case <-ww.c: | ||
| break | ||
|
|
||
| case <-time.After(10 * time.Second): | ||
| require.Fail(t, "timed out waiting for schema watch to run") | ||
| } | ||
|
|
||
| require.True(t, gracefulShutdown(pool, serveResource)) | ||
| }) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: use CleanupNetwork, and call it before require, as it handle nils