Repository navigation
fix(rueidiscompatmock): make Cache() mockable via the expectation queue - #1023
Merged
rueian merged 11 commits intoAug 24, 2026
Merged
Conversation
This was referenced Aug 16, 2026
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
rueian
reviewed
Aug 19, 2026
…tations Addresses review feedback on redis#1023: sharing ExpectGet between a plain Get and a Cache(ttl).Get() call meant a test couldn't require that a specific call went through the cache path. ViaCache() is an opt-in chainable modifier on every Expected* builder; a marked expectation only matches a call routed through DoCache, while unmarked expectations keep matching either origin exactly as before.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 59eaff3. Configure here.
…ectations Cursor Bugbot found that in MatchExpectationsInOrder(false) mode, an unmarked expectation queued ahead of a ViaCache()-marked expectation for the same command could "steal" a cached call, since the unordered scan picked the first matching entry regardless of specificity. matchLocked now scans for a requireCache=true candidate first when the call arrived via DoCache, falling back to the original scan otherwise. Plain calls are unaffected. Verified the new regression test actually fails without the fix before restoring it.
…uted calls Per @rueian's review on redis#1023: a call routed through Cache(ttl) could previously be satisfied by an unmarked expectation as a compatibility fallback, so ExpectationsWereMet() passed even when a test never asserted the call went through the cache path. Matching is now symmetric: expectation.matches rejects the call whenever e.requireCache != viaCache, so a plain ExpectGet only matches a plain call and a ViaCache() expectation only matches a cache-routed call. Updates the TestCacheCompat* suite for the new contract and replaces the two tests that codified the old fallback with tests proving it no longer applies. Documents the symmetric behavior in rueidiscompatmock/README.md.
rueian
reviewed
Aug 23, 2026
The comment on ViaCache() still described the old "unmarked expectations match either origin" fallback that was removed in 3852a76. Flagged as stale by @rueian on redis#1023.
rueian
reviewed
Aug 23, 2026
rueian
reviewed
Aug 23, 2026
…tion error, remove duplicate test, gofmt Addresses three review comments from @rueian on redis#1023: - adapter.go: the "no expectation for command" error now appends "via cache" when the unmatched call was routed through DoCache, so a forgotten .ViaCache() is distinguishable from an ordinary mismatch at a glance. Factored into a shared unmatchedErr helper used by both consume and consumeWithRaw. Added TestCacheCompatUnmatchedPlainCallOmitsViaCache and tightened TestCacheCompatUnmatched to assert the suffix. - adapter_test.go: removed TestCacheCompatGet, which became an exact duplicate of TestCacheCompatViaCacheRequired once ViaCache() was added to it in 3852a76. - adapter_test.go: applied gofmt (100*time.Millisecond, no spaces around *) per review suggestion.
rueian
reviewed
Aug 24, 2026
rueian
approved these changes
Aug 24, 2026
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
2 participants
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.
rueidiscompatmocknow supports mocking client-side caching.Cache(ttl)commands dispatch through the existingExpect*expectation queue instead of panicking withunexpected call to DoCache(issue #972). The newDoCachestub reuses the same ordered queue asDo/DoMulti, so noExpect*API changed and the TTL argument is deliberately not matched (documented in the README).Design notes:
rueidiscompatmockonly;rueidiscompatand the rootmock/package are untouched. This follows the maintainer's direction on fix(rueidiscompat): make CacheCompat a mockable interface #1021 instead of the rejected interface-ization ofCacheCompat.DoMultiCacheis not stubbed: therueidiscompatcache surface never calls it (verified against the receiver block inrueidiscompat/adapter.go).Validated with the module's full test suite including
-race. Eight newTestCacheCompat*tests cover dispatch through both adapter entry points, multi-command queues,Cacheable-wrapped commands, nil/error results, TTL irrelevance, and unmatched-command reporting.Fixes #972.
Related: #1021
Note
Low Risk
Test-helper only; no production Redis client or auth/data path changes. Matching origin (cache vs plain) is a behavior change for existing mock tests that mix Cache() with unmarked Expect* calls.
Overview
Makes
rueidiscompat.Cache(ttl)commands mockable inrueidiscompatmockinstead of panicking onDoCache. Cached calls reuse the existingExpect*queue; TTL is not matched.Matching is origin-aware:
ExpectGet("k").ViaCache()only matchesrdb.Cache(ttl).Get(...), and unmarked expectations only match plain calls. Unordered matching still prefers the origin-specific entry rather than the first same-command expectation.ViaCache()is added on all expected-result types. Unmatched cache calls reportvia cachein the error.DoMultiCacheis not stubbed.Reviewed by Cursor Bugbot for commit def365b. Bugbot is set up for automated code reviews on this repo. Configure here.