Skip to content

fix(sandbox): implement Redis set ops and raw command/pipeline instead of raising - #53

Merged
MikaAK merged 3 commits into
mainfrom
fix/sandbox-redis-set-ops
Sep 3, 2026
Merged

MikaAK merged 3 commits into
mainfrom
fix/sandbox-redis-set-ops

Conversation

@MikaAK

@MikaAK MikaAK commented Aug 18, 2026

Copy link
Copy Markdown
Owner

Summary

Cache.Sandbox raised "Not Implemented" from smembers/3, sadd/4, command/3, command!/3, pipeline/3 and pipeline!/3.

Under sandbox?: Mix.env() === :test the use Cache wrappers delegate to Cache.Sandbox, so dialyzer (running in test) has been correctly inferring that every Redis cache module's injected command/1,2, command!/1,2, pipeline/1,2, pipeline!/1,2, sadd/2,3, smembers/2 have no local return — 11 no_return warnings per Redis cache module, forcing a :no_return ignore entry per module in every consuming app.

  • sadd/smembers — real in-memory sets (MapSet under the key), Redis semantics: SADD returns the new-member count, SMEMBERS of a missing key is [].
  • command/pipeline — PING, GET, EXISTS, DEL mapped onto the sandbox map; anything else returns {:error, %ErrorMessage{code: :not_implemented}} — the same error shape the Redis adapter produces on failure — instead of raising. Raw GET returns the stored binary undecoded, exactly as Redis would (the Redis-backed sandbox term-encodes on put). pipeline runs commands in order and halts on the first error.
  • Bang variants raise only on that error branch, so they have a real success typing too.

Verification

  • New test/cache/redis_sandbox_test.exs (11 tests, sandbox only, no Redis needed) — green.
  • Dialyzer on a sandbox?: true Redis cache module: main produces the 11 no_return warnings above; this branch produces 0.
  • Consuming umbrella (cheddar_flow_ex, 6 Redis cache modules): with this branch as a path dep and all six :no_return ignore entries deleted, dialyzer goes 174 → 108 total warnings, all remaining ones already skipped, 0 unnecessary skips.
  • Full suite: the 13 pre-existing real-Redis (RedisJSONTest/RedisHashTest under parallel contention) failures are unchanged from main; everything else green.

@codecov

codecov Bot commented Aug 18, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.89%. Comparing base (5a2700f) to head (fa45e0b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #53   +/-   ##
=======================================
  Coverage   83.89%   83.89%           
=======================================
  Files          24       24           
  Lines         708      708           
=======================================
  Hits          594      594           
  Misses        114      114           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

…d of raising

Cache.Sandbox raised "Not Implemented" from smembers/3, sadd/4, command/3,
command!/3, pipeline/3 and pipeline!/3. Under sandbox?: Mix.env() === :test
the use-Cache wrappers delegate to Cache.Sandbox, so dialyzer (running in
test) correctly inferred every Redis cache module's injected command/1,
pipeline/1, sadd/2, smembers/2 ... had no local return — forcing a
:no_return ignore entry per Redis cache module in every consuming app.

- sadd/smembers: real in-memory sets (MapSet under the key), Redis
  semantics — SADD returns the new-member count, SMEMBERS of a missing key
  is [].
- command/pipeline: PING, GET, EXISTS, DEL mapped onto the sandbox map;
  anything else returns {:error, %ErrorMessage{code: :not_implemented}} —
  the same error shape the Redis adapter produces — instead of raising.
  The bang variants raise only on that error branch, so they have a real
  success typing too.
- pipeline runs commands in order and halts on the first error.
@MikaAK
MikaAK force-pushed the fix/sandbox-redis-set-ops branch from c8dbe37 to fa45e0b Compare September 3, 2026 07:12
@MikaAK
MikaAK merged commit a201bf1 into main Sep 3, 2026
5 checks passed
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.

1 participant