Skip to content

Commit a2bbec4

Browse files
committed
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
1 parent 14e70d8 commit a2bbec4

1 file changed

Lines changed: 37 additions & 17 deletions

File tree

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

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,39 +6,59 @@
66
* There is no live-Redis path in this file: every suite below runs on the
77
* mock. (An earlier version of this header promised `RUN_REAL_REDIS=1` +
88
* `REDIS_URL` and "conditional `describe.skipIf` blocks at the bottom" —
9-
* no such blocks were ever here, and the two names appeared nowhere in
10-
* this package outside that sentence.)
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.)
1114
*
1215
* ## The double is one major version behind the client it doubles
1316
*
1417
* This package depends on `ioredis@^6`, while `ioredis-mock@8.13.1`
15-
* declares `peerDependencies: { ioredis: "^5" }` — so `pnpm install`
16-
* prints an unmet-peer warning for it. That gap is real, and it is
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
1721
* declared here rather than closed, because it was measured to be inert
1822
* on the surface these suites actually drive. Measured against ioredis
1923
* 5.11.1 (newest release satisfying the mock's `^5` peer) and 6.0.0 (the
2024
* version resolved in this workspace):
2125
*
2226
* - Every Redis command issued by this package's `src/*.ts` — get, set,
23-
* del, incr, incrby, pttl, watch, unwatch, multi, exec, publish,
24-
* subscribe, unsubscribe, quit, eval — carries all of its v5
25-
* overloads verbatim into v6's `RedisCommander.d.ts`. `set` is a
26-
* strict superset there (v6 adds the IFEQ/IFNE/IFDEQ/IFDNE tokens);
27-
* nothing used here was removed or re-shaped.
28-
* - v6's one substantive change reachable from this package is RESP3
29-
* reply mapping, and it is opt-in: the class is declared with a
30-
* `ReplyMapping` parameter defaulting to "legacy", `ChainableCommander`
31-
* defaults to "resp2", and `duplicate()` with no override inherits the
32-
* caller's mapping. This package never passes `replyMapping`, so every
33-
* reply shape it sees is still the v5 one.
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.
3453
* - The three `RedisOptions` keys client.ts sets — lazyConnect,
3554
* maxRetriesPerRequest, enableAutoPipelining — are declared
3655
* identically in both versions.
3756
*
3857
* That named set is the whole basis for the claim; it is not a statement
3958
* about ioredis 5 vs 6 in general. If the `ioredis` or `ioredis-mock`
40-
* range in package.json moves, this paragraph expires and the diff has to
41-
* be re-taken.
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.
4262
*
4363
* ## What these suites therefore do NOT certify
4464
*

0 commit comments

Comments
 (0)