From 8424fb2e30418301cd54f01578890da5a56fbd99 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Tue, 29 Oct 2024 08:04:26 +0100 Subject: [PATCH 1/3] chore: use require instead of t.Fatal Signed-off-by: Matthieu MOREL --- container_test.go | 4 +- docker_test.go | 61 ++++++++-------------------- image_test.go | 32 ++++----------- logconsumer_test.go | 18 +++------ modules/influxdb/influxdb_test.go | 8 +--- modules/kafka/kafka_helpers_test.go | 14 +++---- modules/kafka/kafka_test.go | 16 ++------ modules/mariadb/mariadb_test.go | 4 +- modules/neo4j/neo4j_test.go | 49 ++++++----------------- modules/pulsar/pulsar_test.go | 4 +- modules/rabbitmq/rabbitmq_test.go | 36 +++++------------ modules/redis/redis_test.go | 8 +--- modules/valkey/valkey_test.go | 8 +--- modules/weaviate/weaviate_test.go | 12 ++---- port_forwarding_test.go | 12 ++---- provider_test.go | 6 +-- testing.go | 8 +--- wait/exec_test.go | 8 +--- wait/sql_test.go | 62 ++++------------------------- 19 files changed, 93 insertions(+), 277 deletions(-) diff --git a/container_test.go b/container_test.go index 72927bc55a..632ede9d26 100644 --- a/container_test.go +++ b/container_test.go @@ -137,9 +137,7 @@ func Test_GetDockerfile(t *testing.T) { for _, testCase := range testTable { t.Run(testCase.name, func(t *testing.T) { n := testCase.ContainerRequest.GetDockerfile() - if n != testCase.ExpectedDockerfileName { - t.Fatalf("expected Dockerfile name: %s, received: %s", testCase.ExpectedDockerfileName, n) - } + require.Equalf(t, n, testCase.ExpectedDockerfileName, "expected Dockerfile name: %s, received: %s", testCase.ExpectedDockerfileName, n) }) } } diff --git a/docker_test.go b/docker_test.go index 4be29f8fcd..9e97a4f45e 100644 --- a/docker_test.go +++ b/docker_test.go @@ -346,9 +346,7 @@ func TestContainerTerminationRemovesDockerImage(t *testing.T) { require.NoError(t, err) _, _, err = dockerClient.ImageInspectWithRaw(ctx, nginxAlpineImage) - if err != nil { - t.Fatal("nginx image should not have been removed") - } + require.NoErrorf(t, err, "nginx image should not have been removed") }) t.Run("if built from Dockerfile", func(t *testing.T) { @@ -380,9 +378,7 @@ func TestContainerTerminationRemovesDockerImage(t *testing.T) { require.NoError(t, err) _, _, err = dockerClient.ImageInspectWithRaw(ctx, imageID) - if err == nil { - t.Fatal("custom built image should have been removed", err) - } + require.Errorf(t, err, "custom built image should have been removed") }) } @@ -785,9 +781,7 @@ func TestContainerCreationWaitsForLogAndPortContextTimeout(t *testing.T) { Started: true, }) CleanupContainer(t, c) - if err == nil { - t.Fatal("Expected timeout") - } + require.Errorf(t, err, "Expected timeout") } func TestContainerCreationWaitingForHostPort(t *testing.T) { @@ -1139,9 +1133,7 @@ func TestContainerWithTmpFs(t *testing.T) { // exec_reader_example { c, reader, err := ctr.Exec(ctx, []string{"ls", path}) require.NoError(t, err) - if c != 1 { - t.Fatalf("File %s should not have existed, expected return code 1, got %v", path, c) - } + require.Equalf(t, 1, c, "File %s should not have existed, expected return code 1, got %v", path, c) buf := new(strings.Builder) _, err = io.Copy(buf, reader) @@ -1154,16 +1146,12 @@ func TestContainerWithTmpFs(t *testing.T) { // exec_example { c, _, err = ctr.Exec(ctx, []string{"touch", path}) require.NoError(t, err) - if c != 0 { - t.Fatalf("File %s should have been created successfully, expected return code 0, got %v", path, c) - } + require.Equalf(t, 0, c, "File %s should have been created successfully, expected return code 0, got %v", path, c) // } c, _, err = ctr.Exec(ctx, []string{"ls", path}) require.NoError(t, err) - if c != 0 { - t.Fatalf("File %s should exist, expected return code 0, got %v", path, c) - } + require.Equalf(t, 0, c, "File %s should exist, expected return code 0, got %v", path, c) } func TestContainerNonExistentImage(t *testing.T) { @@ -1177,9 +1165,7 @@ func TestContainerNonExistentImage(t *testing.T) { CleanupContainer(t, ctr) var nf errdefs.ErrNotFound - if !errors.As(err, &nf) { - t.Fatalf("the error should have been an errdefs.ErrNotFound: %v", err) - } + require.ErrorAsf(t, err, &nf, "the error should have been an errdefs.ErrNotFound: %v", err) }) t.Run("the context cancellation is propagated to container creation", func(t *testing.T) { @@ -1194,9 +1180,7 @@ func TestContainerNonExistentImage(t *testing.T) { Started: true, }) CleanupContainer(t, c) - if !errors.Is(err, ctx.Err()) { - t.Fatalf("err should be a ctx cancelled error %v", err) - } + require.ErrorIsf(t, err, ctx.Err(), "err should be a ctx cancelled error %v", err) }) } @@ -1267,9 +1251,8 @@ func TestContainerWithCustomHostname(t *testing.T) { CleanupContainer(t, ctr) require.NoError(t, err) - if actualHostname := readHostname(t, ctr.GetContainerID()); actualHostname != hostname { - t.Fatalf("expected hostname %s, got %s", hostname, actualHostname) - } + actualHostname := readHostname(t, ctr.GetContainerID()) + require.Equalf(t, actualHostname, hostname, "expected hostname %s, got %s", hostname, actualHostname) } func TestContainerInspect_RawInspectIsCleanedOnStop(t *testing.T) { @@ -1293,15 +1276,11 @@ func TestContainerInspect_RawInspectIsCleanedOnStop(t *testing.T) { func readHostname(tb testing.TB, containerId string) string { tb.Helper() containerClient, err := NewDockerClientWithOpts(context.Background()) - if err != nil { - tb.Fatalf("Failed to create Docker client: %v", err) - } + require.NoErrorf(tb, err, "Failed to create Docker client") defer containerClient.Close() containerDetails, err := containerClient.ContainerInspect(context.Background(), containerId) - if err != nil { - tb.Fatalf("Failed to inspect container: %v", err) - } + require.NoErrorf(tb, err, "Failed to inspect container") return containerDetails.Config.Hostname } @@ -1340,9 +1319,7 @@ func TestDockerContainerCopyFileToContainer(t *testing.T) { _ = nginxC.CopyFileToContainer(ctx, filepath.Join(".", "testdata", "hello.sh"), tc.copiedFileName, 700) c, _, err := nginxC.Exec(ctx, []string{"bash", tc.copiedFileName}) require.NoError(t, err) - if c != 0 { - t.Fatalf("File %s should exist, expected return code 0, got %v", tc.copiedFileName, c) - } + require.Equalf(t, 0, c, "File %s should exist, expected return code 0, got %v", tc.copiedFileName, c) }) } } @@ -1551,9 +1528,7 @@ func TestDockerContainerCopyToContainer(t *testing.T) { require.NoError(t, err) c, _, err := nginxC.Exec(ctx, []string{"bash", tc.copiedFileName}) require.NoError(t, err) - if c != 0 { - t.Fatalf("File %s should exist, expected return code 0, got %v", tc.copiedFileName, c) - } + require.Equalf(t, 0, c, "File %s should exist, expected return code 0, got %v", tc.copiedFileName, c) }) } } @@ -1579,9 +1554,7 @@ func TestDockerContainerCopyFileFromContainer(t *testing.T) { _ = nginxC.CopyFileToContainer(ctx, filepath.Join(".", "testdata", "hello.sh"), "/"+copiedFileName, 700) c, _, err := nginxC.Exec(ctx, []string{"bash", copiedFileName}) require.NoError(t, err) - if c != 0 { - t.Fatalf("File %s should exist, expected return code 0, got %v", copiedFileName, c) - } + require.Equalf(t, 0, c, "File %s should exist, expected return code 0, got %v", copiedFileName, c) reader, err := nginxC.CopyFileFromContainer(ctx, "/"+copiedFileName) require.NoError(t, err) @@ -1611,9 +1584,7 @@ func TestDockerContainerCopyEmptyFileFromContainer(t *testing.T) { _ = nginxC.CopyFileToContainer(ctx, filepath.Join(".", "testdata", "empty.sh"), "/"+copiedFileName, 700) c, _, err := nginxC.Exec(ctx, []string{"bash", copiedFileName}) require.NoError(t, err) - if c != 0 { - t.Fatalf("File %s should exist, expected return code 0, got %v", copiedFileName, c) - } + require.Equalf(t, 0, c, "File %s should exist, expected return code 0, got %v", copiedFileName, c) reader, err := nginxC.CopyFileFromContainer(ctx, "/"+copiedFileName) require.NoError(t, err) diff --git a/image_test.go b/image_test.go index 39f8134acd..0abb97270e 100644 --- a/image_test.go +++ b/image_test.go @@ -15,9 +15,7 @@ func TestImageList(t *testing.T) { t.Setenv("DOCKER_HOST", core.MustExtractDockerHost(context.Background())) provider, err := ProviderDocker.GetProvider() - if err != nil { - t.Fatalf("failed to get provider %v", err) - } + require.NoErrorf(t, err, "failed to get provider") defer func() { _ = provider.Close() @@ -29,18 +27,12 @@ func TestImageList(t *testing.T) { ctr, err := provider.CreateContainer(context.Background(), req) CleanupContainer(t, ctr) - if err != nil { - t.Fatalf("creating test container %v", err) - } + require.NoErrorf(t, err, "creating test container") images, err := provider.ListImages(context.Background()) - if err != nil { - t.Fatalf("listing images %v", err) - } + require.NoErrorf(t, err, "listing images") - if len(images) == 0 { - t.Fatal("no images retrieved") - } + require.NotEmptyf(t, images, "no images retrieved") // look if the list contains the container image for _, img := range images { @@ -56,9 +48,7 @@ func TestSaveImages(t *testing.T) { t.Setenv("DOCKER_HOST", core.MustExtractDockerHost(context.Background())) provider, err := ProviderDocker.GetProvider() - if err != nil { - t.Fatalf("failed to get provider %v", err) - } + require.NoErrorf(t, err, "failed to get provider") defer func() { _ = provider.Close() @@ -70,20 +60,14 @@ func TestSaveImages(t *testing.T) { ctr, err := provider.CreateContainer(context.Background(), req) CleanupContainer(t, ctr) - if err != nil { - t.Fatalf("creating test container %v", err) - } + require.NoErrorf(t, err, "creating test container") output := filepath.Join(t.TempDir(), "images.tar") err = provider.SaveImages(context.Background(), output, req.Image) - if err != nil { - t.Fatalf("saving image %q: %v", req.Image, err) - } + require.NoErrorf(t, err, "saving image %q", req.Image) info, err := os.Stat(output) require.NoError(t, err) - if info.Size() == 0 { - t.Fatalf("output file is empty") - } + require.NotEqualf(t, 0, info.Size(), "output file is empty") } diff --git a/logconsumer_test.go b/logconsumer_test.go index e4ee916e0f..900c8d8ae1 100644 --- a/logconsumer_test.go +++ b/logconsumer_test.go @@ -278,9 +278,7 @@ func TestContainerLogWithErrClosed(t *testing.T) { time.Sleep(10 * time.Millisecond) t.Log("retrying get endpoint") } - if err != nil { - t.Fatal("get endpoint:", err) - } + require.NoErrorf(t, err, "get endpoint") opts := []client.Opt{client.WithHost(remoteDocker), client.WithAPIVersionNegotiation()} @@ -333,9 +331,7 @@ func TestContainerLogWithErrClosed(t *testing.T) { hitNginx() time.Sleep(time.Second * 1) msgs := consumer.Msgs() - if len(msgs)-existingLogs != 1 { - t.Fatalf("logConsumer should have 1 new log message, instead has: %v", msgs[existingLogs:]) - } + require.Equalf(t, 1, len(msgs)-existingLogs, "logConsumer should have 1 new log message, instead has: %v", msgs[existingLogs:]) existingLogs = len(consumer.Msgs()) iptableArgs := []string{ @@ -357,12 +353,10 @@ func TestContainerLogWithErrClosed(t *testing.T) { hitNginx() time.Sleep(time.Second * 1) msgs = consumer.Msgs() - if len(msgs)-existingLogs != 2 { - t.Fatalf( - "LogConsumer should have 2 new log messages after detecting closed connection and"+ - " re-requesting logs. Instead has:\n%s", msgs[existingLogs:], - ) - } + require.Equalf(t, 2, len(msgs)-existingLogs, + "LogConsumer should have 2 new log messages after detecting closed connection and"+ + " re-requesting logs. Instead has:\n%s", msgs[existingLogs:], + ) } func TestContainerLogsShouldBeWithoutStreamHeader(t *testing.T) { diff --git a/modules/influxdb/influxdb_test.go b/modules/influxdb/influxdb_test.go index e04a800dc6..08d9a72126 100644 --- a/modules/influxdb/influxdb_test.go +++ b/modules/influxdb/influxdb_test.go @@ -43,9 +43,7 @@ func TestV2Container(t *testing.T) { state, err := influxDbContainer.State(ctx) require.NoError(t, err) - if !state.Running { - t.Fatal("InfluxDB container is not running") - } + require.Truef(t, state.Running, "InfluxDB container is not running") } func TestWithInitDb(t *testing.T) { @@ -72,9 +70,7 @@ func TestWithInitDb(t *testing.T) { response, err := cli.Query(q) require.NoError(t, err) - if response.Error() != nil { - t.Fatal(response.Error()) - } + require.NoError(t, response.Error()) testJson, err := json.Marshal(response.Results) require.NoError(t, err) diff --git a/modules/kafka/kafka_helpers_test.go b/modules/kafka/kafka_helpers_test.go index 4a49a00f50..3792d3fc72 100644 --- a/modules/kafka/kafka_helpers_test.go +++ b/modules/kafka/kafka_helpers_test.go @@ -3,6 +3,8 @@ package kafka import ( "testing" + "github.com/stretchr/testify/require" + "github.com/testcontainers/testcontainers-go" ) @@ -55,9 +57,7 @@ func TestConfigureQuorumVoters(t *testing.T) { t.Run(test.name, func(t *testing.T) { configureControllerQuorumVoters(test.req) - if test.req.Env["KAFKA_CONTROLLER_QUORUM_VOTERS"] != test.expectedVoters { - t.Fatalf("expected KAFKA_CONTROLLER_QUORUM_VOTERS to be %s, got %s", test.expectedVoters, test.req.Env["KAFKA_CONTROLLER_QUORUM_VOTERS"]) - } + require.Equalf(t, test.expectedVoters, test.req.Env["KAFKA_CONTROLLER_QUORUM_VOTERS"], "expected KAFKA_CONTROLLER_QUORUM_VOTERS to be %s, got %s", test.expectedVoters, test.req.Env["KAFKA_CONTROLLER_QUORUM_VOTERS"]) }) } } @@ -99,12 +99,12 @@ func TestValidateKRaftVersion(t *testing.T) { t.Run(test.name, func(t *testing.T) { err := validateKRaftVersion(test.image) - if test.wantErr && err == nil { - t.Fatalf("expected error, got nil") + if test.wantErr { + require.Errorf(t, err, "expected error, got nil") } - if !test.wantErr && err != nil { - t.Fatalf("expected no error, got %s", err) + if !test.wantErr { + require.NoErrorf(t, err, "expected no error, got %s", err) } }) } diff --git a/modules/kafka/kafka_test.go b/modules/kafka/kafka_test.go index c1fca69fb2..763f29e3e0 100644 --- a/modules/kafka/kafka_test.go +++ b/modules/kafka/kafka_test.go @@ -24,9 +24,7 @@ func TestKafka(t *testing.T) { assertAdvertisedListeners(t, kafkaContainer) - if !strings.EqualFold(kafkaContainer.ClusterID, "kraftCluster") { - t.Fatalf("expected clusterID to be %s, got %s", "kraftCluster", kafkaContainer.ClusterID) - } + require.Truef(t, strings.EqualFold(kafkaContainer.ClusterID, "kraftCluster"), "expected clusterID to be %s, got %s", "kraftCluster", kafkaContainer.ClusterID) // getBrokers { brokers, err := kafkaContainer.Brokers(ctx) @@ -65,12 +63,8 @@ func TestKafka(t *testing.T) { <-done - if !strings.EqualFold(string(consumer.message.Key), "key") { - t.Fatalf("expected key to be %s, got %s", "key", string(consumer.message.Key)) - } - if !strings.EqualFold(string(consumer.message.Value), "value") { - t.Fatalf("expected value to be %s, got %s", "value", string(consumer.message.Value)) - } + require.Truef(t, strings.EqualFold(string(consumer.message.Key), "key"), "expected key to be %s, got %s", "key", string(consumer.message.Key)) + require.Truef(t, strings.EqualFold(string(consumer.message.Value), "value"), "expected value to be %s, got %s", "value", string(consumer.message.Value)) } func TestKafka_invalidVersion(t *testing.T) { @@ -98,7 +92,5 @@ func assertAdvertisedListeners(t *testing.T, container testcontainers.Container) bs, err := io.ReadAll(r) require.NoError(t, err) - if !strings.Contains(string(bs), "BROKER://"+hostname+":9092") { - t.Fatalf("expected advertised listeners to contain %s, got %s", "BROKER://"+hostname+":9092", string(bs)) - } + require.Containsf(t, string(bs), "BROKER://"+hostname+":9092", "expected advertised listeners to contain %s, got %s", "BROKER://"+hostname+":9092", string(bs)) } diff --git a/modules/mariadb/mariadb_test.go b/modules/mariadb/mariadb_test.go index f042555e51..706ee2eb76 100644 --- a/modules/mariadb/mariadb_test.go +++ b/modules/mariadb/mariadb_test.go @@ -53,9 +53,7 @@ func TestMariaDBWithNonRootUserAndEmptyPassword(t *testing.T) { mariadb.WithDatabase("foo"), mariadb.WithUsername("test"), mariadb.WithPassword("")) - if err.Error() != "empty password can be used only with the root user" { - t.Fatal(err) - } + require.EqualError(t, err, "empty password can be used only with the root user") } func TestMariaDBWithRootUserAndEmptyPassword(t *testing.T) { diff --git a/modules/neo4j/neo4j_test.go b/modules/neo4j/neo4j_test.go index a5cff51182..9014eba6f5 100644 --- a/modules/neo4j/neo4j_test.go +++ b/modules/neo4j/neo4j_test.go @@ -29,9 +29,7 @@ func TestNeo4j(outer *testing.T) { driver := createDriver(t, ctx, ctr) err := driver.VerifyConnectivity(ctx) - if err != nil { - t.Fatalf("should have successfully connected to server but did not: %s", err) - } + require.NoErrorf(t, err, "should have successfully connected to server but did not") }) outer.Run("exercises APOC plugin", func(t *testing.T) { @@ -40,9 +38,7 @@ func TestNeo4j(outer *testing.T) { result, err := neo.ExecuteQuery(ctx, driver, "RETURN apoc.number.arabicToRoman(1986) AS output", nil, neo.EagerResultTransformer) - if err != nil { - t.Fatalf("expected APOC query to successfully run but did not: %s", err) - } + require.NoErrorf(t, err, "expected APOC query to successfully run but did not") if value, _ := result.Records[0].Get("output"); value != "MCMLXXXVI" { t.Fatalf("did not get expected roman number: %s", value) } @@ -51,9 +47,7 @@ func TestNeo4j(outer *testing.T) { outer.Run("is configured with custom Neo4j settings", func(t *testing.T) { env := getContainerEnv(t, ctx, ctr) - if !strings.Contains(env, "NEO4J_dbms_tx__log_rotation_size=42M") { - t.Fatal("expected to custom setting to be exported but was not") - } + require.Containsf(t, env, "NEO4J_dbms_tx__log_rotation_size=42M", "expected to custom setting to be exported but was not") }) } @@ -81,9 +75,7 @@ func TestNeo4jWithEnterpriseLicense(t *testing.T) { env := getContainerEnv(t, ctx, ctr) - if !strings.Contains(env, "NEO4J_ACCEPT_LICENSE_AGREEMENT=yes") { - t.Fatal("expected to accept license agreement but did not") - } + require.Containsf(t, env, "NEO4J_ACCEPT_LICENSE_AGREEMENT=yes", "expected to accept license agreement but did not") }) } } @@ -106,12 +98,8 @@ func TestNeo4jWithWrongSettings(outer *testing.T) { neo4j.WithNeo4jSetting("AUTH", "neo4j/thisisgonnafail"), ) testcontainers.CleanupContainer(t, ctr) - if err == nil { - t.Fatalf("expected env to fail due to conflicting auth settings but did not") - } - if ctr != nil { - t.Fatalf("container must not be created with conflicting auth settings") - } + require.Errorf(t, err, "expected env to fail due to conflicting auth settings but did not") + require.Nilf(t, ctr, "container must not be created with conflicting auth settings") }) outer.Run("warns about overwrites of setting keys", func(t *testing.T) { @@ -134,17 +122,13 @@ func TestNeo4jWithWrongSettings(outer *testing.T) { !Contains(errorLogs, `setting "some.key" with value "value2" is now overwritten with value "value3"`+"\n") { t.Fatalf("expected setting overwrites to be logged") } - if !strings.Contains(getContainerEnv(t, ctx, ctr), "NEO4J_some_key=value3") { - t.Fatalf("expected custom setting to be set with last value") - } + require.Containsf(t, getContainerEnv(t, ctx, ctr), "NEO4J_some_key=value3", "expected custom setting to be set with last value") }) outer.Run("rejects nil logger", func(t *testing.T) { ctr, err := neo4j.Run(ctx, "neo4j:4.4", neo4j.WithLogger(nil)) testcontainers.CleanupContainer(t, ctr) - if ctr != nil { - t.Fatalf("container must not be created with nil logger") - } + require.Nilf(t, ctr, "container must not be created with nil logger") if err == nil || err.Error() != "nil logger is not permitted" { t.Fatalf("expected config validation error but got no error") } @@ -171,9 +155,8 @@ func createDriver(t *testing.T, ctx context.Context, container *neo4j.Neo4jConta driver, err := neo.NewDriverWithContext(boltUrl, neo.BasicAuth("neo4j", testPassword, "")) require.NoError(t, err) t.Cleanup(func() { - if err := driver.Close(ctx); err != nil { - t.Fatalf("failed to close neo: %s", err) - } + err := driver.Close(ctx) + require.NoErrorf(t, err, "failed to close neo: %s", err) }) return driver } @@ -181,16 +164,10 @@ func createDriver(t *testing.T, ctx context.Context, container *neo4j.Neo4jConta func getContainerEnv(t *testing.T, ctx context.Context, container *neo4j.Neo4jContainer) string { t.Helper() exec, reader, err := container.Exec(ctx, []string{"env"}) - if err != nil { - t.Fatalf("expected env to successfully run but did not: %s", err) - } - if exec != 0 { - t.Fatalf("expected env to exit with status 0 but exited with: %d", exec) - } + require.NoErrorf(t, err, "expected env to successfully run but did not") + require.Equalf(t, 0, exec, "expected env to exit with status 0 but exited with: %d", exec) envVars, err := io.ReadAll(reader) - if err != nil { - t.Fatalf("expected to read all bytes from env output but did not: %s", err) - } + require.NoErrorf(t, err, "expected to read all bytes from env output but did not") return string(envVars) } diff --git a/modules/pulsar/pulsar_test.go b/modules/pulsar/pulsar_test.go index 8cea6a4119..de2c4bd437 100644 --- a/modules/pulsar/pulsar_test.go +++ b/modules/pulsar/pulsar_test.go @@ -166,9 +166,7 @@ func TestPulsar(t *testing.T) { case <-ticker.C: t.Fatal("did not receive message in time") case msg := <-msgChan: - if string(msg) != "hello world" { - t.Fatal("received unexpected message bytes") - } + require.Equalf(t, "hello world", string(msg), "received unexpected message bytes") } // get topic statistics using the Admin endpoint diff --git a/modules/rabbitmq/rabbitmq_test.go b/modules/rabbitmq/rabbitmq_test.go index 6c4cca0c8c..45c0f5e2c1 100644 --- a/modules/rabbitmq/rabbitmq_test.go +++ b/modules/rabbitmq/rabbitmq_test.go @@ -46,9 +46,7 @@ func TestRunContainer_connectUsingAmqps(t *testing.T) { IsCA: true, ParentDir: tmpDir, }) - if caCert == nil { - t.Fatal("failed to generate CA certificate") - } + require.NotNilf(t, caCert, "failed to generate CA certificate") cert := tlscert.SelfSignedFromRequest(tlscert.Request{ Name: "client", @@ -57,9 +55,7 @@ func TestRunContainer_connectUsingAmqps(t *testing.T) { Parent: caCert, ParentDir: tmpDir, }) - if cert == nil { - t.Fatal("failed to generate certificate") - } + require.NotNilf(t, cert, "failed to generate certificate") sslSettings := rabbitmq.SSLSettings{ CACertFile: caCert.CertPath, @@ -216,27 +212,13 @@ func TestRunContainer_withAllSettings(t *testing.T) { testcontainers.CleanupContainer(t, rabbitmqContainer) require.NoError(t, err) - if !assertEntity(t, rabbitmqContainer, "queues", "queue1", "queue2", "queue3", "queue4") { - t.Fatal(err) - } - if !assertEntity(t, rabbitmqContainer, "exchanges", "direct-exchange", "topic-exchange", "topic-exchange-2", "topic-exchange-3", "topic-exchange-4") { - t.Fatal(err) - } - if !assertEntity(t, rabbitmqContainer, "users", "user1", "user2") { - t.Fatal(err) - } - if !assertEntity(t, rabbitmqContainer, "policies", "max length policy", "alternate exchange policy") { - t.Fatal(err) - } - if !assertEntityWithVHost(t, rabbitmqContainer, "policies", 2, "max length policy", "alternate exchange policy") { - t.Fatal(err) - } - if !assertEntity(t, rabbitmqContainer, "operator_policies", "operator policy 1") { - t.Fatal(err) - } - if !assertPluginIsEnabled(t, rabbitmqContainer, "rabbitmq_shovel", "rabbitmq_random_exchange") { - t.Fatal(err) - } + require.True(t, assertEntity(t, rabbitmqContainer, "queues", "queue1", "queue2", "queue3", "queue4")) + require.True(t, assertEntity(t, rabbitmqContainer, "exchanges", "direct-exchange", "topic-exchange", "topic-exchange-2", "topic-exchange-3", "topic-exchange-4")) + require.True(t, assertEntity(t, rabbitmqContainer, "users", "user1", "user2")) + require.True(t, assertEntity(t, rabbitmqContainer, "policies", "max length policy", "alternate exchange policy")) + require.True(t, assertEntityWithVHost(t, rabbitmqContainer, "policies", 2, "max length policy", "alternate exchange policy")) + require.True(t, assertEntity(t, rabbitmqContainer, "operator_policies", "operator policy 1")) + require.True(t, assertPluginIsEnabled(t, rabbitmqContainer, "rabbitmq_shovel", "rabbitmq_random_exchange")) } func assertEntity(t *testing.T, container testcontainers.Container, listCommand string, entities ...string) bool { diff --git a/modules/redis/redis_test.go b/modules/redis/redis_test.go index 58d1c14c9c..e2352a1bf6 100644 --- a/modules/redis/redis_test.go +++ b/modules/redis/redis_test.go @@ -120,9 +120,7 @@ func assertSetsGets(t *testing.T, ctx context.Context, redisContainer *tcredis.R t.Log("received response from redis") - if pong != "PONG" { - t.Fatalf("received unexpected response from redis: %s", pong) - } + require.Equalf(t, "PONG", pong, "received unexpected response from redis: %s", pong) for i := 0; i < keyCount; i++ { // Set data @@ -137,9 +135,7 @@ func assertSetsGets(t *testing.T, ctx context.Context, redisContainer *tcredis.R savedValue, err := client.Get(ctx, key).Result() require.NoError(t, err) - if savedValue != value { - t.Fatalf("Expected value %s. Got %s.", savedValue, value) - } + require.Equal(t, savedValue, value) } } diff --git a/modules/valkey/valkey_test.go b/modules/valkey/valkey_test.go index 26e2ea93b8..44412afa4a 100644 --- a/modules/valkey/valkey_test.go +++ b/modules/valkey/valkey_test.go @@ -109,9 +109,7 @@ func assertSetsGets(t *testing.T, ctx context.Context, valkeyContainer *tcvalkey msg, err := res.ToString() require.NoError(t, err) - if msg != "PONG" { - t.Fatalf("received unexpected response from valkey: %s", res.String()) - } + require.Equalf(t, "PONG", msg, "received unexpected response from valkey: %s", res.String()) for i := 0; i < keyCount; i++ { // Set data @@ -132,9 +130,7 @@ func assertSetsGets(t *testing.T, ctx context.Context, valkeyContainer *tcvalkey retVal, err := resp.ToString() require.NoError(t, err) - if retVal != value { - t.Fatalf("Expected value %s. Got %s.", value, retVal) - } + require.Equal(t, retVal, value) } } diff --git a/modules/weaviate/weaviate_test.go b/modules/weaviate/weaviate_test.go index faf8e42f92..f1d6de3eeb 100644 --- a/modules/weaviate/weaviate_test.go +++ b/modules/weaviate/weaviate_test.go @@ -48,17 +48,11 @@ func TestWeaviate(t *testing.T) { opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials())) conn, err := grpc.NewClient(host, opts...) - if err != nil { - tt.Fatalf("failed to dial connection: %v", err) - } + require.NoErrorf(t, err, "failed to dial connection") client := grpc_health_v1.NewHealthClient(conn) check, err := client.Check(context.TODO(), &grpc_health_v1.HealthCheckRequest{}) - if err != nil { - tt.Fatalf("failed to get a health check: %v", err) - } - if grpc_health_v1.HealthCheckResponse_SERVING.Enum().Number() != check.Status.Number() { - tt.Fatalf("unexpected status code: %d", check.Status.Number()) - } + require.NoErrorf(t, err, "failed to get a health check") + require.Equalf(t, grpc_health_v1.HealthCheckResponse_SERVING.Enum().Number(), check.Status.Number(), "unexpected status code: %d", check.Status.Number()) }) t.Run("Weaviate client", func(tt *testing.T) { diff --git a/port_forwarding_test.go b/port_forwarding_test.go index 8f8681f22c..d2b24abb93 100644 --- a/port_forwarding_test.go +++ b/port_forwarding_test.go @@ -133,13 +133,9 @@ func assertContainerHasHostAccess(t *testing.T, c testcontainers.Container, port t.Helper() for _, port := range ports { code, response := httpRequest(t, c, port) - if code != 0 { - t.Fatalf("expected status code [%d] but got [%d]", 0, code) - } + require.Equalf(t, 0, code, "expected status code [%d] but got [%d]", 0, code) - if response != expectedResponse { - t.Fatalf("expected [%s] but got [%s]", expectedResponse, response) - } + require.Equalf(t, expectedResponse, response, "expected [%s] but got [%s]", expectedResponse, response) } } @@ -148,8 +144,6 @@ func assertContainerHasNoHostAccess(t *testing.T, c testcontainers.Container, po for _, port := range ports { _, response := httpRequest(t, c, port) - if response == expectedResponse { - t.Fatalf("expected not to get [%s] but got [%s]", expectedResponse, response) - } + require.NotEqualf(t, expectedResponse, response, "expected not to get [%s] but got [%s]", expectedResponse, response) } } diff --git a/provider_test.go b/provider_test.go index 097c83a02c..e4937a49d0 100644 --- a/provider_test.go +++ b/provider_test.go @@ -4,6 +4,8 @@ import ( "context" "testing" + "github.com/stretchr/testify/require" + "github.com/testcontainers/testcontainers-go/internal/core" ) @@ -70,9 +72,7 @@ func TestProviderTypeGetProviderAutodetect(t *testing.T) { return } provider, ok := got.(*DockerProvider) - if !ok { - t.Fatalf("ProviderType.GetProvider() = %T, want %T", got, &DockerProvider{}) - } + require.Truef(t, ok, "ProviderType.GetProvider() = %T, want %T", got, &DockerProvider{}) if provider.defaultBridgeNetworkName != tt.want { t.Errorf("ProviderType.GetProvider() = %v, want %v", provider.defaultBridgeNetworkName, tt.want) } diff --git a/testing.go b/testing.go index 0601d9fa83..1670d7c059 100644 --- a/testing.go +++ b/testing.go @@ -36,14 +36,10 @@ func SkipIfProviderIsNotHealthy(t *testing.T) { func SkipIfDockerDesktop(t *testing.T, ctx context.Context) { t.Helper() cli, err := NewDockerClientWithOpts(ctx) - if err != nil { - t.Fatalf("failed to create docker client: %s", err) - } + require.NoErrorf(t, err, "failed to create docker client: %s", err) info, err := cli.Info(ctx) - if err != nil { - t.Fatalf("failed to get docker info: %s", err) - } + require.NoErrorf(t, err, "failed to get docker info: %s", err) if info.OperatingSystem == "Docker Desktop" { t.Skip("Skipping test that requires host network access when running in Docker Desktop") diff --git a/wait/exec_test.go b/wait/exec_test.go index ab84fabd17..a431b146da 100644 --- a/wait/exec_test.go +++ b/wait/exec_test.go @@ -144,9 +144,7 @@ func TestExecStrategyWaitUntilReady_DeadlineExceeded(t *testing.T) { } wg := wait.NewExecStrategy([]string{"true"}) err := wg.WaitUntilReady(ctx, target) - if !errors.Is(err, context.DeadlineExceeded) { - t.Fatal(err) - } + require.ErrorIs(t, err, context.DeadlineExceeded) } func TestExecStrategyWaitUntilReady_CustomExitCode(t *testing.T) { @@ -174,9 +172,7 @@ func TestExecStrategyWaitUntilReady_withExitCode(t *testing.T) { wg = wait.NewExecStrategy([]string{"true"}).WithExitCode(0) wg.WithStartupTimeout(time.Second * 2) err = wg.WaitUntilReady(context.Background(), target) - if err == nil { - t.Fatalf("Expected strategy to timeout out") - } + require.Errorf(t, err, "Expected strategy to timeout out") } func TestExecStrategyWaitUntilReady_CustomResponseMatcher(t *testing.T) { diff --git a/wait/sql_test.go b/wait/sql_test.go index 2a7db5b330..63179ee8ab 100644 --- a/wait/sql_test.go +++ b/wait/sql_test.go @@ -18,9 +18,7 @@ func Test_waitForSql_WithQuery(t *testing.T) { return "fake-url" }) - if got := w.query; got != defaultForSqlQuery { - t.Fatalf("expected %s, got %s", defaultForSqlQuery, got) - } + require.Equal(t, defaultForSqlQuery, w.query) }) t.Run("custom query", func(t *testing.T) { const q = "SELECT 100;" @@ -29,9 +27,7 @@ func Test_waitForSql_WithQuery(t *testing.T) { return "fake-url" }).WithQuery(q) - if got := w.query; got != q { - t.Fatalf("expected %s, got %s", q, got) - } + require.Equal(t, q, w.query) }) } @@ -133,14 +129,7 @@ func TestWaitForSQLFailsWhileGettingPortDueToOOMKilledContainer(t *testing.T) { { err := wg.WaitUntilReady(context.Background(), target) - if err == nil { - t.Fatal("no error") - } - - expected := "container crashed with out-of-memory (OOMKilled)" - if err.Error() != expected { - t.Fatalf("expected %q, got %q", expected, err.Error()) - } + require.EqualError(t, err, "container crashed with out-of-memory (OOMKilled)") } } @@ -171,14 +160,7 @@ func TestWaitForSQLFailsWhileGettingPortDueToExitedContainer(t *testing.T) { { err := wg.WaitUntilReady(context.Background(), target) - if err == nil { - t.Fatal("no error") - } - - expected := "container exited with code 1" - if err.Error() != expected { - t.Fatalf("expected %q, got %q", expected, err.Error()) - } + require.EqualError(t, err, "container exited with code 1") } } @@ -208,14 +190,7 @@ func TestWaitForSQLFailsWhileGettingPortDueToUnexpectedContainerStatus(t *testin { err := wg.WaitUntilReady(context.Background(), target) - if err == nil { - t.Fatal("no error") - } - - expected := "unexpected container status \"dead\"" - if err.Error() != expected { - t.Fatalf("expected %q, got %q", expected, err.Error()) - } + require.EqualError(t, err, "unexpected container status \"dead\"") } } @@ -240,14 +215,7 @@ func TestWaitForSQLFailsWhileQueryExecutingDueToOOMKilledContainer(t *testing.T) { err := wg.WaitUntilReady(context.Background(), target) - if err == nil { - t.Fatal("no error") - } - - expected := "container crashed with out-of-memory (OOMKilled)" - if err.Error() != expected { - t.Fatalf("expected %q, got %q", expected, err.Error()) - } + require.EqualError(t, err, "container crashed with out-of-memory (OOMKilled)") } } @@ -273,14 +241,7 @@ func TestWaitForSQLFailsWhileQueryExecutingDueToExitedContainer(t *testing.T) { { err := wg.WaitUntilReady(context.Background(), target) - if err == nil { - t.Fatal("no error") - } - - expected := "container exited with code 1" - if err.Error() != expected { - t.Fatalf("expected %q, got %q", expected, err.Error()) - } + require.EqualError(t, err, "container exited with code 1") } } @@ -305,13 +266,6 @@ func TestWaitForSQLFailsWhileQueryExecutingDueToUnexpectedContainerStatus(t *tes { err := wg.WaitUntilReady(context.Background(), target) - if err == nil { - t.Fatal("no error") - } - - expected := "unexpected container status \"dead\"" - if err.Error() != expected { - t.Fatalf("expected %q, got %q", expected, err.Error()) - } + require.EqualError(t, err, "unexpected container status \"dead\"") } } From ba74bbb7d48148cd3fbe313595925eb3fa745ebc Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Tue, 29 Oct 2024 10:25:58 +0100 Subject: [PATCH 2/3] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel de la Peña --- docker_test.go | 12 ++++++------ image_test.go | 2 +- modules/neo4j/neo4j_test.go | 2 +- port_forwarding_test.go | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docker_test.go b/docker_test.go index 9e97a4f45e..dc3b245d1b 100644 --- a/docker_test.go +++ b/docker_test.go @@ -1146,12 +1146,12 @@ func TestContainerWithTmpFs(t *testing.T) { // exec_example { c, _, err = ctr.Exec(ctx, []string{"touch", path}) require.NoError(t, err) - require.Equalf(t, 0, c, "File %s should have been created successfully, expected return code 0, got %v", path, c) + require.Zerof(t, c, "File %s should have been created successfully, expected return code 0, got %v", path, c) // } c, _, err = ctr.Exec(ctx, []string{"ls", path}) require.NoError(t, err) - require.Equalf(t, 0, c, "File %s should exist, expected return code 0, got %v", path, c) + require.Zerof(t, c, "File %s should exist, expected return code 0, got %v", path, c) } func TestContainerNonExistentImage(t *testing.T) { @@ -1319,7 +1319,7 @@ func TestDockerContainerCopyFileToContainer(t *testing.T) { _ = nginxC.CopyFileToContainer(ctx, filepath.Join(".", "testdata", "hello.sh"), tc.copiedFileName, 700) c, _, err := nginxC.Exec(ctx, []string{"bash", tc.copiedFileName}) require.NoError(t, err) - require.Equalf(t, 0, c, "File %s should exist, expected return code 0, got %v", tc.copiedFileName, c) + require.Zerof(t, c, "File %s should exist, expected return code 0, got %v", tc.copiedFileName, c) }) } } @@ -1528,7 +1528,7 @@ func TestDockerContainerCopyToContainer(t *testing.T) { require.NoError(t, err) c, _, err := nginxC.Exec(ctx, []string{"bash", tc.copiedFileName}) require.NoError(t, err) - require.Equalf(t, 0, c, "File %s should exist, expected return code 0, got %v", tc.copiedFileName, c) + require.Zerof(t, c, "File %s should exist, expected return code 0, got %v", tc.copiedFileName, c) }) } } @@ -1554,7 +1554,7 @@ func TestDockerContainerCopyFileFromContainer(t *testing.T) { _ = nginxC.CopyFileToContainer(ctx, filepath.Join(".", "testdata", "hello.sh"), "/"+copiedFileName, 700) c, _, err := nginxC.Exec(ctx, []string{"bash", copiedFileName}) require.NoError(t, err) - require.Equalf(t, 0, c, "File %s should exist, expected return code 0, got %v", copiedFileName, c) + require.Zerof(t, c, "File %s should exist, expected return code 0, got %v", copiedFileName, c) reader, err := nginxC.CopyFileFromContainer(ctx, "/"+copiedFileName) require.NoError(t, err) @@ -1584,7 +1584,7 @@ func TestDockerContainerCopyEmptyFileFromContainer(t *testing.T) { _ = nginxC.CopyFileToContainer(ctx, filepath.Join(".", "testdata", "empty.sh"), "/"+copiedFileName, 700) c, _, err := nginxC.Exec(ctx, []string{"bash", copiedFileName}) require.NoError(t, err) - require.Equalf(t, 0, c, "File %s should exist, expected return code 0, got %v", copiedFileName, c) + require.Zerof(t, c, "File %s should exist, expected return code 0, got %v", copiedFileName, c) reader, err := nginxC.CopyFileFromContainer(ctx, "/"+copiedFileName) require.NoError(t, err) diff --git a/image_test.go b/image_test.go index 0abb97270e..b5a95640a8 100644 --- a/image_test.go +++ b/image_test.go @@ -69,5 +69,5 @@ func TestSaveImages(t *testing.T) { info, err := os.Stat(output) require.NoError(t, err) - require.NotEqualf(t, 0, info.Size(), "output file is empty") + require.NotZerof(t, info.Size(), "output file is empty") } diff --git a/modules/neo4j/neo4j_test.go b/modules/neo4j/neo4j_test.go index 9014eba6f5..5918b7dc31 100644 --- a/modules/neo4j/neo4j_test.go +++ b/modules/neo4j/neo4j_test.go @@ -165,7 +165,7 @@ func getContainerEnv(t *testing.T, ctx context.Context, container *neo4j.Neo4jCo t.Helper() exec, reader, err := container.Exec(ctx, []string{"env"}) require.NoErrorf(t, err, "expected env to successfully run but did not") - require.Equalf(t, 0, exec, "expected env to exit with status 0 but exited with: %d", exec) + require.Zerof(t, exec, "expected env to exit with status 0 but exited with: %d", exec) envVars, err := io.ReadAll(reader) require.NoErrorf(t, err, "expected to read all bytes from env output but did not") return string(envVars) diff --git a/port_forwarding_test.go b/port_forwarding_test.go index d2b24abb93..2936a46457 100644 --- a/port_forwarding_test.go +++ b/port_forwarding_test.go @@ -133,7 +133,7 @@ func assertContainerHasHostAccess(t *testing.T, c testcontainers.Container, port t.Helper() for _, port := range ports { code, response := httpRequest(t, c, port) - require.Equalf(t, 0, code, "expected status code [%d] but got [%d]", 0, code) + require.Zerof(t, code, "expected status code [%d] but got [%d]", 0, code) require.Equalf(t, expectedResponse, response, "expected [%s] but got [%s]", expectedResponse, response) } From aeeb7abb2f42537131617ec39ba2d61e8511463f Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Tue, 29 Oct 2024 10:30:14 +0100 Subject: [PATCH 3/3] Update kafka_helpers_test.go --- modules/kafka/kafka_helpers_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/modules/kafka/kafka_helpers_test.go b/modules/kafka/kafka_helpers_test.go index 3792d3fc72..6ef7deb60f 100644 --- a/modules/kafka/kafka_helpers_test.go +++ b/modules/kafka/kafka_helpers_test.go @@ -101,9 +101,7 @@ func TestValidateKRaftVersion(t *testing.T) { if test.wantErr { require.Errorf(t, err, "expected error, got nil") - } - - if !test.wantErr { + } else { require.NoErrorf(t, err, "expected no error, got %s", err) } })