Skip to content

migrator: tolerate context.Canceled in consumer group test stream goroutine#4302

Merged
Jeffail merged 1 commit intomainfrom
mmt/migrator-fixes-v2
May 5, 2026
Merged

migrator: tolerate context.Canceled in consumer group test stream goroutine#4302
Jeffail merged 1 commit intomainfrom
mmt/migrator-fixes-v2

Conversation

@mmatczuk
Copy link
Copy Markdown
Contributor

Commits

  • tolerate context.Canceled in consumer group test stream goroutine

Jira

  • CON-446

@claude
Copy link
Copy Markdown

claude Bot commented Apr 20, 2026

Commits
LGTM

Review
Single-file change that replaces require.NoError(t, stream.Run(ctx)) with the documented pattern of tolerating context.Canceled and using t.Errorf from a background goroutine. Matches the async-operations guidance in the project test patterns (see integration_test.go#L420-L426).

LGTM

This was referenced Apr 20, 2026
Copy link
Copy Markdown
Contributor

@Jeffail Jeffail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — correct narrow fix. context.Canceled is genuinely expected here: the goroutine's deferred cancel() races stream.StopWithin on shutdown and either ordering is benign. The require.NoErrort.Errorf swap also fixes the latent FailNow-from-non-test-goroutine bug per the testing package contract. Matches the dominant repo idiom.

Worth a follow-up (out of scope for this PR): the RunStreamInBackground helper at internal/impl/snowflake/integration_test.go:143 already implements this exact pattern (cleanup-driven cancel + wg.Wait + Canceled tolerance + non-FailNow assertion). Promoting it to a shared test helper would let us migrate the remaining require.NoError(t, stream.Run(...))-in-goroutine call sites across this file (e.g. L633, L688, L789, L901) and the sibling packages in one sweep instead of landing per-call-site one-line PRs.

@Jeffail Jeffail merged commit 3d74524 into main May 5, 2026
7 checks passed
@Jeffail Jeffail deleted the mmt/migrator-fixes-v2 branch May 5, 2026 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants