test: trying out new deps#27
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #27 +/- ##
==========================================
+ Coverage 90.48% 90.70% +0.22%
==========================================
Files 285 281 -4
Lines 9078 8996 -82
==========================================
- Hits 8214 8160 -54
+ Misses 589 558 -31
- Partials 275 278 +3
|
This reverts commit e0ef5b4.
…ning consumer
`(*redis.Client).Subscribe(ctx, topic)` in go-redis v8 is asynchronous
— it returns a *PubSub before the SUBSCRIBE command has reached the
server. The go-redis source documents this explicitly in a doc comment
at redis.go line 660: callers are supposed to call Receive() on the
returned *PubSub to block until the server confirms the subscription,
otherwise a publisher racing the subscriber will silently drop the
first message (Redis pub/sub does not buffer for late subscribers).
provideRedisConsumer was skipping that step, so
Test_redisConsumer_Consume/with_error_handling_message would
intermittently hang for the full 10-minute test deadline: SUBSCRIBE and
PUBLISH raced, PUBLISH won, the message was dropped server-side, the
handler never fired, and the bare <-errorsChan receive blocked forever.
The "redis: discarding bad PubSub connection: EOF" log lines everyone
was blaming on testcontainers are the internal health-check goroutine
seeing its socket torn out when the container was killed at the
deadline — symptom, not cause.
Fix:
- provideRedisConsumer now calls subscription.Receive(ctx) right after
Subscribe and propagates any error. Its signature becomes
(*redisConsumer, error).
- The log line "subscribed to topic!" now fires *after* the server
confirms, so it reflects reality.
- (*consumerProvider).ProvideConsumer propagates the error and
constructs the consumer outside the cache mutex — we don't want to
serialize the SUBSCRIBE round-trip behind the lock. A re-check under
the write lock protects against duplicate construction on concurrent
callers.
Behavior change worth calling out for bisect: ProvideConsumer is now
fail-fast. If Redis is unreachable at provisioning time, the caller
gets an error immediately instead of a silently-broken consumer whose
messages would have been dropped anyway. Services that call
ProvideConsumer during startup wiring will now fail startup on queue
unavailability, which is strictly better than booting into a broken
state.
Test changes:
- Test_consumerProvider_ProvideConsumer/standard and /hitting_cache
were passing fake addresses (QueueAddresses: []string{t.Name()}) and
only worked because Subscribe never dialed. Replaced with
container-backed equivalents that actually exercise the SUBSCRIBE
round-trip and verify the cache returns the same instance on a
second call.
- Test_consumerProvider_ProvideConsumer/with_empty_topic kept as-is —
it returns before Subscribe is reached, fake address never dialed.
- Test_redisConsumer_Consume's channels are now buffered and the
error-handler subtest uses select+timeout on errorsChan. Strictly
defense-in-depth now that the root race is fixed, but it means a
future regression fails in seconds instead of ten minutes.
Stress-tested: 20x Test_redisConsumer_Consume runs in 31s, 10x
full-package runs in 17s, zero flakes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Swap `github.com/go-redis/redis/v8` for `github.com/redis/go-redis/v9` (the module moved from the go-redis org to the redis org). v8 has been unmaintained since early 2023; v9 is the current maintained line. Pure mechanical port — this codebase uses none of the APIs v9 broke: - No custom `redis.Hook` implementations (v9 replaced `BeforeProcess`/ `AfterProcess` with `DialHook`/`ProcessHook`/`ProcessPipelineHook`). - No `redisotel` instrumentation (all tracing/metrics is wrapped at the platform level via our own `tracing.Tracer` and `metrics.Provider`). - No `WithContext()` calls (we already pass ctx as the first arg everywhere, which is the v9-mandatory style). - No hook-based command interception. Every go-redis API we actually touch is stable between v8.11.5 and v9.18.0: `Options`/`ClusterOptions` fields, `NewClient`/`NewClusterClient`, `Get`/`Set`/`Del`/`Ping`/`SetNX`/`Eval`/`Publish`/`Subscribe`/`Close`, `*Cmd`/`*BoolCmd`/`*IntCmd`/`*StatusCmd`/`*StringCmd` return types and their `.Result()`/`.Int64()`/`.Err()` accessors, `*PubSub.Receive`/ `.Channel`, `ChannelOption`, `redis.Nil` sentinel, `redis.ErrClosed`, and the mock command constructors `NewCmd`/`NewBoolCmd`/`NewIntCmd`/ `NewStatusCmd`/`NewStringCmd` plus their `SetVal`/`SetErr` methods. Touches four production packages — `messagequeue/redis`, `cache/redis`, `ratelimiting/redis`, `distributedlock/redis` — plus their tests. Also picks up a handful of small in-flight test assertions added to `publisher_test.go` as part of the ongoing testing-refactor work on this branch; they were mixed in the working tree. Verified: - `go build ./...` clean. - `go vet ./messagequeue/redis/... ./cache/redis/... ./ratelimiting/redis/... ./distributedlock/redis/...` clean. - Full test suite green (race, shuffle, failfast). - Stress test: 5x Test_redisConsumer_Consume passes on v9. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR aim to accomplish?
wanna see what matryer/moq and shoenig/test would look and feel like