Skip to content

Commit 5b41477

Browse files
os-warrenclaude
andauthored
docs(service-cluster-redis): record the measured ioredis 5-vs-6 gap in the contract test header, and drop a live-Redis path that never existed (#15985)
* docs(service-cluster-redis): record the measured ioredis 5-vs-6 gap in the contract test header, and drop a promised live-Redis path that never existed The contract test runs `ioredis-mock@8.13.1` (peer `ioredis: ^5`) against a package depending on `ioredis@^6`, with the untyped import suppressed by `@ts-expect-error`. This commit changes no code and no dependency: it writes down the measurement that was missing, so the next reader starts where this one finished. Measured against ioredis 5.11.1 (newest release satisfying the mock's `^5` peer) and 6.0.0 (the version resolved in this workspace): - all 14 Redis commands this package issues carry every v5 overload verbatim into v6's RedisCommander.d.ts; `set` is a strict superset there (v6 adds IFEQ/IFNE/IFDEQ/IFDNE), and nothing used here was removed or re-shaped - v6's RESP3 reply mapping is opt-in: the class defaults its ReplyMapping parameter to "legacy", ChainableCommander defaults to "resp2", and duplicate() with no override inherits the caller's mapping. This package never passes replyMapping, so every reply shape it sees is the v5 one - the three RedisOptions keys client.ts sets (lazyConnect, maxRetriesPerRequest, enableAutoPipelining) are declared identically The peer mismatch is therefore real but inert on the surface these suites drive. The header now says so, names the version pair the claim rests on, and states that the claim expires if either range in package.json moves. Also recorded: these suites reach the mock only through the injected client and the pub/sub duplicate, never through createRedisClient(), so `new Redis(url, options)` — this package's only contact with ioredis's constructor and connection surface, and the area v6 changed most — is exercised by nothing in this file. That is why the gap is inert here, and it is not a reason to trust the double. The header additionally promised `RUN_REAL_REDIS=1` + `REDIS_URL` and "conditional describe.skipIf blocks at the bottom". No such blocks exist, and both names occurred nowhere in the package outside that one sentence. The false promise is removed rather than left standing as an escape hatch nobody can take. The `@ts-expect-error` stays. Removing it fails typecheck with TS7016 — ioredis-mock ships no `types` field and no .d.ts of its own — so the suppression has not rotted. Its comment now also records what it costs: with the module untyped the client is `any` and satisfies ioredis's Redis type without being checked against it, which is the second reason a v5-vs-v6 divergence could not surface here. `@types/ioredis-mock` would type the seam but only asserts `new(): ioredis.Redis` rather than describing the mock, so adopting it would trade an honest `any` for an unearned certainty; that trade is left to a maintainer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y * docs(service-cluster-redis): narrow three contract-test header sentences to the populations actually measured Review found three sentences in the header whose stated population was wider than the measurement behind them. All three are corrected here; the diff stays comments-only (proved: every added and removed line is a comment line, with a control showing the filter admits code). 1. The "no live-Redis path" parenthetical claimed both names appeared nowhere in the package. Re-scanned the WHOLE package rather than `src/`: `RUN_REAL_REDIS` and `skipIf` do occur nowhere outside that sentence, so the escape hatch genuinely never existed — but `REDIS_URL` occurs at `README.md:42`, as the env var a caller feeds to `createRedisClient()`. The sentence now says exactly that instead of over-reaching. 2. `multi` was listed among the commands carrying their v5 overloads verbatim into `RedisCommander.d.ts`. That is false for `multi` specifically: it is declared there in NEITHER version (measured 0 and 0, against a control member `exec` reading 1 and 1); it lives on `Transaction`. Measured on its real home, it DID change — all four overloads went from returning `ChainableCommander` to returning it parameterised by a reply mapping. It is removed from the verbatim list and given its own bullet stating why the change is inert here: the parameter defaults to "resp2", the class defaults to "legacy" and extends `Transaction` at "resp2", so a client built without `replyMapping` resolves `multi()` to the non-RESP3 instantiation. 3. "v6's one substantive change reachable from this package" was wrong in the other direction: v6 also changes connection defaults that are NOT opt-in — `protocol: 3` (no such option in v5) and `keepAlive` 0 to 30000. The sentence is scoped to "the surface these suites drive", which is what was measured, and the connection-default changes are named as changed-but-unexercised, since no suite here calls `createRedisClient()`. Two further wording fixes, each closing a way the note could read as true when it is not: - the expiry clause now also fires on a RESOLVED-version move under an unchanged caret range, which a lockfile bump alone will do; as written it expired only when a range moved - the unmet-peer warning is scoped to a resolving install, since a frozen re-link prints nothing Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2024eca commit 5b41477

1 file changed

Lines changed: 78 additions & 3 deletions

File tree

packages/services/service-cluster-redis/src/redis.contract.test.ts

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,87 @@
33
/**
44
* Driver contract tests for the Redis cluster driver, run against
55
* `ioredis-mock` so they execute without a real Redis instance in CI.
6+
* There is no live-Redis path in this file: every suite below runs on the
7+
* mock. (An earlier version of this header promised `RUN_REAL_REDIS=1` +
8+
* `REDIS_URL` and "conditional `describe.skipIf` blocks at the bottom" —
9+
* no such blocks were ever here. Scanning the whole package, not just
10+
* `src/`: `RUN_REAL_REDIS` and `skipIf` occur nowhere outside that
11+
* sentence, so the escape hatch never existed. `REDIS_URL` does occur, at
12+
* `README.md:42` — but as the env var a caller feeds to
13+
* `createRedisClient()`, which is unrelated to any test path.)
614
*
7-
* The same suites can be invoked against a live Redis by setting
8-
* `RUN_REAL_REDIS=1` and providing `REDIS_URL` — see the conditional
9-
* `describe.skipIf` blocks at the bottom.
15+
* ## The double is one major version behind the client it doubles
16+
*
17+
* This package depends on `ioredis@^6`, while `ioredis-mock@8.13.1`
18+
* declares `peerDependencies: { ioredis: "^5" }` — so a resolving
19+
* `pnpm install` prints an unmet-peer warning for it (a frozen re-link
20+
* prints nothing). That gap is real, and it is
21+
* declared here rather than closed, because it was measured to be inert
22+
* on the surface these suites actually drive. Measured against ioredis
23+
* 5.11.1 (newest release satisfying the mock's `^5` peer) and 6.0.0 (the
24+
* version resolved in this workspace):
25+
*
26+
* - Every Redis command issued by this package's `src/*.ts` — get, set,
27+
* del, incr, incrby, pttl, watch, unwatch, exec, publish, subscribe,
28+
* unsubscribe, quit, eval — carries all of its v5 overloads verbatim
29+
* into v6's `RedisCommander.d.ts`. `set` is a strict superset there
30+
* (v6 adds the IFEQ/IFNE/IFDEQ/IFDNE tokens); nothing used here was
31+
* removed or re-shaped.
32+
* - `multi()` is deliberately not in that list: it is declared on
33+
* `Transaction` (`transaction.d.ts`) and appears in
34+
* `RedisCommander.d.ts` in neither version. Measured separately, it
35+
* DID change — all four overloads went from returning
36+
* `ChainableCommander` to `ChainableCommander` parameterised by a
37+
* mapping. It is inert here because the parameter defaults to "resp2"
38+
* and a client built without `replyMapping` reaches it as such: the
39+
* class defaults to "legacy" and extends `Transaction` at "resp2".
40+
* So this package's `multi()` resolves to the non-RESP3 instantiation,
41+
* and `exec()`'s own declaration is byte-identical across the pair.
42+
* - v6's one substantive change reachable from the surface these suites
43+
* drive is RESP3 reply mapping, and it is opt-in: the class is
44+
* declared with a `ReplyMapping` parameter defaulting to "legacy",
45+
* `ChainableCommander` defaults to "resp2", and `duplicate()` with no
46+
* override inherits the caller's mapping. This package never passes
47+
* `replyMapping`, so every reply shape these suites see is the v5 one.
48+
* ⚠️ Scoped deliberately: v6 also changes connection defaults that are
49+
* NOT opt-in — `protocol: 3` (no such option in v5) and `keepAlive`
50+
* 0 -> 30000. Those are reached in production through
51+
* `createRedisClient()`, which no suite here calls (see below), so
52+
* they are changed-but-unexercised rather than absent.
53+
* - The three `RedisOptions` keys client.ts sets — lazyConnect,
54+
* maxRetriesPerRequest, enableAutoPipelining — are declared
55+
* identically in both versions.
56+
*
57+
* That named set is the whole basis for the claim; it is not a statement
58+
* about ioredis 5 vs 6 in general. If the `ioredis` or `ioredis-mock`
59+
* range in package.json moves — OR if the version either one RESOLVES to
60+
* moves under an unchanged caret range, which a lockfile bump alone will
61+
* do — this paragraph expires and the diff has to be re-taken.
62+
*
63+
* ## What these suites therefore do NOT certify
64+
*
65+
* They reach the mock only through the injected `client` and the
66+
* `client.duplicate()` the pub/sub adapter makes. They never call
67+
* `createRedisClient()`, so `new Redis(url, options)` — this package's
68+
* only contact with ioredis's constructor and connection surface, and the
69+
* area v6 changed most — is exercised by nothing in this file. That is
70+
* why the version gap is inert here, and it is not a reason to trust the
71+
* double: the mock is simply never asked to stand in for the surface on
72+
* which the two majors differ.
1073
*/
1174

75+
// `ioredis-mock` publishes no type declarations of its own — no `types`
76+
// field in its manifest and no `.d.ts` in the tarball — so this import
77+
// raises TS7016 ("could not find a declaration file ... implicitly has an
78+
// 'any' type") and the directive below is what silences it. Note the
79+
// second cost, beyond the missing types: with the module untyped
80+
// `RedisMock` is `any`, so every `client:` argument constructed from it
81+
// satisfies ioredis's `Redis` type without ever being checked against it.
82+
// That is the other reason a v5-vs-v6 divergence could not surface here.
83+
// `@types/ioredis-mock` exists and would type this seam, but it only
84+
// *asserts* `new(): ioredis.Redis` rather than describing the mock, so
85+
// adopting it would trade an honest `any` for an unearned certainty —
86+
// that trade has not been made, deliberately.
1287
// @ts-expect-error — ioredis-mock has no published types
1388
import RedisMock from 'ioredis-mock';
1489
import { describe, expect, it, vi } from 'vitest';

0 commit comments

Comments
 (0)