Skip to content

Commit

Permalink
chore: use require instead of t.Fatal (#2855)
Browse files Browse the repository at this point in the history
* chore: use require instead of t.Fatal

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>

* Apply suggestions from code review

Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>

* Update kafka_helpers_test.go

---------

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>
  • Loading branch information
mmorel-35 and mdelapenya authored Oct 29, 2024
1 parent 950d06b commit 09a5482
Show file tree
Hide file tree
Showing 19 changed files with 93 additions and 279 deletions.
4 changes: 1 addition & 3 deletions container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
Expand Down
61 changes: 16 additions & 45 deletions docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
})
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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.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)
if c != 0 {
t.Fatalf("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) {
Expand All @@ -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) {
Expand All @@ -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)
})
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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
}
Expand Down Expand Up @@ -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.Zerof(t, c, "File %s should exist, expected return code 0, got %v", tc.copiedFileName, c)
})
}
}
Expand Down Expand Up @@ -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.Zerof(t, c, "File %s should exist, expected return code 0, got %v", tc.copiedFileName, c)
})
}
}
Expand All @@ -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.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)
Expand Down Expand Up @@ -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.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)
Expand Down
32 changes: 8 additions & 24 deletions image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 {
Expand All @@ -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()
Expand All @@ -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.NotZerof(t, info.Size(), "output file is empty")
}
18 changes: 6 additions & 12 deletions logconsumer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()}

Expand Down Expand Up @@ -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{
Expand All @@ -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) {
Expand Down
8 changes: 2 additions & 6 deletions modules/influxdb/influxdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)

Expand Down
16 changes: 7 additions & 9 deletions modules/kafka/kafka_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package kafka
import (
"testing"

"github.com/stretchr/testify/require"

"github.com/testcontainers/testcontainers-go"
)

Expand Down Expand Up @@ -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"])
})
}
}
Expand Down Expand Up @@ -99,12 +99,10 @@ 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 && err != nil {
t.Fatalf("expected no error, got %s", err)
if test.wantErr {
require.Errorf(t, err, "expected error, got nil")
} else {
require.NoErrorf(t, err, "expected no error, got %s", err)
}
})
}
Expand Down
16 changes: 4 additions & 12 deletions modules/kafka/kafka_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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))
}
4 changes: 1 addition & 3 deletions modules/mariadb/mariadb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 09a5482

Please sign in to comment.