chore: enable usetesting linter #3316
Conversation
|
Warning Rate limit exceeded@mmorel-35 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (107)
Summary by CodeRabbit
No user-facing functionality changes; behavior remains the same outside of improved test stability and tooling. WalkthroughRefactors tests to derive contexts from testing.T via t.Context() instead of context.Background()/TODO across the codebase, updates examples to use a shared ctx, and adds the “usetesting” linter with configuration. A few test helper functions’ signatures now accept a context parameter to propagate the test context. Changes
Sequence Diagram(s)sequenceDiagram
participant T as testing.T
participant Test as Test Code
participant Prov as Testcontainers Provider
participant Svc as Service/Client
T->>Test: t.Context()
Test->>Prov: Run(ctx)
Prov-->>Test: Container
Test->>Prov: Endpoint/Inspect/Exec(ctx)
Test->>Svc: Client Ops (ctx)
note over Test,Svc: ctx carries test deadlines/cancellation
T-->>Test: Test ends → ctx canceled
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
mmorel-35
force-pushed
the
usetesting
branch
3 times, most recently
from
8324faf to
246e83d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (17)
modules/artemis/artemis_test.go (1)
17-121: Use subtest-local contextDeriving
ctxfrom the parenttmeans every subtest shares the same parent context, so operations won’t be canceled when an individual subtest ends (only when the outer test finishes). That reintroduces the background-context behavior we’re trying to eliminate. Please grab a freshctx := t.Context()inside eacht.Runclosure instead.- ctx := t.Context() - for _, test := range tests { t.Run(test.name, func(t *testing.T) { - ctr, err := artemis.Run(ctx, "apache/activemq-artemis:2.30.0-alpine", test.opts...) + ctx := t.Context() + + ctr, err := artemis.Run(ctx, "apache/activemq-artemis:2.30.0-alpine", test.opts...) testcontainers.CleanupContainer(t, ctr) require.NoError(t, err) // consoleURL { u, err := ctr.ConsoleURL(ctx) @@ // brokerEndpoint { host, err := ctr.BrokerEndpoint(ctx)modules/couchbase/couchbase_test.go (1)
23-39: Avoidt.Context()until the repo requires Go 1.22+
testing.T.Context()only exists starting with Go 1.22. The module’sgo.modstill targets Go 1.21, so this change breaks builds on the currently supported toolchain. Please keep usingcontext.Background()(or add an adapter guarded by Go build tags) until the minimum Go version is bumped.Apply the following diff to revert to
context.Background():- ctx := t.Context() + ctx := context.Background()Repeat this fix for every test in this file that now calls
t.Context().Also applies to: 46-62, 68-87, 111-143
modules/etcd/etcd_unit_test.go (1)
86-110: Use the subtest’s tt for context, cleanup and assertions (avoid parent T).Inside the returned subtest func, derive ctx from tt, and pass tt to CleanupContainer and all require.* calls. Using the parent t leaks cleanup to the parent scope and ties ctx to the wrong test.
Apply this diff:
- ctx := t.Context() + ctx := tt.Context() - testcontainers.CleanupContainer(t, ctr) + testcontainers.CleanupContainer(tt, ctr) @@ - require.Empty(t, node.childNodes) // child nodes has no children + require.Empty(tt, node.childNodes) // child nodes has no children @@ - require.NoError(t, err) - require.Contains(t, string(output), fmt.Sprintf("etcd-%d", i+1)) + require.NoError(tt, err) + require.Contains(tt, string(output), fmt.Sprintf("etcd-%d", i+1))This also better satisfies the ustesting linter’s expectations for per-test context usage.
modules/nebulagraph/nebulagraph_test.go (1)
35-35: Fix cleanup to avoid using a canceled context (can leak containers).
defer cancel()(Line 27) runs beforet.Cleanup, soctxis canceled whenTerminateexecutes, risking a no-op and leaked containers. Use a fresh context for cleanup and assert errors.Apply this diff:
- t.Cleanup(func() { _ = container.Terminate(ctx) }) + t.Cleanup(func() { + cleanupCtx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + require.NoError(t, container.Terminate(cleanupCtx)) + })modules/localstack/v2/s3_test.go (1)
133-136: Fix failing assertion: pointer vs value type mismatch.In AWS SDK v2, Object.Size is int64 (not *int64). Current assert compares *int64 to int64, which will fail.
Apply:
- assert.Equal(t, aws.Int64(int64(len(body1))), objects[0].Size) + assert.Equal(t, int64(len(body1)), objects[0].Size)modules/redpanda/redpanda_test.go (2)
500-505: Fix variable used in Schema Registry TLS assertion.The check uses adminAPIURL instead of schemaRegistryURL, so it’s not validating the intended endpoint.
Apply:
- require.True(t, strings.HasPrefix(adminAPIURL, "https://"), "SchemaRegistryAddress should return https url") + require.True(t, strings.HasPrefix(schemaRegistryURL, "https://"), "SchemaRegistryAddress should return https url")
87-90: Assert the expected error type with require.ErrorIs.Currently require.Error is only checking that some error occurred; it does not verify UnknownTopicOrPartition. Use ErrorIs to assert the specific error.
Apply:
- require.Error(t, results.FirstErr(), kerr.UnknownTopicOrPartition) + require.ErrorIs(t, results.FirstErr(), kerr.UnknownTopicOrPartition)Also applies to: 521-524
modules/dynamodb/dynamodb_test.go (3)
31-39: Remove stale error assertion after getDynamoDBClientYou’re checking the previous err variable; assert the client instead.
Apply:
cli := getDynamoDBClient(t, ctr) -require.NoError(t, err, "failed to get dynamodb client handle") +require.NotNil(t, cli, "dynamodb client should be created")
93-101: Fix two no-op assertions using unrelated errBoth checks reference a stale err; assert cli2 and drop the redundant check after query.
Apply:
cli2 := getDynamoDBClient(t, ctr2) -require.NoError(t, err, "failed to get dynamodb client handle") +require.NotNil(t, cli2, "dynamodb client should be created") @@ queryResult := queryItem(t, cli1, value) -require.NoError(t, err) require.Equal(t, value, queryResult)Also applies to: 109-113
116-141: Remove stale error assertion after second getDynamoDBClientSame pattern; assert the client rather than checking old err.
Apply:
cli = getDynamoDBClient(t, ctr2) -require.NoError(t, err, "failed to get dynamodb client handle") +require.NotNil(t, cli, "dynamodb client should be created")modules/gcloud/firestore/firestore_test.go (1)
30-36: Close the gRPC connection to avoid leaks.When using option.WithGRPCConn, the client won’t close the supplied conn. Defer a Close() after successful dial.
Apply this diff:
conn, err := grpc.NewClient(firestoreContainer.URI(), grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) +defer conn.Close()modules/gcloud/spanner/spanner_test.go (1)
48-53: Fail the test instead of logging and returning on client creation error.Current code can silently pass on failure.
Apply this diff:
- instanceAdmin, err := instance.NewInstanceAdminClient(ctx, options...) - if err != nil { - log.Printf("failed to create instance admin client: %v", err) - return - } + instanceAdmin, err := instance.NewInstanceAdminClient(ctx, options...) + require.NoError(t, err) defer instanceAdmin.Close()modules/compose/compose_api_test.go (1)
463-471: Double teardown: cleanup schedules Down, and you also call Down explicitlycleanup(t, compose) registers a Down in t.Cleanup; calling compose.Down again in the test can cause a second teardown and a failure during cleanup. Prefer one or the other.
Option A (remove the explicit cleanup call in this test):
- cleanup(t, compose)Option B (keep cleanup and drop the explicit Down):
- require.NoError(t, compose.Down(t.Context(), RemoveOrphans(true), RemoveVolumes(true), RemoveImagesLocal), "compose.Down()")If you need to assert file removal immediately, use Option A for this test only.
Also applies to: 477-478
modules/valkey/valkey_test.go (1)
155-158: Fix assertion argument orderSwap expected/actual in require.Equal for clearer failures.
- require.Equal(t, retVal, value) + require.Equal(t, value, retVal)docker_auth_test.go (1)
180-186: Avoid nil deref when client creation failsdefer on a nil client will panic if NewDockerClientWithOpts fails.
testcontainersClient, err := NewDockerClientWithOpts(ctx, client.WithVersion(daemonMaxVersion)) if err != nil { t.Log("could not create client to cleanup registry: ", err) + return } defer testcontainersClient.Close()modules/compose/compose_test.go (1)
371-384: Fix compile error: t.Context used in helpers that accept testing.TBBoth helper functions take testing.TB, which doesn’t have Context(). Replace the parameter with *testing.T (call sites already pass t) and update uses.
-func assertVolumeDoesNotExist(tb testing.TB, volumeName string) { - tb.Helper() - containerClient, err := testcontainers.NewDockerClientWithOpts(t.Context()) - require.NoErrorf(tb, err, "Failed to get provider") - volumeList, err := containerClient.VolumeList(t.Context(), volume.ListOptions{Filters: filters.NewArgs(filters.Arg("name", volumeName))}) - require.NoErrorf(tb, err, "Failed to list volumes") - if len(volumeList.Warnings) > 0 { - tb.Logf("Volume list warnings: %v", volumeList.Warnings) - } - require.Emptyf(tb, volumeList.Volumes, "Volume list is not empty") +func assertVolumeDoesNotExist(t *testing.T, volumeName string) { + t.Helper() + containerClient, err := testcontainers.NewDockerClientWithOpts(t.Context()) + require.NoErrorf(t, err, "Failed to get provider") + volumeList, err := containerClient.VolumeList(t.Context(), volume.ListOptions{Filters: filters.NewArgs(filters.Arg("name", volumeName))}) + require.NoErrorf(t, err, "Failed to list volumes") + if len(volumeList.Warnings) > 0 { + t.Logf("Volume list warnings: %v", volumeList.Warnings) + } + require.Emptyf(t, volumeList.Volumes, "Volume list is not empty") } -func assertContainerEnvironmentVariables( - tb testing.TB, +func assertContainerEnvironmentVariables( + t *testing.T, composeIdentifier, serviceName string, present map[string]string, absent map[string]string, ) { - tb.Helper() - containerClient, err := testcontainers.NewDockerClientWithOpts(t.Context()) - require.NoErrorf(tb, err, "Failed to get provider") - containers, err := containerClient.ContainerList(t.Context(), container.ListOptions{}) - require.NoErrorf(tb, err, "Failed to list containers") - require.NotEmptyf(tb, containers, "container list empty") + t.Helper() + containerClient, err := testcontainers.NewDockerClientWithOpts(t.Context()) + require.NoErrorf(t, err, "Failed to get provider") + containers, err := containerClient.ContainerList(t.Context(), container.ListOptions{}) + require.NoErrorf(t, err, "Failed to list containers") + require.NotEmptyf(t, containers, "container list empty") containerNameRegexp := regexp.MustCompile(fmt.Sprintf(`^\/?%s(_|-)%s(_|-)\d$`, composeIdentifier, serviceName)) var containerID string @@ - details, err := containerClient.ContainerInspect(t.Context(), containerID) - require.NoErrorf(tb, err, "Failed to inspect container") + details, err := containerClient.ContainerInspect(t.Context(), containerID) + require.NoErrorf(t, err, "Failed to inspect container") @@ - assert.Contains(tb, details.Config.Env, keyVal) + assert.Contains(t, details.Config.Env, keyVal) @@ - assert.NotContains(tb, details.Config.Env, keyVal) + assert.NotContains(t, details.Config.Env, keyVal)Also applies to: 386-398, 400-415
modules/chroma/chroma_test.go (1)
36-47: Use the established ctx (or tt.Context) and stop using TODO inside testsInside the subtest, you’re referencing the outer t.Context() and using context.TODO(). Prefer the established ctx or derive from the subtest’s tt.Context(). Also remove the now‑unused context import if you switch off TODO.
Apply:
- endpoint, err := ctr.RESTEndpoint(t.Context()) + endpoint, err := ctr.RESTEndpoint(ctx) require.NoErrorf(tt, err, "failed to get REST endpoint") chromaClient, err := chromago.NewClient(endpoint) // } require.NoErrorf(tt, err, "failed to create client") - hb, err := chromaClient.Heartbeat(context.TODO()) + hb, err := chromaClient.Heartbeat(ctx)If context is no longer used elsewhere in this file, drop the context import.
🧹 Nitpick comments (57)
modules/vearch/vearch_test.go (1)
20-23: Prefer the subtest’s own contextInside the subtest we still use the parent test’s context. If this subtest (or a future one) ever calls
t.Parallel()the parent can finish earlier, cancelingctxand breaking the nested operations. Grabbingt.Context()inside the subtest keeps deadlines/cancellation scoped correctly.t.Run("REST Endpoint", func(t *testing.T) { + subCtx := t.Context() // restEndpoint { - restEndpoint, err := ctr.RESTEndpoint(ctx) + restEndpoint, err := ctr.RESTEndpoint(subCtx) // }modules/mssql/mssql_test.go (2)
48-69: Prefer per-subtest contexts for cancellation
ctxis captured before thet.Runcalls and shared across the subtests. If one subtest times out or bails out early, the shared parent context stays alive until the outer test finishes, leaving the container operations in the timed-out subtest without a timely cancellation signal. Please derive the context from each subtest’st.Context()instead, e.g. obtainsubtestCtx := t.Context()inside eacht.Runclosure and pass that tomssql.Run.
194-218: Give each subtest its own contextHere the helper receives the parent test’s context, so every subtest shares one cancellation lifecycle. When a single subtest fails or times out, the container/database work in that subtest will continue until all sibling subtests complete, which is not ideal. Grab a fresh
ctx := t.Context()inside eacht.Runbody (or passt.Context()directly toassertContainer) to ensure per-subtest cancellation.modules/clickhouse/clickhouse_test.go (2)
160-201: Derive the context inside each subtestLine 171 currently reuses the parent test’s context, which means every subtest shares a single
ctx. Grabbing a fresht.Context()inside the subtest keeps the context lifecycle aligned with the subtest that owns it and avoids surprises if we ever parallelize or extend these cases.Apply this diff inside the
t.Runbody:- ctx := t.Context() - testCases := []struct { desc string configOption testcontainers.CustomizeRequestOption }{ {"XML_Config", clickhouse.WithConfigFile(filepath.Join("testdata", "config.xml"))}, // <allow_no_password>1</allow_no_password> {"YAML_Config", clickhouse.WithYamlConfigFile(filepath.Join("testdata", "config.yaml"))}, // allow_no_password: true } for _, tC := range testCases { t.Run(tC.desc, func(t *testing.T) { + ctx := t.Context() + ctr, err := clickhouse.Run(ctx, "clickhouse/clickhouse-server:23.3.8.21-alpine", clickhouse.WithUsername(user),
319-335: Close rows and surface iteration errorsLine 320 opens a result set but never closes it, and any iteration error remains silent. That can leak resources on some drivers and hide failures. Let’s close the rows and return
rows.Err()once the loop finishes.func getAllRows(ctx context.Context, conn driver.Conn) ([]Test, error) { rows, err := conn.Query(ctx, "SELECT * FROM test_table;") if err != nil { return nil, err } + defer rows.Close() var data []Test for rows.Next() { var r Test err := rows.Scan(&r.ID) if err != nil { return nil, err } data = append(data, r) } + + if err := rows.Err(); err != nil { + return nil, err + } return data, nil }modules/nebulagraph/nebulagraph_test.go (3)
26-27: Optional: rely on t.Context() directly instead of adding a 10m timeout.
t.Context()already carries the test deadline; nesting a 10m timeout is redundant. Prefer the bare test context and add shorter, step-scoped timeouts only where needed.Example:
-ctx, cancel := context.WithTimeout(t.Context(), 10*time.Minute) -defer cancel() +ctx := t.Context()Note: remove the now-unused time import if you apply this.
42-46: Nit: clarify variable name.Rename
portttoportStrfor readability.- host, portt, err := net.SplitHostPort(conn) + host, portStr, err := net.SplitHostPort(conn) require.NoError(t, err) - portInt, err := strconv.Atoi(portt) + portInt, err := strconv.Atoi(portStr)
97-97: Comment doesn’t match the assertion.The code validates the YIELD result equals 1, not that it contains a storage node. Update the comment to reflect the actual check.
modules/k6/k6_test.go (1)
52-54: Optional: assert provider.Close() errors in cleanupClose likely returns an error; asserting it makes failures visible instead of being silently dropped.
Apply this diff:
- defer provider.Close() + defer func() { + require.NoError(t, provider.Close()) + }()internal/core/docker_host_test.go (1)
185-230: Consistent use of t.Context() across helper invocations — looks goodAll updated call sites align with existing function signatures. No behavior change.
If you want to reduce repetition, consider a tiny helper like testCtxWithHost(t, host string) that wraps context.WithValue(t.Context(), DockerHostContextKey, host).
Also applies to: 256-278, 283-315, 344-383, 395-431, 438-448, 457-482
lifecycle_test.go (2)
704-724: Capture t.Context() once to reduce repetitionMinor nit: create ctx := t.Context() once and reuse to avoid repeated calls and keep a single context identity through the sequence.
-err := hooks.Creating(t.Context())(req) +ctx := t.Context() +err := hooks.Creating(ctx)(req) require.NoError(t, err) c := &DockerContainer{} -err = hooks.Created(t.Context())(c) +err = hooks.Created(ctx)(c) require.NoError(t, err) -err = hooks.Starting(t.Context())(c) +err = hooks.Starting(ctx)(c) require.NoError(t, err) -err = hooks.Started(t.Context())(c) +err = hooks.Started(ctx)(c) require.NoError(t, err) -err = hooks.Readied(t.Context())(c) +err = hooks.Readied(ctx)(c) require.NoError(t, err) -err = hooks.Stopping(t.Context())(c) +err = hooks.Stopping(ctx)(c) require.NoError(t, err) -err = hooks.Stopped(t.Context())(c) +err = hooks.Stopped(ctx)(c) require.NoError(t, err) -err = hooks.Terminating(t.Context())(c) +err = hooks.Terminating(ctx)(c) require.NoError(t, err) -err = hooks.Terminated(t.Context())(c) +err = hooks.Terminated(ctx)(c) require.NoError(t, err)
829-848: Add a nil-guard before calling ctr.Logs(ctx)Run returns an error by design here; ensure ctr is non-nil before invoking Logs to avoid a potential panic if implementation changes.
ctr, err := Run(ctx, "alpine", opts...) CleanupContainer(t, ctr) // it should fail because the waiting for condition is not met require.Error(t, err) +require.NotNil(t, ctr, "expected a non-nil container even on error to read logs") containerLogs, err := ctr.Logs(ctx) require.NoError(t, err) defer containerLogs.Close()If the API guarantees non-nil ctr on error, consider documenting that in the test with a brief comment.
modules/influxdb/influxdb_test.go (1)
490-505: Optional: avoid repeated t.Context() calls for consistency/readabilityOther tests bind ctx once; mirror that here.
Apply:
func TestWithConfigFile(t *testing.T) { influxVersion := "1.8.10" - influxDBContainer, err := influxdb.Run(t.Context(), + ctx := t.Context() + influxDBContainer, err := influxdb.Run(ctx, "influxdb:"+influxVersion, influxdb.WithConfigFile(filepath.Join("testdata", "influxdb.conf")), ) testcontainers.CleanupContainer(t, influxDBContainer) require.NoError(t, err) - if state, err := influxDBContainer.State(t.Context()); err != nil || !state.Running { + if state, err := influxDBContainer.State(ctx); err != nil || !state.Running { require.NoError(t, err) } /// influxConnectionUrl { cli, err := influxclient.NewHTTPClient(influxclient.HTTPConfig{ - Addr: influxDBContainer.MustConnectionUrl(t.Context()), + Addr: influxDBContainer.MustConnectionUrl(ctx), })modules/mongodb/atlaslocal/atlaslocal_test.go (3)
573-575: Fix misleading assertion message in requireEnvVar.The failure message is hardcoded to DO_NOT_TRACK but this helper checks arbitrary env vars.
Apply:
- require.Equal(t, expected, out, "DO_NOT_TRACK env var value mismatch") + require.Equalf(t, expected, out, "%s env var value mismatch", envVarName)
585-587: Assert non-empty logs explicitly.Currently you read one byte and ignore the result. Assert that at least one byte was read to make the test meaningful.
- buf := make([]byte, 1) - _, _ = reader.Read(buf) // read at least one byte to ensure non-empty + buf := make([]byte, 1) + n, _ := reader.Read(buf) + require.Greater(t, n, 0, "mongot logs should not be empty")- buf := make([]byte, 1) - _, _ = reader.Read(buf) // Read at least one byte to ensure non-empty + buf := make([]byte, 1) + n, _ := reader.Read(buf) + require.Greater(t, n, 0, "runner logs should not be empty")Also applies to: 610-612
28-28: Nit: keep context usage style consistent.Most call sites use t.Context() inline; here you assign ctx := t.Context(). Consider inlining for consistency, or adopt the local var everywhere.
modules/localstack/v2/s3_test.go (2)
60-63: Pass the test context to AWS config loader.Avoid context.TODO() in tests; use the per-test ctx to propagate deadlines/cancellation.
Apply:
- awsCfg, err := config.LoadDefaultConfig(context.TODO(), + awsCfg, err := config.LoadDefaultConfig(ctx, config.WithRegion(region), config.WithCredentialsProvider(credentials.NewStaticCredentialsProvider(accesskey, secretkey, token)), )
87-87: Avoid identifier shadowing: rename local s3Client variable.Shadowing the s3Client function hampers readability and may confuse tooling.
Apply:
- s3Client, err := s3Client(ctx, ctr) + client, err := s3Client(ctx, ctr) require.NoError(t, err) t.Run("S3 operations", func(t *testing.T) { bucketName := "localstack-bucket" // Create Bucket - outputBucket, err := s3Client.CreateBucket(ctx, &s3.CreateBucketInput{ + outputBucket, err := client.CreateBucket(ctx, &s3.CreateBucketInput{ Bucket: aws.String(bucketName), }) require.NoError(t, err) require.NotNil(t, outputBucket) // put object s3Key1 := "key1" body1 := []byte("Hello from localstack 1") - outputObject, err := s3Client.PutObject(ctx, &s3.PutObjectInput{ + outputObject, err := client.PutObject(ctx, &s3.PutObjectInput{ Bucket: aws.String(bucketName), Key: aws.String(s3Key1), Body: bytes.NewReader(body1), ContentLength: aws.Int64(int64(len(body1))), ContentType: aws.String("application/text"), ContentDisposition: aws.String("attachment"), }) require.NoError(t, err) require.NotNil(t, outputObject) t.Run("List Buckets", func(t *testing.T) { - output, err := s3Client.ListBuckets(ctx, &s3.ListBucketsInput{}) + output, err := client.ListBuckets(ctx, &s3.ListBucketsInput{}) require.NoError(t, err) require.NotNil(t, output) buckets := output.Buckets require.Len(t, buckets, 1) assert.Equal(t, bucketName, *buckets[0].Name) }) t.Run("List Objects in Bucket", func(t *testing.T) { - output, err := s3Client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{ + output, err := client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{ Bucket: aws.String(bucketName), }) require.NoError(t, err) require.NotNil(t, output)modules/localstack/v1/s3_test.go (2)
69-70: Avoid overshadowing the imported session package.Rename the variable to prevent confusion with github.com/aws/aws-sdk-go/aws/session.
Apply:
- session, err := awsSession(ctx, ctr) + sess, err := awsSession(ctx, ctr) require.NoError(t, err) - s3Uploader := s3manager.NewUploader(session) + s3Uploader := s3manager.NewUploader(sess)
81-98: Propagate ctx to AWS SDK v1 calls via WithContext.Use the WithContext variants to honor t.Context() deadlines/cancellation.
Apply:
- outputBucket, err := s3API.CreateBucket(&s3.CreateBucketInput{ + outputBucket, err := s3API.CreateBucketWithContext(ctx, &s3.CreateBucketInput{ Bucket: aws.String(bucketName), }) @@ - outputObject, err := s3API.PutObject(&s3.PutObjectInput{ + outputObject, err := s3API.PutObjectWithContext(ctx, &s3.PutObjectInput{ Bucket: aws.String(bucketName), Key: aws.String(s3Key1), Body: bytes.NewReader(body1), ContentLength: aws.Int64(int64(len(body1))), ContentType: aws.String("application/text"), ContentDisposition: aws.String("attachment"), })And:
- output, err := s3API.ListBuckets(nil) + output, err := s3API.ListBucketsWithContext(ctx, &s3.ListBucketsInput{})And:
- output, err := s3API.ListObjects(&s3.ListObjectsInput{ + output, err := s3API.ListObjectsWithContext(ctx, &s3.ListObjectsInput{ Bucket: aws.String(bucketName), })Also applies to: 102-103, 111-114
modules/redpanda/redpanda_test.go (2)
635-644: Use idiomatic lower-camel case for local var name.RPNetwork looks exported; prefer rpNetwork for a local variable.
Apply:
- RPNetwork, err := network.New(ctx) + rpNetwork, err := network.New(ctx) require.NoError(t, err) - testcontainers.CleanupNetwork(t, RPNetwork) + testcontainers.CleanupNetwork(t, rpNetwork) // 2. Attempt Start Redpanda ctr ctr, err := redpanda.Run(ctx, "redpandadata/redpanda:v23.2.18", redpanda.WithListener("redpanda:99092"), - network.WithNetwork([]string{"redpanda-host"}, RPNetwork), + network.WithNetwork([]string{"redpanda-host"}, rpNetwork), )
31-31: Scope subtest contexts int.Runto isolate each subtest’s cancellation and error handling from the parentt.Context().modules/dynamodb/dynamodb_test.go (3)
233-241: Set an explicit Region for local DynamoDB to avoid env couplingPrevents LoadDefaultConfig from relying on external AWS_REGION config.
Apply:
-cfg, err := config.LoadDefaultConfig(t.Context(), config.WithCredentialsProvider(credentials.StaticCredentialsProvider{ - Value: aws.Credentials{ - AccessKeyID: "DUMMYIDEXAMPLE", - SecretAccessKey: "DUMMYEXAMPLEKEY", - }, -})) +cfg, err := config.LoadDefaultConfig( + t.Context(), + config.WithRegion("us-east-1"), // explicit region for local DynamoDB + config.WithCredentialsProvider(credentials.StaticCredentialsProvider{ + Value: aws.Credentials{ + AccessKeyID: "DUMMYIDEXAMPLE", + SecretAccessKey: "DUMMYEXAMPLEKEY", + }, + }), +)
102-104: Guard against empty ListTables before indexingImproves failure output and avoids panics if no tables are returned.
Apply:
+require.NotEmpty(t, result.TableNames, "no tables found") actualTableName := result.TableNames[0] require.Equal(t, tableName, actualTableName)
257-262: Guard against empty ListTables before indexingSame suggestion for the helper.
Apply:
+require.NotEmpty(t, result.TableNames, "no tables found") actualTableName := result.TableNames[0] require.Equal(t, tableName, actualTableName)modules/neo4j/neo4j_test.go (3)
21-50: Prefer subtest-local contexts; keep outer for suite setup onlyUse outer.Context() only for suite-scoped setup (container). Inside each outer.Run, derive tCtx := t.Context() and use it for driver and queries to tie cancellation to the subtest.
Apply this minimal change for the suite ctx:
- ctx := outer.Context() + suiteCtx := outer.Context()Then inside the subtests (illustrative snippet):
outer.Run("connects via Bolt", func(t *testing.T) { tCtx := t.Context() driver := createDriver(t, tCtx, ctr) err := driver.VerifyConnectivity(tCtx) require.NoErrorf(t, err, "should have successfully connected to server but did not") }) outer.Run("exercises APOC plugin", func(t *testing.T) { tCtx := t.Context() driver := createDriver(t, tCtx, ctr) result, err := neo.ExecuteQuery(tCtx, driver, "RETURN apoc.number.arabicToRoman(1986) AS output", nil, neo.EagerResultTransformer) require.NoErrorf(t, err, "expected APOC query to successfully run but did not") // ... }) outer.Run("is configured with custom Neo4j settings", func(t *testing.T) { tCtx := t.Context() env := getContainerEnv(t, tCtx, ctr) require.Containsf(t, env, "NEO4J_dbms_tx__log_rotation_size=42M", "expected to custom setting to be exported but was not") })
56-79: Derive context inside each subtest; avoid sharing parent t.Context() across parallel subtestsSince subtests call t.Parallel(), bind their operations to t.Context() inside each t.Run and drop the outer ctx var here.
Apply this small change:
- ctx := t.Context() + // derive per-subtest ctx via t.Context() inside each t.RunThen inside the closure:
t.Run(edition, func(t *testing.T) { t.Parallel() tCtx := t.Context() ctr, err := neo4j.Run(tCtx, img, neo4j.WithAdminPassword(testPassword), neo4j.WithAcceptCommercialLicenseAgreement()) testcontainers.CleanupContainer(t, ctr) require.NoError(t, err) env := getContainerEnv(t, tCtx, ctr) require.Containsf(t, env, "NEO4J_ACCEPT_LICENSE_AGREEMENT=yes", "expected to accept license agreement but did not") })
85-124: Use t.Context() per subtest here as wellAvoid using outer.Context() for subtest-scoped container runs; derive ctx within each t.Run to ensure correct lifetime.
Apply this small change:
- ctx := outer.Context() + // derive per-subtest ctx via t.Context() inside each t.RunThen inside each subtest:
outer.Run("without authentication", func(t *testing.T) { tCtx := t.Context() ctr, err := neo4j.Run(tCtx, "neo4j:4.4") testcontainers.CleanupContainer(t, ctr) require.NoError(t, err) }) outer.Run("auth setting outside WithAdminPassword raises error", func(t *testing.T) { tCtx := t.Context() ctr, err := neo4j.Run(tCtx, "neo4j:4.4", neo4j.WithAdminPassword(testPassword), neo4j.WithNeo4jSetting("AUTH", "neo4j/thisisgonnafail")) testcontainers.CleanupContainer(t, ctr) 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) { tCtx := t.Context() logger := &inMemoryLogger{} ctr, err := neo4j.Run(tCtx, "neo4j:4.4", testcontainers.WithLogger(logger), neo4j.WithAdminPassword(testPassword), neo4j.WithNeo4jSetting("some.key", "value1"), neo4j.WithNeo4jSettings(map[string]string{"some.key": "value2"}), neo4j.WithNeo4jSetting("some.key", "value3")) testcontainers.CleanupContainer(t, ctr) require.NoError(t, err) errorLogs := logger.Logs() require.Containsf(t, errorLogs, `setting "some.key" with value "value1" is now overwritten with value "value2"`+"\n", "expected setting overwrites to be logged") // ... require.Containsf(t, getContainerEnv(t, tCtx, ctr), "NEO4J_some_key=value3", "expected custom setting to be set with last value") })modulegen/main_test.go (3)
547-557: Close file handles per-iteration to avoid FD buildupDeferring Close() inside a loop accumulates open files and can hit ulimit on larger doc sets. Wrap per-copy in a small closure (or close explicitly).
Apply this diff:
- srcReader, err := os.Open(filepath.Join(ctx.DocsDir(), moduleType, mkFile.Name())) - require.NoError(t, err) - defer srcReader.Close() - - dstWriter, err := os.Create(filepath.Join(tmpCtx.DocsDir(), moduleType, mkFile.Name())) - require.NoError(t, err) - defer dstWriter.Close() - - _, err = io.Copy(dstWriter, srcReader) - require.NoError(t, err) + func() { + src, err := os.Open(filepath.Join(ctx.DocsDir(), moduleType, mkFile.Name())) + require.NoError(t, err) + defer src.Close() + + dst, err := os.Create(filepath.Join(tmpCtx.DocsDir(), moduleType, mkFile.Name())) + require.NoError(t, err) + defer dst.Close() + + _, err = io.Copy(dst, src) + require.NoError(t, err) + }()
148-154: Run subtests in parallel (safe here)These subtests have no shared I/O; parallelizing speeds up the suite without side effects.
Apply this diff:
- outer.Run(test.name, func(t *testing.T) { + outer.Run(test.name, func(t *testing.T) { + t.Parallel() if test.expectedErr != nil { require.EqualError(t, test.module.Validate(), test.expectedErr.Error()) } else { require.NoError(t, test.module.Validate()) } })
377-378: Change “In example” → “In tests” in modulegen/main_test.go
Minimum Go version is 1.24+ (testing.T.Context() introduced in Go 1.24), so t.Context() is supported.- require.Equal(t, "In example: `Run(t.Context(), \""+module.Image+"\")`.", data[41]) + require.Equal(t, "In tests: `Run(t.Context(), \""+module.Image+"\")`.", data[41])modules/meilisearch/meilisearch_test.go (1)
34-34: Consider binding HTTP requests to the test context.Use
req = req.WithContext(ctx)and optionally a client with a Timeout to ensure requests are canceled when the test ends.modules/pulsar/pulsar_test.go (3)
154-158: Close the producer to avoid leaking resources.Add a cleanup for the producer similar to the client and consumer.
Apply:
producer, err := pc.CreateProducer(pulsar.ProducerOptions{ Topic: "test-topic", }) require.NoError(t, err) +t.Cleanup(func() { producer.Close() })
164-171: Stop the time source to prevent leaks.Prefer a timer or stop the ticker after use.
Apply:
- ticker := time.NewTicker(1 * time.Minute) + timer := time.NewTimer(1 * time.Minute) select { - case <-ticker.C: + case <-timer.C: t.Fatal("did not receive message in time") case msg := <-msgChan: require.Equalf(t, "hello world", string(msg), "received unexpected message bytes") + timer.Stop() }
188-196: Guard the type assertion for subscriptions.Avoid panics if the response changes shape.
Apply:
- subscriptions := stats["subscriptions"] - require.NotNil(t, subscriptions) - - subscriptionsMap := subscriptions.(map[string]any) + subscriptions := stats["subscriptions"] + require.NotNil(t, subscriptions) + subscriptionsMap, ok := subscriptions.(map[string]any) + require.True(t, ok, "subscriptions must be an object") // check that the subscription exists _, ok := subscriptionsMap[subscriptionName] require.True(t, ok)modules/dind/dind_test.go (1)
18-19: Prefer WithTimeout for clarity.Using WithTimeout conveys the intention directly.
- ctx, cancel := context.WithDeadline(t.Context(), time.Now().Add(3*time.Minute)) + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Minute)modules/dockermodelrunner/dockermodelrunner_test.go (1)
68-72: Avoid variable shadowing for readability.Rename the timeout context to avoid shadowing outer ctx.
- ctx, cancel := context.WithTimeout(ctx, 1*time.Millisecond) + timeoutCtx, cancel := context.WithTimeout(ctx, 1*time.Millisecond) defer cancel() - err := ctr.PullModel(ctx, testModelFQMN) + err := ctr.PullModel(timeoutCtx, testModelFQMN)modules/vault/vault_test.go (2)
61-67: Bind the HTTP request to the test context and handle its error.Prevents hangs and improves cancellation on test timeout.
Apply this diff:
- request, _ := http.NewRequest(http.MethodGet, hostAddress+"/v1/secret/data/test1", nil) + request, err := http.NewRequestWithContext(ctx, http.MethodGet, hostAddress+"/v1/secret/data/test1", nil) + require.NoError(t, err) request.Header.Add("X-Vault-Token", token) response, err := http.DefaultClient.Do(request)
96-104: Use a per-subtest context in “Write secret” for consistency.Keeps deadlines scoped to the current subtest.
Apply this diff:
t.Run("From vault client library", func(t *testing.T) { + ctx := t.Context() client, err := vaultClient.New( vaultClient.WithAddress(hostAddress), vaultClient.WithRequestTimeout(30*time.Second), )modules/registry/registry_test.go (2)
95-116: Use the subtest’s context (tt.Context()) inside tt subtestsInside subtests declared as func(tt *testing.T), you’re still passing t.Context() from the parent. Use tt.Context() so cancellation/deadlines are scoped correctly to each subtest.
Apply:
- redisC, err := testcontainers.GenericContainer(t.Context(), testcontainers.GenericContainerRequest{ + redisC, err := testcontainers.GenericContainer(tt.Context(), testcontainers.GenericContainerRequest{- redisC, err := testcontainers.GenericContainer(t.Context(), testcontainers.GenericContainerRequest{ + redisC, err := testcontainers.GenericContainer(tt.Context(), testcontainers.GenericContainerRequest{- state, err := redisC.State(t.Context()) + state, err := redisC.State(tt.Context())Also applies to: 117-139, 140-145
21-34: Propagate ctx to HTTP request
Replace thehttp.Get(httpAddress+"/v2/_catalog")call with an explicit request usinghttp.NewRequestWithContext(ctx, http.MethodGet, httpAddress+"/v2/_catalog", nil)andhttp.DefaultClient.Do(req)to ensure the call is canceled when the test context ends.modules/opensearch/opensearch_test.go (1)
14-33: LGTM; consider binding HTTP requests to ctxNice use of t.Context(). As a minor improvement, create the request with http.NewRequestWithContext(ctx, ...) and/or set a small http.Client timeout to avoid hangs if the test context is canceled.
modules/consul/consul_test.go (1)
16-41: Prefer subtest-scoped contexts in t.Run closuresYou capture ctx from the parent test and reuse it inside subtests. For better scoping and cancellation, derive ctx inside each t.Run closure.
Example:
t.Run(test.name, func(t *testing.T) { ctx := t.Context() ctr, err := consul.Run(ctx, "hashicorp/consul:1.15", test.opts...) // ... })Also consider using http.NewRequestWithContext(ctx, ...) or an http.Client with Timeout for the health check to honor test cancellation.
modules/kafka/kafka_test.go (2)
39-41: Use the already-derived ctx inside the goroutineAvoid re-calling t.Context() inside the goroutine; use the ctx you already derived.
Apply this diff:
- if err := client.Consume(t.Context(), []string{topic}, consumer); err != nil { + if err := client.Consume(ctx, []string{topic}, consumer); err != nil {
33-35: Close Sarama client and producer to avoid leaksDefer closing the consumer group and producer.
Apply this diff:
client, err := sarama.NewConsumerGroup(brokers, "groupName", config) require.NoError(t, err) +defer func() { _ = client.Close() }() @@ producer, err := sarama.NewSyncProducer(brokers, config) require.NoError(t, err) +defer func() { _ = producer.Close() }()Also applies to: 52-54
modules/compose/compose_api_test.go (1)
40-41: Use ctx instead of t.Context() for intra-test callsFor consistency, prefer the ctx you derived at the start of the test.
Apply these diffs:
- container, err := compose.ServiceContainer(t.Context(), service) + container, err := compose.ServiceContainer(ctx, service)- _, err = compose.ServiceContainer(t.Context(), "api-mysql") + _, err = compose.ServiceContainer(ctx, "api-mysql")- c, err := compose.ServiceContainer(t.Context(), serviceName) + c, err := compose.ServiceContainer(ctx, serviceName)Also applies to: 103-104, 186-189
modules/rabbitmq/examples_test.go (1)
235-253: Validate exec exit code in assertPluginsCheck the returned exit code to catch failures where output parsing might still pass.
Apply this diff:
-func assertPlugins(ctx context.Context, container testcontainers.Container, plugins ...string) error { +func assertPlugins(ctx context.Context, container testcontainers.Container, plugins ...string) error { for _, plugin := range plugins { - _, out, err := container.Exec(ctx, []string{"rabbitmq-plugins", "is_enabled", plugin}) + code, out, err := container.Exec(ctx, []string{"rabbitmq-plugins", "is_enabled", plugin}) if err != nil { return fmt.Errorf("failed to execute command: %w", err) } + if code != 0 { + return fmt.Errorf("rabbitmq-plugins exited with non-zero code: %d", code) + } check, err := io.ReadAll(out) if err != nil { return fmt.Errorf("failed to read output: %w", err) } if !strings.Contains(string(check), plugin+" is enabled") { return fmt.Errorf("plugin %q is not enabled", plugin) } } return nil }modules/valkey/valkey_test.go (1)
52-60: Use subtest-scoped t.Context() inside t.Run closuresDerive a fresh context per subtest to align with ustesting best practices and tighter cancellation boundaries.
Apply:
for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - valkeyContainer, err := tcvalkey.Run(ctx, tt.image, tcvalkey.WithConfigFile(filepath.Join("testdata", "valkey7.conf"))) + ctx := t.Context() + valkeyContainer, err := tcvalkey.Run(ctx, tt.image, tcvalkey.WithConfigFile(filepath.Join("testdata", "valkey7.conf"))) testcontainers.CleanupContainer(t, valkeyContainer) require.NoError(t, err) assertSetsGets(t, ctx, valkeyContainer, 1) }) } t.Run("mtls-disabled", func(t *testing.T) { - valkeyContainer, err := tcvalkey.Run(ctx, "valkey/valkey:7.2.5", tcvalkey.WithTLS()) + ctx := t.Context() + valkeyContainer, err := tcvalkey.Run(ctx, "valkey/valkey:7.2.5", tcvalkey.WithTLS()) testcontainers.CleanupContainer(t, valkeyContainer) require.NoError(t, err) assertSetsGets(t, ctx, valkeyContainer, 1) }) t.Run("mtls-enabled", func(t *testing.T) { - valkeyContainer, err := tcvalkey.Run(ctx, "valkey/valkey:7.2.5", tcvalkey.WithTLS(), testcontainers.WithCmdArgs("--tls-auth-clients", "no")) + ctx := t.Context() + valkeyContainer, err := tcvalkey.Run(ctx, "valkey/valkey:7.2.5", tcvalkey.WithTLS(), testcontainers.WithCmdArgs("--tls-auth-clients", "no")) testcontainers.CleanupContainer(t, valkeyContainer) require.NoError(t, err) assertSetsGets(t, ctx, valkeyContainer, 1) })Also applies to: 86-101
modules/dockermodelrunner/docker_desktop_test.go (1)
27-35: Close Docker client to avoid leaksEnsure the client is closed after Info to free resources.
func isDockerDesktopRunning(ctx context.Context, l log.Logger) (bool, error) { - cli, err := testcontainers.NewDockerClientWithOpts(ctx) + cli, err := testcontainers.NewDockerClientWithOpts(ctx) if err != nil { return false, fmt.Errorf("failed to create docker client: %w", err) } + defer cli.Close() - info, err := cli.Info(ctx) + info, err := cli.Info(ctx)modules/postgres/postgres_test.go (1)
396-417: Prefer subtest-scoped contexts within nested t.Run blocksAvoid mixing parent ctx with subtest operations; use t.Context() for Exec/Restore to respect subtest deadlines.
t.Run("Test inserting a user", func(t *testing.T) { t.Cleanup(func() { // 3. In each test, reset the DB to its snapshot state. - err = ctr.Restore(ctx) + err = ctr.Restore(t.Context()) require.NoError(t, err) }) conn, err := pgx.Connect(t.Context(), dbURL) require.NoError(t, err) defer conn.Close(t.Context()) - _, err = conn.Exec(ctx, "INSERT INTO users(name, age) VALUES ($1, $2)", "test", 42) + _, err = conn.Exec(t.Context(), "INSERT INTO users(name, age) VALUES ($1, $2)", "test", 42) require.NoError(t, err) }) t.Run("Test querying empty DB", func(t *testing.T) { t.Cleanup(func() { - err = ctr.Restore(ctx) + err = ctr.Restore(t.Context()) require.NoError(t, err) }) conn, err := pgx.Connect(t.Context(), dbURL) require.NoError(t, err) defer conn.Close(t.Context()) }) t.Run("Test that the restore works when not using defaults", func(t *testing.T) { - _, _, err = ctr.Exec(ctx, []string{"psql", "-U", user, "-d", dbname, "-c", "INSERT INTO users(name, age) VALUES ('test', 42)"}) + _, _, err = ctr.Exec(t.Context(), []string{"psql", "-U", user, "-d", dbname, "-c", "INSERT INTO users(name, age) VALUES ('test', 42)"}) require.NoError(t, err) // Doing the restore before we connect since this resets the pgx connection - err = ctr.Restore(ctx) + err = ctr.Restore(t.Context()) require.NoError(t, err) conn, err := pgx.Connect(t.Context(), dbURL) require.NoError(t, err) defer conn.Close(t.Context()) }) t.Run("Test inserting a user", func(t *testing.T) { t.Cleanup(func() { // 3. In each test, reset the DB to its snapshot state. - err := ctr.Restore(ctx) + err := ctr.Restore(t.Context()) require.NoError(t, err) }) conn, err2 := pgx.Connect(t.Context(), dbURL) require.NoError(t, err2) defer conn.Close(t.Context()) - _, err2 = conn.Exec(ctx, "INSERT INTO users(name, age) VALUES ($1, $2)", "test", 42) + _, err2 = conn.Exec(t.Context(), "INSERT INTO users(name, age) VALUES ($1, $2)", "test", 42) require.NoError(t, err2) }) t.Run("Test querying empty DB", func(t *testing.T) { // 4. Run as many tests as you need, they will each get a clean database t.Cleanup(func() { - err := ctr.Restore(ctx) + err := ctr.Restore(t.Context()) require.NoError(t, err) }) conn, err2 := pgx.Connect(t.Context(), dbURL) require.NoError(t, err2) defer conn.Close(t.Context()) })Also applies to: 421-434, 466-483, 545-567, 568-585
docker_auth_test.go (1)
105-111: Avoid shadowing imported package name “registry”Using a local variable named registry shadows the imported docker/types/registry package and can confuse readers.
- registry, cfg, err := DockerImageAuth(t.Context(), exampleAuth+"/my/image:latest") + reg, cfg, err := DockerImageAuth(t.Context(), exampleAuth+"/my/image:latest") - require.Equal(t, exampleAuth, registry) + require.Equal(t, exampleAuth, reg) - registry, cfg, err := DockerImageAuth(t.Context(), imageReg+imagePath) + reg, cfg, err := DockerImageAuth(t.Context(), imageReg+imagePath) - require.Equal(t, imageReg, registry) + require.Equal(t, imageReg, reg) - registry, cfg, err := DockerImageAuth(t.Context(), imageReg+imagePath) + reg, cfg, err := DockerImageAuth(t.Context(), imageReg+imagePath) require.ErrorIs(t, err, dockercfg.ErrCredentialsNotFound) require.Empty(t, cfg) - require.Equal(t, imageReg, registry) + require.Equal(t, imageReg, reg) - registry, cfg, err := DockerImageAuth(t.Context(), imageReg+imagePath) + reg, cfg, err := DockerImageAuth(t.Context(), imageReg+imagePath) require.ErrorIs(t, err, dockercfg.ErrCredentialsNotFound) require.Empty(t, cfg) - require.Equal(t, imageReg, registry) + require.Equal(t, imageReg, reg)Also applies to: 118-124, 133-137, 153-157
modules/registry/examples_test.go (1)
76-76: Use the same ctx for consistency in ExampleRun_withAuthenticationMinor consistency nit: prefer ctx over context.Background() here.
- redisC, err := testcontainers.GenericContainer(context.Background(), testcontainers.GenericContainerRequest{ + redisC, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{- state, err := redisC.State(context.Background()) + state, err := redisC.State(ctx)Also applies to: 100-100
modules/compose/compose_test.go (1)
400-411: Guard against missing container match to fail fastIf no container matches, containerID is empty and inspect fails later with a less clear error. Add an explicit check.
for i := range containers { c := containers[i] for j := range c.Names { if containerNameRegexp.MatchString(c.Names[j]) { containerID = c.ID break containerLoop } } } + if containerID == "" { + t.Fatalf("container %q for compose %q not found", serviceName, composeIdentifier) + }docker_test.go (1)
1091-1107: Optionally derive ctx per subtestInside t.Run closures, consider using the subtest’s t.Context() instead of reusing a parent-scoped ctx. This tightens cancellation to the subtest’s lifecycle.
Example:
- t.Run(tc.name, func(t *testing.T) { - nginxC, err := Run(ctx, nginxImage, + t.Run(tc.name, func(t *testing.T) { + subCtx := t.Context() + nginxC, err := Run(subCtx, nginxImage, WithExposedPorts(nginxDefaultPort), WithWaitStrategy(wait.ForListeningPort(nginxDefaultPort)), ) CleanupContainer(t, nginxC) require.NoError(t, err) - _ = nginxC.CopyFileToContainer(ctx, filepath.Join(".", "testdata", "hello.sh"), tc.copiedFileName, 700) - c, _, err := nginxC.Exec(ctx, []string{"bash", tc.copiedFileName}) + _ = nginxC.CopyFileToContainer(subCtx, filepath.Join(".", "testdata", "hello.sh"), tc.copiedFileName, 700) + c, _, err := nginxC.Exec(subCtx, []string{"bash", tc.copiedFileName})modules/ollama/examples_test.go (1)
249-257: Close the Docker client to avoid leaking resourcesAdd a defer to close the client after use. Also consider using context-aware HTTP requests later in this example.
Apply this diff:
- cli, err := testcontainers.NewDockerClientWithOpts(ctx) + cli, err := testcontainers.NewDockerClientWithOpts(ctx) + if err == nil { + defer cli.Close() + }modules/ollama/ollama_test.go (2)
25-33: Prefer subtest context for ConnectionStringInside the subtest, use t.Context() instead of the parent ctx for consistency and tighter scope.
Apply this diff:
- connectionStr, err := ctr.ConnectionString(ctx) + connectionStr, err := ctr.ConnectionString(t.Context())
90-96: Use t.Context() when starting the reused container inside the subtestTies container startup to the subtest’s cancellation.
Apply this diff:
- ctr2, err := ollama.Run(ctx, "ollama/ollama:0.5.7", testcontainers.WithReuseByName("ollama-container")) + ctr2, err := ollama.Run(t.Context(), "ollama/ollama:0.5.7", testcontainers.WithReuseByName("ollama-container"))
modules/k3s/k3s_example_test.go
Outdated
| func ExampleRun() { | ||
| // runK3sContainer { | ||
| ctx := context.Background() | ||
| ctx := t.Context() |
There was a problem hiding this comment.
Fix the undefined t in the example.
ExampleRun does not receive a *testing.T, so t is undefined here and the file won’t compile. Keep using a background context (and reintroduce the import) for this example.
Apply this diff to restore a valid context setup:
@@
-import (
- "fmt"
- "log"
+import (
+ "context"
+ "fmt"
+ "log"
@@
- ctx := t.Context()
+ ctx := context.Background()🤖 Prompt for AI Agents
In modules/k3s/k3s_example_test.go around line 17, the example uses an undefined
variable `t` (ctx := t.Context()) which prevents compilation; replace that with
a background context (e.g., ctx := context.Background()) and re-add the
necessary "context" import at top of the file so the example compiles and runs
as a standalone ExampleRun.
| if err := nwr.Remove(ctx); err != nil { | ||
| log.Printf("failed to remove network: %s", err) | ||
| } | ||
| }() |
There was a problem hiding this comment.
Fix missing context import (compile error).
ctx := context.Background() is used, but the context package is not imported.
Apply this diff to add the import:
import (
+ "context"
"fmt"
"log"
"time"🤖 Prompt for AI Agents
In modules/nats/examples_test.go around lines 88 to 91, the test uses ctx :=
context.Background() but the context package is not imported causing a compile
error; fix by adding the standard "context" import to the file's import block
(or include it in the existing grouped imports), then run go fmt/goimports to
ensure imports are ordered and the file compiles.
246e83d to
4773e6b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
modules/milvus/milvus_test.go (1)
20-32: Use the subtest’s context for subtest operationsInside the subtest we keep calling back to the parent’s
t.Context(). That bypasses the cancellation semantics the subtest provides (e.g., a failing subtest won’t cancel outstanding RPCs tied to the parent context), which can prolong work or leak resources. Grabtt.Context()once and thread it through the subtest calls.t.Run("Connect to Milvus with gRPC", func(tt *testing.T) { + ttCtx := tt.Context() // connectionString { - connectionStr, err := ctr.ConnectionString(ctx) + connectionStr, err := ctr.ConnectionString(ttCtx) // } require.NoError(t, err) - milvusClient, err := client.NewGrpcClient(t.Context(), connectionStr) + milvusClient, err := client.NewGrpcClient(ttCtx, connectionStr) require.NoError(t, err) defer milvusClient.Close() - v, err := milvusClient.GetVersion(ctx) + v, err := milvusClient.GetVersion(ttCtx) require.NoError(t, err)modules/pulsar/pulsar_test.go (1)
188-196: Guard the type assertion to avoid panics on schema changesUnprotected cast may panic if the API changes. Use a checked assertion.
- subscriptions := stats["subscriptions"] - require.NotNil(t, subscriptions) - subscriptionsMap := subscriptions.(map[string]any) - - // check that the subscription exists - _, ok := subscriptionsMap[subscriptionName] - require.True(t, ok) + subs, ok := stats["subscriptions"].(map[string]any) + require.True(t, ok, "subscriptions has unexpected structure") + // check that the subscription exists + _, exists := subs[subscriptionName] + require.True(t, exists)examples/nginx/nginx_test.go (1)
23-25: Close response body and bind HTTP request to test context.Avoids resource leak and ensures cancellation on test timeout.
- resp, err := http.Get(nginxC.URI) - require.NoError(t, err) - require.Equal(t, http.StatusOK, resp.StatusCode) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, nginxC.URI, nil) + require.NoError(t, err) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode)modules/redpanda/redpanda_test.go (1)
503-505: Fix assertion to check the correct URL variableYou’re asserting SchemaRegistryAddress uses https but checking adminAPIURL instead. Use schemaRegistryURL.
Apply:
- require.True(t, strings.HasPrefix(adminAPIURL, "https://"), "SchemaRegistryAddress should return https url") + require.True(t, strings.HasPrefix(schemaRegistryURL, "https://"), "SchemaRegistryAddress should return https url")
🧹 Nitpick comments (20)
modules/vearch/vearch_test.go (1)
20-22: Use the subtest’s own context.Inside the subtest we still rely on the parent-level
ctx. If this subtest ever switches tot.Parallel()(or spawns work that should be bound to the subtest lifecycle), the parent context would be cancelled as soon as the outer test returns, potentially cutting the subtest’s work short. Deriving a fresh context from the subtest’stesting.Tkeeps cancellation semantics local to the subtest.- restEndpoint, err := ctr.RESTEndpoint(ctx) + subCtx := t.Context() + restEndpoint, err := ctr.RESTEndpoint(subCtx)modules/consul/consul_test.go (1)
16-48: Use subtest-scoped context insidet.Run.Deriving the context from the parent
tmeans it will be canceled as soon as the parent frame returns. If any subtest later callst.Parallel(), that shared context will be canceled while the parallel subtests are still running, causing container setup/teardown to fail spuriously. Please grab the context from the subtest’s*testing.Tinstead and thread it through the calls.- ctx := t.Context() tests := []struct { name string opts []testcontainers.ContainerCustomizer }{ @@ - for _, test := range tests { + for _, test := range tests { t.Run(test.name, func(t *testing.T) { - ctr, err := consul.Run(ctx, "hashicorp/consul:1.15", test.opts...) + ctx := t.Context() + ctr, err := consul.Run(ctx, "hashicorp/consul:1.15", test.opts...) testcontainers.CleanupContainer(t, ctr) require.NoError(t, err) @@ - host, err := ctr.ApiEndpoint(ctx) + host, err := ctr.ApiEndpoint(ctx)(Propagate the new
ctxvariable to the remaining calls.)modules/clickhouse/clickhouse_test.go (1)
160-200: Use each subtest’s own context.Inside the subtests you keep using the parent
ctxcaptured beforet.Run. That means the ClickHouse container operations continue to run under the outer test’s lifetime instead of respecting the subtest’s cancellation/deadline, which defeats the per-test context propagation we’re enabling withusetesting. Please derive a fresh context from the subtest’st *testing.Tand pass it through.for _, tC := range testCases { t.Run(tC.desc, func(t *testing.T) { - ctr, err := clickhouse.Run(ctx, + subCtx := t.Context() + + ctr, err := clickhouse.Run(subCtx, "clickhouse/clickhouse-server:23.3.8.21-alpine", clickhouse.WithUsername(user), clickhouse.WithPassword(""), clickhouse.WithDatabase(dbname), tC.configOption, ) testcontainers.CleanupContainer(t, ctr) require.NoError(t, err) - connectionHost, err := ctr.ConnectionHost(ctx) + connectionHost, err := ctr.ConnectionHost(subCtx) require.NoError(t, err)modules/artemis/artemis_test.go (1)
18-120: Use the subtest’s own contextCapturing
ctx := t.Context()before thet.Runloop makes every subtest reuse the parent test’s context, so a subtest callingFailNow, hitting its own deadline, or being cancelled early won’t signal the context-dependent calls inside that subtest. Grab the context inside eacht.Runinstead so the lifecycle matches the concrete subtest.- ctx := t.Context() - for _, test := range tests { t.Run(test.name, func(t *testing.T) { + ctx := t.Context() + ctr, err := artemis.Run(ctx, "apache/activemq-artemis:2.30.0-alpine", test.opts...) testcontainers.CleanupContainer(t, ctr) require.NoError(t, err) @@ // consoleURL { u, err := ctr.ConsoleURL(ctx) // } @@ // brokerEndpoint { host, err := ctr.BrokerEndpoint(ctx) // }modules/pulsar/pulsar_test.go (8)
32-33: Good switch to t.Context()This ties cancellation to the test lifecycle. Consider also deriving a per-subtest context inside t.Run (see below) to avoid coupling subtests to the parent context.
98-116: Use per-subtest context inside t.Run to avoid leaks and align with usetestingDerive stx := t.Context() and use it for all calls inside the subtest (Run, BrokerURL, HTTPServiceURL, etc.) so early subtest termination cancels operations promptly.
Apply this diff pattern (repeat stx for subsequent ctx usages in the subtest):
t.Run(tt.name, func(t *testing.T) { + stx := t.Context() - c, err := testcontainerspulsar.Run( - ctx, + c, err := testcontainerspulsar.Run( + stx, "apachepulsar/pulsar:2.10.2", tt.opts..., ) testcontainers.CleanupContainer(t, c) require.NoError(t, err) // getBrokerURL { - brokerURL, err := c.BrokerURL(ctx) + brokerURL, err := c.BrokerURL(stx) // } // getAdminURL { - serviceURL, err := c.HTTPServiceURL(ctx) + serviceURL, err := c.HTTPServiceURL(stx) // }
141-152: Receive using the subtest contextUse stx to ensure the receive unblocks when the subtest ends.
- msg, err := consumer.Receive(ctx) + msg, err := consumer.Receive(stx)
159-163: Send using the subtest contextPropagate stx to the producer send.
- _, err = producer.Send(ctx, &pulsar.ProducerMessage{ + _, err = producer.Send(stx, &pulsar.ProducerMessage{ Payload: []byte("hello world"), })
164-170: Avoid ticker leak; use context timeout from stxNewTicker isn’t stopped, which can leak resources. Prefer a context deadline tied to the subtest context.
- ticker := time.NewTicker(1 * time.Minute) - select { - case <-ticker.C: - t.Fatal("did not receive message in time") - case msg := <-msgChan: - require.Equalf(t, "hello world", string(msg), "received unexpected message bytes") - } + dctx, cancel := context.WithTimeout(stx, 1*time.Minute) + defer cancel() + select { + case <-dctx.Done(): + t.Fatal("did not receive message in time") + case msg := <-msgChan: + require.Equalf(t, "hello world", string(msg), "received unexpected message bytes") + }
154-158: Close the producerAdd cleanup to close the producer explicitly.
producer, err := pc.CreateProducer(pulsar.ProducerOptions{ Topic: "test-topic", }) require.NoError(t, err) + t.Cleanup(func() { producer.Close() })
177-183: Assert HTTP status code before reading bodyEnsure the Admin endpoint responded OK to avoid parsing unexpected error payloads.
resp, err := httpClient.Get(serviceURL + "/admin/v2/persistent/public/default/test-topic/stats") require.NoError(t, err) defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode)
56-56: Remove leading space in PULSAR_MEM valueMinor nit: avoid leading space after '=' to prevent issues with downstream parsing.
- config.Env = append(config.Env, "PULSAR_MEM= -Xms512m -Xmx512m -XX:MaxDirectMemorySize=512m") + config.Env = append(config.Env, "PULSAR_MEM=-Xms512m -Xmx512m -XX:MaxDirectMemorySize=512m")examples/nginx/nginx_test.go (1)
19-21: Schedule cleanup after confirming container started (or guard nil).Calling CleanupContainer before checking err risks scheduling cleanup on a zero/invalid container if start fails.
- nginxC, err := startContainer(ctx) - testcontainers.CleanupContainer(t, nginxC) - require.NoError(t, err) + nginxC, err := startContainer(ctx) + require.NoError(t, err) + testcontainers.CleanupContainer(t, nginxC)If CleanupContainer already handles nil/zero values safely, acknowledge and keep; otherwise apply the refactor.
logconsumer_test.go (3)
98-106: Close http response bodies and bind requests to ctx.http.Get leaves bodies open; over many calls this leaks connections. Use a ctx-aware request and always close the body. A small helper keeps tests concise.
Add helper (outside these ranges):
func httpGetRequireOK(t *testing.T, ctx context.Context, url string) { t.Helper() req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) require.NoError(t, err) resp, err := http.DefaultClient.Do(req) require.NoError(t, err) io.Copy(io.Discard, resp.Body) // ignore content _ = resp.Body.Close() }Apply for the first block as example:
- _, err = http.Get(ep + "/stdout?echo=hello") - require.NoError(t, err) - - _, err = http.Get(ep + "/stdout?echo=there") - require.NoError(t, err) - - _, err = http.Get(ep + "/stdout?echo=" + lastMessage) - require.NoError(t, err) + httpGetRequireOK(t, ctx, ep+"/stdout?echo=hello") + httpGetRequireOK(t, ctx, ep+"/stdout?echo=there") + httpGetRequireOK(t, ctx, ep+"/stdout?echo="+lastMessage)Repeat similarly for the other ranges above.
Also applies to: 157-165, 207-212, 415-423, 557-562, 588-593
53-58: Return a copy from Msgs() to avoid data races with concurrent writers.Returning the backing slice allows concurrent mutation by Accept, which can trip -race or produce flaky reads. Return a copied slice.
func (g *TestLogConsumer) Msgs() []string { g.mtx.Lock() defer g.mtx.Unlock() - - return g.msgs + cp := make([]string, len(g.msgs)) + copy(cp, g.msgs) + return cp }
60-68: Optional: avoid a goroutine that can outlive the test.devNullAcceptorChan spawns a goroutine that’s never signaled to stop. Not harmful in tests, but you can tie it to t.Cleanup or use a buffered channel to avoid the extra goroutine.
Example:
- Close the channel in t.Cleanup to let the goroutine exit.
- Or replace with: make(chan string, 1); drop sends with non-blocking default case to avoid blocking.
modules/redpanda/redpanda_test.go (4)
166-174: Close Kafka client to avoid leaks in testsAdd a Close to free resources in the “invalid-user” subtest.
Apply:
kafkaCl, err := kgo.NewClient( kgo.SeedBrokers(seedBroker), kgo.SASL(scram.Auth{ User: "wrong", Pass: "wrong", }.AsSha256Mechanism()), ) require.NoError(t, err) + defer kafkaCl.Close()
275-283: Close Kafka client in bootstrap auth “invalid-user” subtestMirror other subtests and close the client.
kafkaCl, err := kgo.NewClient( kgo.SeedBrokers(seedBroker), kgo.SASL(scram.Auth{ User: "wrong", Pass: "wrong", }.AsSha256Mechanism()), ) require.NoError(t, err) + defer kafkaCl.Close()
384-392: Close Kafka client in old-version “invalid-user” subtestPrevent resource leaks.
kafkaCl, err := kgo.NewClient( kgo.SeedBrokers(seedBroker), kgo.SASL(scram.Auth{ User: "wrong", Pass: "wrong", }.AsSha256Mechanism()), ) require.NoError(t, err) + defer kafkaCl.Close()
401-409: Close Kafka client in “wrong-mechanism” subtestAdd a defer Close.
kafkaCl, err := kgo.NewClient( kgo.SeedBrokers(seedBroker), kgo.SASL(plain.Auth{ User: "no-superuser", Pass: "test", }.AsMechanism()), ) require.NoError(t, err) + defer kafkaCl.Close()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
.golangci.yml(2 hunks)container_test.go(6 hunks)docker_auth_test.go(9 hunks)docker_client_test.go(2 hunks)docker_exec_test.go(3 hunks)docker_files_test.go(5 hunks)docker_test.go(55 hunks)examples/nginx/nginx_test.go(1 hunks)examples_test.go(1 hunks)from_dockerfile_test.go(6 hunks)generic_test.go(5 hunks)image_substitutors_test.go(2 hunks)image_test.go(6 hunks)internal/core/docker_host_test.go(25 hunks)internal/core/docker_rootless_test.go(6 hunks)lifecycle_test.go(7 hunks)logconsumer_test.go(11 hunks)modulegen/main_test.go(1 hunks)modules/aerospike/aerospike_test.go(2 hunks)modules/arangodb/arangodb_test.go(2 hunks)modules/arangodb/examples_test.go(1 hunks)modules/artemis/artemis_test.go(2 hunks)modules/azure/azurite/azurite_test.go(3 hunks)modules/azure/azurite/examples_test.go(12 hunks)modules/azure/eventhubs/eventhubs_test.go(3 hunks)modules/azure/eventhubs/examples_test.go(3 hunks)modules/azure/servicebus/examples_test.go(4 hunks)modules/azure/servicebus/servicebus_test.go(3 hunks)modules/azurite/azurite_test.go(1 hunks)modules/cassandra/cassandra_test.go(4 hunks)modules/chroma/chroma_test.go(2 hunks)modules/chroma/examples_test.go(9 hunks)modules/clickhouse/clickhouse_test.go(9 hunks)modules/cockroachdb/cockroachdb_test.go(2 hunks)modules/compose/compose_api_test.go(30 hunks)modules/compose/compose_examples_test.go(3 hunks)modules/compose/compose_test.go(3 hunks)modules/consul/consul_test.go(1 hunks)modules/couchbase/couchbase_test.go(7 hunks)modules/databend/databend_test.go(2 hunks)modules/dind/dind_test.go(1 hunks)modules/dockermcpgateway/dockermcpgateway_test.go(4 hunks)modules/dockermodelrunner/docker_desktop_test.go(2 hunks)modules/dockermodelrunner/dockermodelrunner_test.go(2 hunks)modules/dockermodelrunner/examples_test.go(6 hunks)modules/dolt/dolt_test.go(6 hunks)modules/dynamodb/dynamodb_test.go(9 hunks)modules/elasticsearch/elasticsearch_test.go(4 hunks)modules/etcd/etcd_test.go(3 hunks)modules/etcd/etcd_unit_test.go(4 hunks)modules/gcloud/bigquery/bigquery_test.go(1 hunks)modules/gcloud/bigtable/bigtable_test.go(1 hunks)modules/gcloud/datastore/datastore_test.go(1 hunks)modules/gcloud/firestore/firestore_test.go(2 hunks)modules/gcloud/pubsub/pubsub_test.go(1 hunks)modules/gcloud/spanner/spanner_test.go(1 hunks)modules/grafana-lgtm/grafana_test.go(1 hunks)modules/inbucket/inbucket_test.go(1 hunks)modules/influxdb/influxdb_test.go(5 hunks)modules/k3s/k3s_test.go(3 hunks)modules/k6/k6_test.go(1 hunks)modules/kafka/kafka_test.go(4 hunks)modules/localstack/localstack_test.go(4 hunks)modules/localstack/v1/s3_test.go(1 hunks)modules/localstack/v2/s3_test.go(2 hunks)modules/mariadb/mariadb_test.go(6 hunks)modules/meilisearch/meilisearch_test.go(2 hunks)modules/memcached/memcached_test.go(1 hunks)modules/milvus/milvus_test.go(2 hunks)modules/minio/minio_test.go(1 hunks)modules/mongodb/atlaslocal/atlaslocal_test.go(27 hunks)modules/mongodb/mongodb_test.go(1 hunks)modules/mssql/mssql_test.go(6 hunks)modules/mysql/examples_test.go(0 hunks)modules/mysql/mysql_test.go(4 hunks)modules/nats/examples_test.go(1 hunks)modules/nats/nats_test.go(2 hunks)modules/nebulagraph/nebulagraph_test.go(1 hunks)modules/neo4j/neo4j_test.go(3 hunks)modules/ollama/examples_test.go(3 hunks)modules/ollama/local_test.go(5 hunks)modules/ollama/ollama_test.go(7 hunks)modules/openfga/examples_test.go(8 hunks)modules/openfga/openfga_test.go(1 hunks)modules/openldap/openldap_test.go(4 hunks)modules/opensearch/opensearch_test.go(1 hunks)modules/pinecone/pinecone_test.go(2 hunks)modules/postgres/postgres_test.go(15 hunks)modules/pulsar/pulsar_test.go(1 hunks)modules/qdrant/examples_test.go(9 hunks)modules/qdrant/qdrant_test.go(1 hunks)modules/rabbitmq/examples_test.go(2 hunks)modules/rabbitmq/rabbitmq_test.go(6 hunks)modules/redis/redis_test.go(5 hunks)modules/redpanda/redpanda_test.go(13 hunks)modules/registry/examples_test.go(5 hunks)modules/registry/registry_test.go(9 hunks)modules/scylladb/scylladb_test.go(5 hunks)modules/socat/socat_test.go(5 hunks)modules/solace/examples_test.go(2 hunks)modules/solace/solace_test.go(2 hunks)modules/surrealdb/surrealdb_test.go(2 hunks)modules/toxiproxy/options_test.go(1 hunks)modules/toxiproxy/toxiproxy_test.go(2 hunks)modules/valkey/valkey_test.go(6 hunks)modules/vault/vault_test.go(3 hunks)modules/vearch/vearch_test.go(1 hunks)
⛔ Files not processed due to max files limit (20)
- modules/weaviate/weaviate_test.go
- modules/yugabytedb/yugabytedb_test.go
- mounts_test.go
- network/network_test.go
- options_test.go
- parallel_test.go
- port_forwarding_test.go
- provider_test.go
- reaper_test.go
- reuse_test.go
- wait/all_test.go
- wait/exec_test.go
- wait/exit_test.go
- wait/file_test.go
- wait/health_test.go
- wait/host_port_test.go
- wait/http_test.go
- wait/log_test.go
- wait/sql_test.go
- wait/tls_test.go
💤 Files with no reviewable changes (1)
- modules/mysql/examples_test.go
🚧 Files skipped from review as they are similar to previous changes (62)
- modules/dockermcpgateway/dockermcpgateway_test.go
- modules/dockermodelrunner/dockermodelrunner_test.go
- modules/postgres/postgres_test.go
- modules/toxiproxy/options_test.go
- generic_test.go
- modules/kafka/kafka_test.go
- modules/influxdb/influxdb_test.go
- modules/dind/dind_test.go
- modules/gcloud/bigquery/bigquery_test.go
- modules/registry/registry_test.go
- modules/azure/azurite/azurite_test.go
- modules/meilisearch/meilisearch_test.go
- modules/solace/solace_test.go
- modules/chroma/examples_test.go
- modules/mysql/mysql_test.go
- modules/cassandra/cassandra_test.go
- modules/azure/servicebus/servicebus_test.go
- modules/minio/minio_test.go
- modules/gcloud/datastore/datastore_test.go
- modules/dockermodelrunner/docker_desktop_test.go
- modules/openfga/openfga_test.go
- modules/neo4j/neo4j_test.go
- modules/openldap/openldap_test.go
- modules/ollama/ollama_test.go
- container_test.go
- modules/openfga/examples_test.go
- modules/nats/nats_test.go
- internal/core/docker_host_test.go
- modules/scylladb/scylladb_test.go
- internal/core/docker_rootless_test.go
- image_substitutors_test.go
- modules/k6/k6_test.go
- modules/qdrant/examples_test.go
- modules/aerospike/aerospike_test.go
- modules/qdrant/qdrant_test.go
- modules/couchbase/couchbase_test.go
- lifecycle_test.go
- modules/mssql/mssql_test.go
- modules/opensearch/opensearch_test.go
- examples_test.go
- modules/compose/compose_api_test.go
- docker_files_test.go
- modules/etcd/etcd_unit_test.go
- modules/elasticsearch/elasticsearch_test.go
- modules/arangodb/arangodb_test.go
- modules/arangodb/examples_test.go
- modules/vault/vault_test.go
- modules/gcloud/pubsub/pubsub_test.go
- modules/ollama/examples_test.go
- modulegen/main_test.go
- modules/nebulagraph/nebulagraph_test.go
- docker_exec_test.go
- modules/toxiproxy/toxiproxy_test.go
- modules/memcached/memcached_test.go
- modules/etcd/etcd_test.go
- modules/gcloud/firestore/firestore_test.go
- modules/valkey/valkey_test.go
- modules/mariadb/mariadb_test.go
- .golangci.yml
- modules/nats/examples_test.go
- modules/grafana-lgtm/grafana_test.go
- modules/mongodb/atlaslocal/atlaslocal_test.go
🧰 Additional context used
🧬 Code graph analysis (11)
docker_client_test.go (1)
modulegen/internal/context/context.go (1)
Context(12-14)
docker_test.go (2)
generic.go (1)
Run(122-149)docker_client.go (1)
NewDockerClientWithOpts(117-143)
image_test.go (3)
internal/core/docker_host.go (1)
MustExtractDockerHost(87-98)testing.go (1)
CleanupContainer(91-97)docker.go (1)
SaveDockerImageWithPlatforms(1827-1833)
docker_auth_test.go (1)
docker_auth.go (1)
DockerImageAuth(30-38)
modules/cockroachdb/cockroachdb_test.go (2)
modules/cockroachdb/cockroachdb.go (1)
Run(169-234)modules/cockroachdb/options.go (2)
WithPassword(37-49)WithInsecure(109-119)
modules/compose/compose_test.go (1)
docker_client.go (1)
NewDockerClientWithOpts(117-143)
modules/registry/examples_test.go (1)
generic.go (2)
GenericContainer(52-98)GenericContainerRequest(21-27)
modules/dynamodb/dynamodb_test.go (2)
modules/dynamodb/dynamodb.go (3)
Run(22-53)WithDisableTelemetry(73-80)WithSharedDB(61-70)testing.go (1)
CleanupContainer(91-97)
modules/solace/examples_test.go (1)
modules/solace/solace.go (1)
Container(23-26)
modules/rabbitmq/examples_test.go (1)
container.go (1)
Container(41-73)
modules/dockermodelrunner/examples_test.go (1)
log/logger.go (1)
Default(38-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: lint (modules/neo4j) / lint: modules/neo4j
- GitHub Check: lint (modules/nats) / lint: modules/nats
- GitHub Check: lint (modules/etcd) / lint: modules/etcd
- GitHub Check: lint (modules/azurite) / lint: modules/azurite
- GitHub Check: lint (modules/kafka) / lint: modules/kafka
- GitHub Check: lint (modules/elasticsearch) / lint: modules/elasticsearch
- GitHub Check: lint (modules/couchbase) / lint: modules/couchbase
- GitHub Check: Analyze (go)
🔇 Additional comments (31)
modules/mongodb/mongodb_test.go (1)
120-120: Use the subtest’s contextInside the subtest closure you still pull the parent’s context via
t.Context(). This breaks per-subtest cancellation/timeouts the ustesting linter expects. Switch tott.Context()(and keep using thatctxeverywhere below).- ctx := t.Context() + ctx := tt.Context()modules/azurite/azurite_test.go (1)
13-13: Verify Go version supportst.Context().
testing.T.Context()is only available starting from Go 1.20. Please double-check that the module’sgo.mod(and our CI toolchain) targets at least that version; otherwise this test won’t compile.modules/solace/examples_test.go (1)
76-79: Context propagation fix looks goodPassing the caller-provided context all the way to
BrokerURLForensures the helper honors cancellation/deadline from upstream instead of hardcodingcontext.Background(). Nice tightening for the new linter.modules/rabbitmq/rabbitmq_test.go (2)
20-92: Switching tests to test-scoped context looks solidUsing
t.Context()in these test cases keeps container startup and URL retrieval tied to the test lifecycle, which is exactly what theusetestinglinter asks for. 👍
221-258: Helper lookups now respect test cancellationDeriving the context from
tinside the helpers ensures every exec call is cancelled with the test, keeping the helpers consistent with the new lint rule.modules/localstack/localstack_test.go (3)
68-80: Good use of per-test context for Docker host detectionRouting the DaemonHost lookup through
t.Context()ties the provider call to the test’s lifecycle and matches the expectations of the new linter—looks perfect.
170-199: Container run/inspect now respect test cancellationCapturing
ctx := t.Context()once per iteration keeps the container lifecycle bounded by the parent test, which is exactly what we want here. Nicely done.
202-259: Per-test context propagated through network and client setupThe switch to
t.Context()flows cleanly through network creation, container startup, and the AWS CLI helper, so everything cancels in sync with the test. All good.modules/redis/redis_test.go (2)
19-41: Context handling now aligned with the test lifecycleInitializing the container and helper assertions with
t.Context()ensures clean cancellation and satisfies the new lint rule—looks great.
44-91: Shared test context works smoothly across TLS subtestsReusing the outer
t.Context()keeps each subtest’s operations within the parent test scope, and the helper continues to receive the right context. No issues spotted.modules/inbucket/inbucket_test.go (1)
14-46: Per-test context now governs the entire mail flowSwitching to
t.Context()for container operations and REST calls keeps everything bound to the test’s lifecycle—implementation looks solid.modules/k3s/k3s_test.go (3)
23-93: Deadline derived from t.Context keeps cluster work in checkWrapping the per-test context with a specific deadline still honors test cancellation, and the rest of the load-image workflow follows suit. Nicely executed.
106-141: API server readiness checks now respect t.ContextPropagating
t.Context()through kubeconfig access and pod creation keeps the client-go calls cancel-safe. Looks good.
144-153: Manifest option respects test-managed contextRunning k3s with manifest and wait strategy under
t.Context()ties the lifecycle together cleanly. No concerns here.modules/cockroachdb/cockroachdb_test.go (2)
38-51: Error-path tests now leverage per-test contextUsing
t.Context()for the invalid option combinations keeps the short-lived runs cancellable and lint-compliant. Looks good.
56-94: Primary container test correctly migrates to t.Context()The new context wiring flows through
Run,ConnectionConfig, and the SQL checks without issue. Everything remains tidy and cancellable with the test.modules/pulsar/pulsar_test.go (1)
100-106: Guard against nil container in CleanupContainerCleanupContainer is called before checking
err; verify thattestcontainers.CleanupContainer(t, c)safely handlesc == nilto avoid panics.examples/nginx/nginx_test.go (1)
17-17: Baseline and linter checks passed go.mod specifies Go 1.24.0 and theusetestinglinter is enabled in golangci-lint.modules/ollama/local_test.go (5)
310-316: LGTM: using t.Context() here is correct and consistent with the linter.
357-366: LGTM: correct adoption of t.Context() in this test.
441-444: LGTM: t.Context() usage is appropriate.
532-538: LGTM: migrating to t.Context() aligns with usetesting expectations.
43-49: Good switch to t.Context(); minimum Go version ≥1.22 satisfied (toolchain go1.24.7 in go.mod; CI matrix tests go1.24.x & 1.25.x).docker_client_test.go (3)
12-13: Good switch to t.Context()Using the per-test context is correct and lints cleanly.
22-23: LGTM on per-test context in goroutine testAppropriate use of t.Context() and WaitGroup.
10-19: Verify no lingering Background()/TODO() in tests and Go 1.22+ for t.Context()Repo should consistently use t.Context() in tests and target Go ≥1.22 for this API.
Run:
#!/bin/bash set -euo pipefail echo "go.mod go version/toolchain:" rg -nP '^(go\s+\d+\.\d+|toolchain\s+go\d+\.\d+)' go.mod || true echo echo "Occurrences of context.Background()/TODO() in *_test.go (review and ignore Example* as needed):" rg -nP --type=go -g "*_test.go" -g '!**/vendor/**' '\bcontext\.(Background|TODO)\(' -n -C2 || truemodules/dolt/dolt_test.go (1)
18-19: Consistent adoption of t.Context()Good migration to per-test contexts across the suite.
Also applies to: 46-47, 58-59, 82-83, 99-100, 128-129
modules/redpanda/redpanda_test.go (1)
31-32: Per-test context migration looks goodConsistent use of t.Context() across Redpanda tests.
Also applies to: 93-94, 208-209, 318-319, 443-444, 464-465, 527-528, 569-570, 632-633, 662-663, 717-718, 756-757
docker_auth_test.go (1)
105-111: LGTM on using t.Context() throughoutThe context source is now tied to the test lifecycle; changes look correct.
Also applies to: 118-124, 133-137, 153-157, 161-173, 178-193, 201-218, 226-242, 250-255, 259-284
docker_test.go (2)
58-60: Broad adoption of t.Context() is correctChanges align with the usetesting linter and improve cancellation semantics.
Also applies to: 118-120, 154-156, 176-176, 208-209, 224-225, 236-237, 274-276, 303-304, 338-339, 368-369, 419-420, 439-440, 451-452, 481-482, 495-496, 509-510, 552-553, 579-580, 599-600, 617-618, 629-630, 645-646, 664-665, 683-684, 865-866, 889-890, 942-943, 1000-1001, 1009-1010, 1030-1031, 1048-1049, 1052-1053, 1057-1058, 1062-1065, 1068-1069, 1075-1076, 1110-1111, 1131-1132, 1196-1197, 1266-1267, 1305-1306, 1330-1331, 1362-1363, 1406-1407, 1432-1433, 1451-1452, 1472-1473, 1501-1502, 1507-1509, 1569-1570, 1610-1611, 1739-1740, 1799-1800, 1860-1861, 1874-1875, 1884-1885
1060-1069: Using tb.Context() in helpers requires Go 1.22+readHostname now relies on TB having Context(); with go 1.24 this is fine. Just ensure the toolchain is ≥1.22 in go.mod.
Run:
#!/bin/bash set -euo pipefail rg -nP '^(go\s+\d+\.\d+|toolchain\s+go\d+\.\d+)' go.mod || true
|
|
||
| func Test_LogConsumerGetsCalled(t *testing.T) { | ||
| ctx := context.Background() | ||
| ctx := t.Context() |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Good switch to t.Context(); verify Go 1.22+ in go.mod and CI.
t.Context() requires Go ≥1.22. Please confirm the module’s go directive and CI matrix reflect that, otherwise builds on older Go will fail.
Run:
Also applies to: 131-131, 175-175, 236-236, 346-346, 364-364, 387-387, 433-433, 460-460, 529-531, 654-654
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "go.mod go directive:"
rg -nP '^\s*go\s+\d+\.\d+' go.mod || true
echo
echo "Occurrences of t.Context():"
rg -nP --type=go -C2 '\bt\.Context\(\)' || true
echo
echo "GitHub Actions go-version entries:"
fd -t f '.github/workflows' 2>/dev/null | xargs -I{} rg -nP -C2 'go-version\s*:\s*.*' {} || trueLength of output: 193224
Ensure CI uses Go ≥1.22
go.mod is set to 1.24.0. No go-version entries found in .github/workflows — update your GitHub Actions workflows to pin and test on Go ≥1.22.
🤖 Prompt for AI Agents
In logconsumer_test.go around line 71, the CI configuration doesn't pin a Go
version despite go.mod requiring 1.24.0; update your GitHub Actions workflow
files to pin and test with Go ≥1.22 (preferably 1.24 to match go.mod). Edit any
workflows in .github/workflows to add or update actions/setup-go entries (or the
matrix) to include a go-version like "1.24" or include versions >=1.22, and
ensure jobs use that setup so CI runs with a compatible Go toolchain.
728efc4 to
1fd7782
Compare
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
mmorel-35
force-pushed
the
usetesting
branch
from
1fd7782 to
ae9e0a8
Compare
|
Curious, why disable |
2 participants
What does this PR do?
Enables and fixes issues from usetesting linter
Why is it important?
Related issues