GenericContainer: in case of error: return a reference to the failed container

JordanP · JordanP · commit f150875d4c69 · 2024-01-09T13:09:11.000+01:00
This way, the caller has an opportunity to clean things up and call
Destroy on the failed container.
diff --git a/generic.go b/generic.go
@@ -74,7 +74,8 @@ func GenericContainer(ctx context.Context, req GenericContainerRequest) (Contain
 		c, err = provider.CreateContainer(ctx, req.ContainerRequest)
 	}
 	if err != nil {
-		return nil, fmt.Errorf("%w: failed to create container", err)
+		// At this point `c` might not be nil. Give the caller an opportunity to call Destroy on the container.
+		return c, fmt.Errorf("%w: failed to create container", err)
 	}
 
 	if req.Started && !c.IsRunning() {
diff --git a/generic_test.go b/generic_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -96,3 +97,23 @@ func TestGenericReusableContainer(t *testing.T) {
 		})
 	}
 }
+
+func TestGenericContainerShouldReturnRefOnError(t *testing.T) {
+	// In this test, we are going to cancel the context to exit the `wait.Strategy`.
+	// We want to make sure that the GenericContainer call will still return a reference to the
+	// created container, so that we can Destroy it.
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+
+	c, err := GenericContainer(ctx, GenericContainerRequest{
+		ProviderType: providerType,
+		ContainerRequest: ContainerRequest{
+			Image:      nginxAlpineImage,
+			WaitingFor: wait.ForLog("this string should not be present in the logs"),
+		},
+		Started: true,
+	})
+	require.Error(t, err)
+	require.NotNil(t, c)
+	terminateContainerOnEnd(t, context.Background(), c)
+}

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,8 @@ func GenericContainer(ctx context.Context, req GenericContainerRequest) (Contain`
`74`	`74`	`c, err = provider.CreateContainer(ctx, req.ContainerRequest)`
`75`	`75`	`}`
`76`	`76`	`if err != nil {`
`77`		`- return nil, fmt.Errorf("%w: failed to create container", err)`
	`77`	+ // At this point `c` might not be nil. Give the caller an opportunity to call Destroy on the container.
	`78`	`+ return c, fmt.Errorf("%w: failed to create container", err)`
`78`	`79`	`}`
`79`	`80`
`80`	`81`	`if req.Started && !c.IsRunning() {`