From 91ca87f55307e92127fc77e2d98128d8579b780a Mon Sep 17 00:00:00 2001 From: Marcin Kotlicki Date: Wed, 12 Aug 2026 14:21:27 +0200 Subject: [PATCH 01/11] Add isolated e2e suite proving registerInitialCache populates Redis Adds a self-contained Playwright suite (examples/redis-minimal/e2e/isolated) that boots its own ephemeral Redis container and next start process to verify registerInitialCache actually writes build-time artifacts to Redis on boot, plus the setOnlyIfNotExists option's behavior - previously untested anywhere in the repo since the shared e2e suite only observes an already-running, already-mutated server/Redis. --- .claude/skills/new-e2e-test/SKILL.md | 20 ++ .github/workflows/integration-tests.yml | 3 + .gitignore | 2 + CLAUDE.md | 1 + examples/redis-minimal/README.md | 18 ++ .../e2e/isolated/helpers/isolated-server.ts | 188 ++++++++++++++++++ .../isolated/initial-cache-population.spec.ts | 101 ++++++++++ ...itial-cache-set-only-if-not-exists.spec.ts | 88 ++++++++ examples/redis-minimal/package.json | 3 +- examples/redis-minimal/playwright.config.ts | 4 + .../playwright.isolated.config.ts | 16 ++ turbo.json | 4 + 12 files changed, 447 insertions(+), 1 deletion(-) create mode 100644 examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts create mode 100644 examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts create mode 100644 examples/redis-minimal/e2e/isolated/initial-cache-set-only-if-not-exists.spec.ts create mode 100644 examples/redis-minimal/playwright.isolated.config.ts diff --git a/.claude/skills/new-e2e-test/SKILL.md b/.claude/skills/new-e2e-test/SKILL.md index 5ca7256..249adbf 100644 --- a/.claude/skills/new-e2e-test/SKILL.md +++ b/.claude/skills/new-e2e-test/SKILL.md @@ -108,3 +108,23 @@ cd examples/redis-minimal && npx playwright test .spec.ts # single file, Requires a reachable Redis at `REDIS_URL` (defaults to `redis://127.0.0.1:6379`) — start one locally (e.g. `docker run -p 6379:6379 redis`) before running e2e tests outside CI. + +## Isolated tests (own Redis + own server) + +If a test needs to assert against Redis state independent of the shared `next start`/Redis used +by the rest of this suite — e.g. proving something that happens exactly once at server boot, +before any HTTP request, such as `registerInitialCache` populating Redis from build-time +artifacts — it does **not** belong in `e2e/*.spec.ts` under the shared `playwright.config.ts`. By +the time any of those specs run, the shared server has been up for a while and its Redis has +already been mutated by earlier HTTP-driven cache writes, so "before any request" can't be proven +there. + +See `e2e/isolated/` and `playwright.isolated.config.ts` instead: specs there self-manage a Docker +Redis container and a `next start` child process via `e2e/isolated/helpers/isolated-server.ts` +(`startEphemeralRedis`/`waitForRedisReady`/`waitForRedisPopulated`/`startIsolatedNextServer`/ +`stopIsolatedNextServer`), run via `pnpm test:e2e:isolated` (requires Docker). Key lesson from +building this suite: don't synchronize on "the shared-tags hash has *any* entries" — routes are +written under `registerInitialCache`'s own internal concurrency limit, so an early partial write +can pass an `HLEN > 0` check while routes your test cares about are still being written. Wait for +the *specific* cache paths the test asserts on (`waitForRedisPopulated`'s `expectedCachePaths` +argument) rather than a generic non-empty check. diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 8d99453..c73d9ed 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -40,3 +40,6 @@ jobs: run: pnpm exec playwright install chromium --with-deps - name: Run e2e tests run: pnpm test:e2e + - name: Run isolated registerInitialCache e2e tests + working-directory: examples/redis-minimal + run: pnpm test:e2e:isolated diff --git a/.gitignore b/.gitignore index 2ea12d6..d649e19 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,5 @@ dist *.swp *.swo + +examples/redis-minimal/test-results/ diff --git a/CLAUDE.md b/CLAUDE.md index d22e342..b66dc6d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,6 +75,7 @@ Each handler is a factory function returning an object matching the `Handler` in - Unit tests are colocated with source as `*.test.ts` under `packages/nextjs-cache-handler/src/` (jest + ts-jest, `testEnvironment: "node"`). - E2E tests live in `examples/redis-minimal/e2e` (Playwright) and exercise real caching behavior (fetch tag revalidation, ISR, `unstable_cache`, static params, etc.) against a built Next.js app backed by Redis — run via `pnpm test:e2e` from the root, which builds first (`test:e2e` depends on `^build`/`build` per `turbo.json`). +- A separate isolated suite under `examples/redis-minimal/e2e/isolated` (its own `playwright.isolated.config.ts`, run via `pnpm test:e2e:isolated`, **requires Docker**) proves `registerInitialCache` populates Redis from build-time artifacts on server boot. Each spec self-manages its own ephemeral Redis container and `next start` process — independent of the shared suite's server/Redis — so it can assert Redis state before any HTTP request is made. - Use the `new-example` skill (`.claude/skills/new-example/SKILL.md`) to scaffold a new caching example page in `examples/redis-minimal`, and the `new-e2e-test` skill (`.claude/skills/new-e2e-test/SKILL.md`) to add its Playwright coverage. These are commonly used together — a new example is only complete once it has e2e coverage proving the cache behavior it demonstrates, and writing that coverage needs page-specific facts (path, tags, `cacheLife` profile, expected reload behavior) that only whoever built the example page knows. ## Legacy project docs diff --git a/examples/redis-minimal/README.md b/examples/redis-minimal/README.md index 312290b..96fc3e3 100644 --- a/examples/redis-minimal/README.md +++ b/examples/redis-minimal/README.md @@ -266,6 +266,24 @@ REDIS_URL=redis://localhost:6379 pnpm test:e2e From repo root, Turborepo runs `build` first (cached when unchanged) then Playwright e2e. Playwright starts `next start` on port 3000 (`reuseExistingServer` when not in CI). +### Isolated `registerInitialCache` e2e tests + +`e2e/isolated/` is a separate Playwright suite (its own config, `playwright.isolated.config.ts`) +that proves `registerInitialCache` actually populates Redis from build-time artifacts on server +startup. It doesn't use the shared `webServer`/Redis above — each spec spins up its own ephemeral +Redis container (via the `docker` CLI) and its own `next start` process on dedicated ports, so it +can assert Redis state that can only be explained by `registerInitialCache` having run, before any +HTTP request is made. + +**Requires Docker** to be running locally (the tests spawn `docker run redis:7-alpine` +themselves - no need to start a container manually). Requires a prior `pnpm build` (reuses the +same `.next` output as the main suite). + +```bash +pnpm build +pnpm test:e2e:isolated +``` + ## Technologies - Next.js 16 diff --git a/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts b/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts new file mode 100644 index 0000000..54dfdd4 --- /dev/null +++ b/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts @@ -0,0 +1,188 @@ +import { spawn, exec as execCallback, type ChildProcess } from "node:child_process"; +import { promisify } from "node:util"; +import path from "node:path"; +import { createClient } from "redis"; + +const exec = promisify(execCallback); + +export type IsolatedRedis = { + containerName: string; + port: number; + url: string; +}; + +export type IsolatedNextServer = { + process: ChildProcess; + port: number; + baseURL: string; + getOutput: () => { stdout: string; stderr: string }; +}; + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +function randomSuffix(): string { + return Math.random().toString(36).slice(2, 10); +} + +export async function startEphemeralRedis( + opts: { port?: number; image?: string } = {}, +): Promise { + const port = opts.port ?? 16380; + const image = opts.image ?? "redis:7-alpine"; + const containerName = `nca-e2e-redis-${randomSuffix()}`; + + await exec(`docker run -d --rm --name ${containerName} -p ${port}:6379 ${image}`); + + return { containerName, port, url: `redis://127.0.0.1:${port}` }; +} + +export async function stopEphemeralRedis(redis: IsolatedRedis): Promise { + try { + await exec(`docker rm -f ${redis.containerName}`); + } catch { + // container may already be gone (e.g. --rm already cleaned it up on stop) - nothing to do + } +} + +export async function waitForRedisReady( + url: string, + opts: { timeoutMs?: number; intervalMs?: number } = {}, +): Promise { + const timeoutMs = opts.timeoutMs ?? 30_000; + const intervalMs = opts.intervalMs ?? 250; + const deadline = Date.now() + timeoutMs; + + let lastError: unknown; + while (Date.now() < deadline) { + const client = createClient({ url }); + try { + await client.connect(); + await client.ping(); + await client.quit(); + return; + } catch (error) { + lastError = error; + await client.quit().catch(() => {}); + await sleep(intervalMs); + } + } + + throw new Error(`Redis at ${url} was not ready after ${timeoutMs}ms: ${String(lastError)}`); +} + +/** + * Polls Redis directly for the signal that `registerInitialCache` has finished + * writing a specific, known set of routes, rather than relying on HTTP readiness + * (which says nothing about `register()` having completed) or on the shared-tags + * hash simply being non-empty (which can be true while other routes, processed + * concurrently under `registerInitialCache`'s own parallelism limit, are still + * being written - `HLEN > 0` alone is not a reliable "population finished" signal). + */ +export async function waitForRedisPopulated( + url: string, + expectedCachePaths: string[], + opts: { timeoutMs?: number; intervalMs?: number; hashKey?: string } = {}, +): Promise { + const timeoutMs = opts.timeoutMs ?? 30_000; + const intervalMs = opts.intervalMs ?? 250; + const hashKey = opts.hashKey ?? "nextjs:__sharedTags__"; + const deadline = Date.now() + timeoutMs; + + const client = createClient({ url }); + await client.connect(); + try { + while (Date.now() < deadline) { + const results = await Promise.all( + expectedCachePaths.map((cachePath) => client.hExists(hashKey, cachePath)), + ); + if (results.every(Boolean)) return; + await sleep(intervalMs); + } + throw new Error( + `Redis hash "${hashKey}" did not contain all expected cache paths (${expectedCachePaths.join(", ")}) after ${timeoutMs}ms`, + ); + } finally { + await client.quit(); + } +} + +export async function startIsolatedNextServer(opts: { + port?: number; + cwd?: string; + env: Record; +}): Promise { + const port = opts.port ?? 3010; + const cwd = opts.cwd ?? path.resolve(__dirname, "..", "..", ".."); + + const child = spawn(`pnpm exec next start --port ${port}`, [], { + cwd, + env: { ...process.env, ...opts.env }, + shell: true, + detached: process.platform !== "win32", + stdio: ["ignore", "pipe", "pipe"], + }); + + let stdout = ""; + let stderr = ""; + child.stdout?.on("data", (chunk) => { + stdout += chunk.toString(); + }); + child.stderr?.on("data", (chunk) => { + stderr += chunk.toString(); + }); + + return { + process: child, + port, + baseURL: `http://127.0.0.1:${port}`, + getOutput: () => ({ stdout, stderr }), + }; +} + +export async function waitForHttpReady( + baseURL: string, + opts: { timeoutMs?: number; intervalMs?: number } = {}, +): Promise { + const timeoutMs = opts.timeoutMs ?? 30_000; + const intervalMs = opts.intervalMs ?? 250; + const deadline = Date.now() + timeoutMs; + + let lastError: unknown; + while (Date.now() < deadline) { + try { + const response = await fetch(baseURL); + if (response.status < 500) return; + } catch (error) { + lastError = error; + } + await sleep(intervalMs); + } + + throw new Error(`Server at ${baseURL} was not ready after ${timeoutMs}ms: ${String(lastError)}`); +} + +export async function stopIsolatedNextServer(server: IsolatedNextServer): Promise { + const { process: child } = server; + if (child.exitCode !== null || child.pid === undefined) return; + + if (process.platform === "win32") { + await exec(`taskkill /pid ${child.pid} /T /F`).catch(() => {}); + } else { + try { + process.kill(-child.pid, "SIGTERM"); + } catch { + child.kill("SIGTERM"); + } + } + + await new Promise((resolve) => { + if (child.exitCode !== null) { + resolve(); + return; + } + child.once("exit", () => resolve()); + setTimeout(resolve, 5000); + }); +} diff --git a/examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts b/examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts new file mode 100644 index 0000000..5293464 --- /dev/null +++ b/examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts @@ -0,0 +1,101 @@ +import { test, expect } from "@playwright/test"; +import { createClient } from "redis"; +import { + startEphemeralRedis, + stopEphemeralRedis, + waitForRedisReady, + waitForRedisPopulated, + startIsolatedNextServer, + stopIsolatedNextServer, + type IsolatedRedis, + type IsolatedNextServer, +} from "./helpers/isolated-server"; + +// Routes whose presence in Redis can only be explained by `registerInitialCache` +// having read build-time artifacts off disk - no HTTP request is made anywhere +// in this file, so these keys cannot have been written by runtime cache writes. +const EXPECTED_CACHE_PATHS = [ + "/examples/default-cache", + "/examples/fetch-tags", + "/posts/1", +]; + +test.describe("registerInitialCache populates Redis on boot (setOnlyIfNotExists=false, default)", () => { + let redis: IsolatedRedis; + let server: IsolatedNextServer; + + test.beforeAll(async () => { + redis = await startEphemeralRedis(); + await waitForRedisReady(redis.url); + + server = await startIsolatedNextServer({ + env: { REDIS_URL: redis.url, REDIS_TYPE: "redis" }, + }); + + await waitForRedisPopulated(redis.url, EXPECTED_CACHE_PATHS, { + timeoutMs: 60_000, + }); + }); + + test.afterAll(async () => { + await stopIsolatedNextServer(server); + await stopEphemeralRedis(redis); + }); + + test("shared tags hash is populated before any HTTP request is made", async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + const sharedTagsLen = await client.hLen("nextjs:__sharedTags__"); + expect(sharedTagsLen).toBeGreaterThan(0); + } finally { + await client.quit(); + } + }); + + test("a static App Router page is populated from its pre-rendered artifacts", async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + const value = await client.get("nextjs:/examples/default-cache"); + expect(value).not.toBeNull(); + } finally { + await client.quit(); + } + }); + + test("a fetch-cache entry's tags are populated from disk (proves fetch-cache population specifically)", async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + const pageValue = await client.get("nextjs:/examples/fetch-tags"); + expect(pageValue).not.toBeNull(); + + const rawTags = await client.hGet( + "nextjs:__sharedTags__", + "/examples/fetch-tags", + ); + expect(rawTags).not.toBeNull(); + const tags = JSON.parse(rawTags ?? "[]"); + expect(tags).toContain("futurama"); + } finally { + await client.quit(); + } + }); + + test("a Pages Router (fallback: false) page gets its implicit path tag from disk", async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + const value = await client.get("nextjs:/posts/1"); + expect(value).not.toBeNull(); + + const rawTags = await client.hGet("nextjs:__sharedTags__", "/posts/1"); + expect(rawTags).not.toBeNull(); + const tags = JSON.parse(rawTags ?? "[]"); + expect(tags).toContain("_N_T_/posts/1"); + } finally { + await client.quit(); + } + }); +}); diff --git a/examples/redis-minimal/e2e/isolated/initial-cache-set-only-if-not-exists.spec.ts b/examples/redis-minimal/e2e/isolated/initial-cache-set-only-if-not-exists.spec.ts new file mode 100644 index 0000000..7cd2bbe --- /dev/null +++ b/examples/redis-minimal/e2e/isolated/initial-cache-set-only-if-not-exists.spec.ts @@ -0,0 +1,88 @@ +import { test, expect } from "@playwright/test"; +import { createClient } from "redis"; +import { + startEphemeralRedis, + stopEphemeralRedis, + waitForRedisReady, + waitForRedisPopulated, + startIsolatedNextServer, + stopIsolatedNextServer, + type IsolatedRedis, + type IsolatedNextServer, +} from "./helpers/isolated-server"; + +const SEEDED_CACHE_PATH = "nextjs:/examples/default-cache"; +const SENTINEL = JSON.stringify({ + lastModified: 1, + lifespan: null, + tags: [], + value: { kind: "SENTINEL", __test: true }, +}); + +test.describe("registerInitialCache with setOnlyIfNotExists=true does not clobber existing keys", () => { + let redis: IsolatedRedis; + let server: IsolatedNextServer; + + test.beforeAll(async () => { + redis = await startEphemeralRedis(); + await waitForRedisReady(redis.url); + + // Pre-seed BEFORE starting next start, so registerInitialCache sees an existing key. + const seedClient = createClient({ url: redis.url }); + await seedClient.connect(); + await seedClient.set(SEEDED_CACHE_PATH, SENTINEL); + await seedClient.quit(); + + server = await startIsolatedNextServer({ + env: { + REDIS_URL: redis.url, + REDIS_TYPE: "redis", + INITIAL_CACHE_SET_ONLY_IF_NOT_EXISTS: "true", + }, + }); + + await waitForRedisPopulated( + redis.url, + ["/examples/default-cache", "/examples/fetch-tags"], + { timeoutMs: 60_000 }, + ); + }); + + test.afterAll(async () => { + await stopIsolatedNextServer(server); + await stopEphemeralRedis(redis); + }); + + test("pre-existing value key is not overwritten", async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + const value = await client.get(SEEDED_CACHE_PATH); + expect(value).toBe(SENTINEL); + } finally { + await client.quit(); + } + }); + + test("shared tags hash is still written (unconditional, regardless of setOnlyIfNotExists)", async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + const sharedTagsLen = await client.hLen("nextjs:__sharedTags__"); + expect(sharedTagsLen).toBeGreaterThan(0); + } finally { + await client.quit(); + } + }); + + test("a route that was not pre-seeded is still written fresh", async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + const value = await client.get("nextjs:/examples/fetch-tags"); + expect(value).not.toBeNull(); + } finally { + await client.quit(); + } + }); +}); diff --git a/examples/redis-minimal/package.json b/examples/redis-minimal/package.json index fadcde9..7716908 100644 --- a/examples/redis-minimal/package.json +++ b/examples/redis-minimal/package.json @@ -11,7 +11,8 @@ "build": "next build", "start": "next start", "lint": "eslint .", - "test:e2e": "playwright test" + "test:e2e": "playwright test", + "test:e2e:isolated": "playwright test --config=playwright.isolated.config.ts" }, "dependencies": { "@fortedigital/nextjs-cache-handler": "workspace:*", diff --git a/examples/redis-minimal/playwright.config.ts b/examples/redis-minimal/playwright.config.ts index 7472e1b..7c1f4ac 100644 --- a/examples/redis-minimal/playwright.config.ts +++ b/examples/redis-minimal/playwright.config.ts @@ -4,6 +4,10 @@ const baseURL = process.env.BASE_URL ?? "http://127.0.0.1:3000"; export default defineConfig({ testDir: "./e2e", + // The isolated suite (./e2e/isolated) manages its own ephemeral Redis + // container and `next start` process per spec - it must not run under this + // config's shared webServer/Redis. See playwright.isolated.config.ts. + testIgnore: ["isolated/**"], fullyParallel: false, workers: 1, retries: process.env.CI ? 2 : 0, diff --git a/examples/redis-minimal/playwright.isolated.config.ts b/examples/redis-minimal/playwright.isolated.config.ts new file mode 100644 index 0000000..24b28bc --- /dev/null +++ b/examples/redis-minimal/playwright.isolated.config.ts @@ -0,0 +1,16 @@ +import { defineConfig, devices } from "@playwright/test"; + +export default defineConfig({ + testDir: "./e2e/isolated", + fullyParallel: false, + workers: 1, + retries: process.env.CI ? 1 : 0, + reporter: process.env.CI ? "github" : "list", + // Container start + `next start` boot needs more headroom than the default. + timeout: 60_000, + projects: [{ name: "chromium", use: { ...devices["Desktop Chrome"] } }], + // Deliberately no `webServer` block: each spec here manages its own ephemeral + // Redis container and `next start` process via ./e2e/isolated/helpers/isolated-server.ts, + // so it can assert Redis state that can only be explained by registerInitialCache, + // independent of the shared suite's server/Redis (see playwright.config.ts). +}); diff --git a/turbo.json b/turbo.json index ca5a135..270417a 100644 --- a/turbo.json +++ b/turbo.json @@ -20,6 +20,10 @@ "test:e2e": { "dependsOn": ["^build", "build"], "cache": false + }, + "test:e2e:isolated": { + "dependsOn": ["^build", "build"], + "cache": false } } } From b173a6c6d9ce5c9c615100de8a5279eaac0354c4 Mon Sep 17 00:00:00 2001 From: Marcin Kotlicki Date: Wed, 12 Aug 2026 14:46:17 +0200 Subject: [PATCH 02/11] Parallelize --- .github/workflows/integration-tests.yml | 34 +++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index c73d9ed..8492142 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -35,11 +35,45 @@ jobs: ${{ runner.os }}-turbo-integration-${{ hashFiles('pnpm-lock.yaml') }}- ${{ runner.os }}-turbo-integration- - run: pnpm install --frozen-lockfile + - name: Cache Playwright browsers + uses: actions/cache@v4 + with: + path: ~/.cache/ms-playwright + key: ${{ runner.os }}-playwright-${{ hashFiles('pnpm-lock.yaml') }} - name: Install Playwright browsers working-directory: examples/redis-minimal run: pnpm exec playwright install chromium --with-deps - name: Run e2e tests run: pnpm test:e2e + + redis-minimal-e2e-isolated: + runs-on: ubuntu-latest + env: + CI: "true" + steps: + - uses: actions/checkout@v4 + - uses: pnpm/action-setup@v4 + - uses: actions/setup-node@v4 + with: + node-version: 22.x + cache: pnpm + - name: Cache Turborepo + uses: actions/cache@v4 + with: + path: .turbo + key: ${{ runner.os }}-turbo-integration-${{ hashFiles('pnpm-lock.yaml') }}-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-turbo-integration-${{ hashFiles('pnpm-lock.yaml') }}- + ${{ runner.os }}-turbo-integration- + - run: pnpm install --frozen-lockfile + - name: Cache Playwright browsers + uses: actions/cache@v4 + with: + path: ~/.cache/ms-playwright + key: ${{ runner.os }}-playwright-${{ hashFiles('pnpm-lock.yaml') }} + - name: Install Playwright browsers + working-directory: examples/redis-minimal + run: pnpm exec playwright install chromium --with-deps - name: Run isolated registerInitialCache e2e tests working-directory: examples/redis-minimal run: pnpm test:e2e:isolated From 894d0eb2628d51ae205a66c933cdd6b4a7b2cb7d Mon Sep 17 00:00:00 2001 From: Marcin Kotlicki Date: Wed, 12 Aug 2026 14:57:09 +0200 Subject: [PATCH 03/11] CI Fix attempt --- .../e2e/isolated/helpers/isolated-server.ts | 91 +++++++++++++++---- .../playwright.isolated.config.ts | 6 +- 2 files changed, 75 insertions(+), 22 deletions(-) diff --git a/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts b/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts index 54dfdd4..1efb7b9 100644 --- a/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts +++ b/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts @@ -26,6 +26,44 @@ function randomSuffix(): string { return Math.random().toString(36).slice(2, 10); } +const DEFAULT_CONNECT_ATTEMPT_TIMEOUT_MS = 3_000; + +/** + * Connects with a hard bound on the attempt, so a single hung TCP handshake + * can't swallow an entire retry budget. Needed because right after + * `docker run -d -p`, the port's iptables/docker-proxy forwarding rule on + * Linux can take a moment to fully wire up - a connection attempt in that + * window can hang (no RST, no immediate refusal) rather than fail fast, + * which on CI (unlike Docker Desktop on Windows/macOS) has been observed to + * outlast a naive retry loop's per-attempt `client.connect()` entirely. + */ +async function connectWithTimeout( + url: string, + attemptTimeoutMs = DEFAULT_CONNECT_ATTEMPT_TIMEOUT_MS, +) { + const client = createClient({ url, socket: { connectTimeout: attemptTimeoutMs } }); + let timer: NodeJS.Timeout; + const timeout = new Promise((_, reject) => { + timer = setTimeout( + () => reject(new Error(`connect() did not settle within ${attemptTimeoutMs}ms`)), + attemptTimeoutMs + 1_000, + ); + }); + try { + await Promise.race([client.connect(), timeout]); + return client; + } catch (error) { + try { + client.destroy(); + } catch { + // already torn down - nothing to do + } + throw error; + } finally { + clearTimeout(timer!); + } +} + export async function startEphemeralRedis( opts: { port?: number; image?: string } = {}, ): Promise { @@ -38,7 +76,8 @@ export async function startEphemeralRedis( return { containerName, port, url: `redis://127.0.0.1:${port}` }; } -export async function stopEphemeralRedis(redis: IsolatedRedis): Promise { +export async function stopEphemeralRedis(redis: IsolatedRedis | undefined): Promise { + if (!redis) return; try { await exec(`docker rm -f ${redis.containerName}`); } catch { @@ -56,15 +95,16 @@ export async function waitForRedisReady( let lastError: unknown; while (Date.now() < deadline) { - const client = createClient({ url }); try { - await client.connect(); - await client.ping(); - await client.quit(); + const client = await connectWithTimeout(url); + try { + await client.ping(); + } finally { + await client.quit().catch(() => {}); + } return; } catch (error) { lastError = error; - await client.quit().catch(() => {}); await sleep(intervalMs); } } @@ -90,22 +130,30 @@ export async function waitForRedisPopulated( const hashKey = opts.hashKey ?? "nextjs:__sharedTags__"; const deadline = Date.now() + timeoutMs; - const client = createClient({ url }); - await client.connect(); - try { - while (Date.now() < deadline) { - const results = await Promise.all( - expectedCachePaths.map((cachePath) => client.hExists(hashKey, cachePath)), - ); - if (results.every(Boolean)) return; + let lastError: unknown; + while (Date.now() < deadline) { + try { + const client = await connectWithTimeout(url); + try { + while (Date.now() < deadline) { + const results = await Promise.all( + expectedCachePaths.map((cachePath) => client.hExists(hashKey, cachePath)), + ); + if (results.every(Boolean)) return; + await sleep(intervalMs); + } + } finally { + await client.quit().catch(() => {}); + } + } catch (error) { + lastError = error; await sleep(intervalMs); } - throw new Error( - `Redis hash "${hashKey}" did not contain all expected cache paths (${expectedCachePaths.join(", ")}) after ${timeoutMs}ms`, - ); - } finally { - await client.quit(); } + + throw new Error( + `Redis hash "${hashKey}" did not contain all expected cache paths (${expectedCachePaths.join(", ")}) after ${timeoutMs}ms: ${String(lastError)}`, + ); } export async function startIsolatedNextServer(opts: { @@ -163,7 +211,10 @@ export async function waitForHttpReady( throw new Error(`Server at ${baseURL} was not ready after ${timeoutMs}ms: ${String(lastError)}`); } -export async function stopIsolatedNextServer(server: IsolatedNextServer): Promise { +export async function stopIsolatedNextServer( + server: IsolatedNextServer | undefined, +): Promise { + if (!server) return; const { process: child } = server; if (child.exitCode !== null || child.pid === undefined) return; diff --git a/examples/redis-minimal/playwright.isolated.config.ts b/examples/redis-minimal/playwright.isolated.config.ts index 24b28bc..de336fd 100644 --- a/examples/redis-minimal/playwright.isolated.config.ts +++ b/examples/redis-minimal/playwright.isolated.config.ts @@ -6,8 +6,10 @@ export default defineConfig({ workers: 1, retries: process.env.CI ? 1 : 0, reporter: process.env.CI ? "github" : "list", - // Container start + `next start` boot needs more headroom than the default. - timeout: 60_000, + // Container start + `next start` boot needs more headroom than the default, + // plus slack for a cold `docker pull` on a fresh CI runner where the image + // isn't cached yet. + timeout: 90_000, projects: [{ name: "chromium", use: { ...devices["Desktop Chrome"] } }], // Deliberately no `webServer` block: each spec here manages its own ephemeral // Redis container and `next start` process via ./e2e/isolated/helpers/isolated-server.ts, From 7cb689ab7cbc54d6144037bd06848788d3e4e8f6 Mon Sep 17 00:00:00 2001 From: Marcin Kotlicki Date: Wed, 12 Aug 2026 15:03:59 +0200 Subject: [PATCH 04/11] Fix 2 --- .github/workflows/integration-tests.yml | 1 - package.json | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 8492142..35632d0 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -75,5 +75,4 @@ jobs: working-directory: examples/redis-minimal run: pnpm exec playwright install chromium --with-deps - name: Run isolated registerInitialCache e2e tests - working-directory: examples/redis-minimal run: pnpm test:e2e:isolated diff --git a/package.json b/package.json index e82cdab..54f4033 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,7 @@ "lint": "turbo lint", "test": "turbo test", "test:e2e": "turbo run test:e2e --filter=redis-minimal", + "test:e2e:isolated": "turbo run test:e2e:isolated --filter=redis-minimal", "format": "turbo format", "clean": "turbo clean && rm -rf node_modules" }, From 906657f9426021f57cdf84136e2144c822b2fed5 Mon Sep 17 00:00:00 2001 From: Marcin Kotlicki Date: Wed, 12 Aug 2026 15:11:58 +0200 Subject: [PATCH 05/11] Reporters --- .github/workflows/integration-tests.yml | 18 +++++++ .github/workflows/node.js.yml | 13 ++++- .gitignore | 1 + examples/redis-minimal/playwright.config.ts | 4 +- .../playwright.isolated.config.ts | 4 +- packages/nextjs-cache-handler/jest.config.js | 10 ++++ packages/nextjs-cache-handler/package.json | 1 + pnpm-lock.yaml | 50 +++++++++++++++---- 8 files changed, 89 insertions(+), 12 deletions(-) diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 35632d0..43d5c92 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -7,6 +7,10 @@ on: branches: [master] workflow_dispatch: +permissions: + contents: read + checks: write + jobs: redis-minimal-e2e: runs-on: ubuntu-latest @@ -45,6 +49,13 @@ jobs: run: pnpm exec playwright install chromium --with-deps - name: Run e2e tests run: pnpm test:e2e + - name: E2E Test Report + uses: dorny/test-reporter@v1 + if: always() + with: + name: E2E Test Report + path: examples/redis-minimal/test-results/junit-e2e.xml + reporter: java-junit redis-minimal-e2e-isolated: runs-on: ubuntu-latest @@ -76,3 +87,10 @@ jobs: run: pnpm exec playwright install chromium --with-deps - name: Run isolated registerInitialCache e2e tests run: pnpm test:e2e:isolated + - name: Isolated E2E Test Report + uses: dorny/test-reporter@v1 + if: always() + with: + name: Isolated E2E Test Report + path: examples/redis-minimal/test-results/junit-e2e-isolated.xml + reporter: java-junit diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml index c704b5a..7409387 100644 --- a/.github/workflows/node.js.yml +++ b/.github/workflows/node.js.yml @@ -7,13 +7,17 @@ on: branches: [ "master" ] workflow_dispatch: +permissions: + contents: read + checks: write + jobs: build: runs-on: ubuntu-latest defaults: run: working-directory: ./packages/nextjs-cache-handler - + steps: - uses: actions/checkout@v4 - uses: pnpm/action-setup@v4 @@ -25,3 +29,10 @@ jobs: - run: pnpm install --frozen-lockfile - run: npm run build --if-present - run: npm test + - name: Unit Test Report + uses: dorny/test-reporter@v1 + if: always() + with: + name: Unit Test Report + path: packages/nextjs-cache-handler/test-results/junit.xml + reporter: jest-junit diff --git a/.gitignore b/.gitignore index d649e19..8a2ef12 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,4 @@ dist examples/redis-minimal/test-results/ +packages/nextjs-cache-handler/test-results/ diff --git a/examples/redis-minimal/playwright.config.ts b/examples/redis-minimal/playwright.config.ts index 7c1f4ac..56db2de 100644 --- a/examples/redis-minimal/playwright.config.ts +++ b/examples/redis-minimal/playwright.config.ts @@ -11,7 +11,9 @@ export default defineConfig({ fullyParallel: false, workers: 1, retries: process.env.CI ? 2 : 0, - reporter: process.env.CI ? "github" : "list", + reporter: process.env.CI + ? [["github"], ["junit", { outputFile: "test-results/junit-e2e.xml" }]] + : "list", use: { baseURL, trace: "on-first-retry", diff --git a/examples/redis-minimal/playwright.isolated.config.ts b/examples/redis-minimal/playwright.isolated.config.ts index de336fd..fb5044c 100644 --- a/examples/redis-minimal/playwright.isolated.config.ts +++ b/examples/redis-minimal/playwright.isolated.config.ts @@ -5,7 +5,9 @@ export default defineConfig({ fullyParallel: false, workers: 1, retries: process.env.CI ? 1 : 0, - reporter: process.env.CI ? "github" : "list", + reporter: process.env.CI + ? [["github"], ["junit", { outputFile: "test-results/junit-e2e-isolated.xml" }]] + : "list", // Container start + `next start` boot needs more headroom than the default, // plus slack for a cold `docker pull` on a fresh CI runner where the image // isn't cached yet. diff --git a/packages/nextjs-cache-handler/jest.config.js b/packages/nextjs-cache-handler/jest.config.js index 5ea35fb..39d0fd3 100644 --- a/packages/nextjs-cache-handler/jest.config.js +++ b/packages/nextjs-cache-handler/jest.config.js @@ -8,4 +8,14 @@ export default { transform: { ...tsJestTransformCfg, }, + reporters: [ + "default", + [ + "jest-junit", + { + outputDirectory: "test-results", + outputName: "junit.xml", + }, + ], + ], }; diff --git a/packages/nextjs-cache-handler/package.json b/packages/nextjs-cache-handler/package.json index c4ad2e7..454a889 100644 --- a/packages/nextjs-cache-handler/package.json +++ b/packages/nextjs-cache-handler/package.json @@ -122,6 +122,7 @@ "globals": "^17.7.0", "ioredis": "^5.11.1", "jest": "^30.4.2", + "jest-junit": "^17.0.0", "prettier": "^3.9.4", "prettier-plugin-packagejson": "3.0.2", "rimraf": "6.1.3", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e71d4b2..dc338be 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -118,6 +118,9 @@ importers: jest: specifier: ^30.4.2 version: 30.4.2(@types/node@22.20.1) + jest-junit: + specifier: ^17.0.0 + version: 17.0.0 prettier: specifier: ^3.9.4 version: 3.9.4 @@ -2942,6 +2945,10 @@ packages: resolution: {integrity: sha512-rFrcONd8jeFsyw+Z9CrScJgglRf2+NFmNam8dKu7n+SoHqNYT47mn0DdEcVUZJpvh7Iz6/si7f7yUH7GJHVgnw==} engines: {node: ^18.14.0 || ^20.0.0 || ^22.0.0 || >=24.0.0} + jest-junit@17.0.0: + resolution: {integrity: sha512-RYWCkq4j59gUXj5DsgbIE7xFBZzu1gtibPhyjSjMmGaOTLnqlXhg7x9zuGCwgbCuMAyoyvk0Mi8wSrRR5uOeLA==} + engines: {node: '>=20.0.0'} + jest-leak-detector@30.4.1: resolution: {integrity: sha512-IpmyiioeHxiWDhesHnUFmOxcTzwCwKpgACgWajtAP+nYQXiY7DakTxB6Bx9JFiRMljr0AX1PvnQdaU1KFoz6NQ==} engines: {node: ^18.14.0 || ^20.0.0 || ^22.0.0 || >=24.0.0} @@ -3270,6 +3277,11 @@ packages: resolution: {integrity: sha512-tEBHqDnIoM/1rXME1zgka9g6Q2lcoCkxHLuc7ODJ5BxbP5d4c2Z5cGgtXAku59200Cx7diuHTOYfSBD8n6mm8A==} engines: {node: '>=16 || 14 >=14.17'} + mkdirp@1.0.4: + resolution: {integrity: sha512-vVqVZQyf3WLx2Shd0qJ9xuvqgAyKPLAiqITEtqW0oIUjzo3PePDd6fW9iFz30ef7Ysp/oiWqbhszeGWW2T6Gzw==} + engines: {node: '>=10'} + hasBin: true + mlly@1.8.0: resolution: {integrity: sha512-l8D9ODSRWLe2KHJSifWGwBqpTZXIXTeo8mlKjY+E2HAakaTeNpqAyBZ8GSqLzHgw4XmHmC8whvpjJNMbFZN7/g==} @@ -4062,6 +4074,10 @@ packages: peerDependencies: browserslist: '>= 4.21.0' + uuid@14.0.1: + resolution: {integrity: sha512-6ZxzVpzDXDa3bJWaHilVayA+BH/1zmxCJoVgvmqJnid/gPoKHxUrS/aC/T6LGQtNHT+XHG9fXPJB4d+IrU30Ew==} + hasBin: true + v8-to-istanbul@9.3.0: resolution: {integrity: sha512-kiGUalWN+rgBJ/1OHZsBtU4rXZOfj/7rKQxULKlIzwzQSvMJUUNgPwJEEh7gU6xEVxC0ahoOBvN2YI8GH6FNgA==} engines: {node: '>=10.12.0'} @@ -4112,6 +4128,9 @@ packages: resolution: {integrity: sha512-+QU2zd6OTD8XWIJCbffaiQeH9U73qIqafo1x6V1snCWYGJf6cVE0cDR4D8xRzcEnfI21IFrUPzPGtcPf8AC+Rw==} engines: {node: ^14.17.0 || ^16.13.0 || >=18.0.0} + xml@1.0.1: + resolution: {integrity: sha512-huCv9IH9Tcf95zuYCsQraZtWnJvBtLVE0QHMOs8bWyZAFZNDcYjsPq1nEx8jKA9y+Beo9v+7OBPRisQTjinQMw==} + y18n@5.0.8: resolution: {integrity: sha512-0pfFzegeDWJHJIAmTLRP2DwHjdF5s7jo9tuztdQxAhINCdvS+3nGINqPd00AphqJR/0LhANUS6/+7SCb98YOfA==} engines: {node: '>=10'} @@ -6176,8 +6195,8 @@ snapshots: '@next/eslint-plugin-next': 16.3.0(eslint@9.39.1(jiti@2.6.1)) eslint: 9.39.1(jiti@2.6.1) eslint-import-resolver-node: 0.3.9 - eslint-import-resolver-typescript: 3.10.1(eslint-plugin-import@2.32.0)(eslint@9.39.1(jiti@2.6.1)) - eslint-plugin-import: 2.32.0(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.1(jiti@2.6.1)) + eslint-import-resolver-typescript: 3.10.1(eslint-plugin-import@2.32.0(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)) + eslint-plugin-import: 2.32.0(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)) eslint-plugin-jsx-a11y: 6.10.2(eslint@9.39.1(jiti@2.6.1)) eslint-plugin-react: 7.37.5(eslint@9.39.1(jiti@2.6.1)) eslint-plugin-react-hooks: 7.0.1(eslint@9.39.1(jiti@2.6.1)) @@ -6203,7 +6222,7 @@ snapshots: transitivePeerDependencies: - supports-color - eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0)(eslint@9.39.1(jiti@2.6.1)): + eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)): dependencies: '@nolyfill/is-core-module': 1.0.39 debug: 4.4.3 @@ -6214,21 +6233,21 @@ snapshots: tinyglobby: 0.2.15 unrs-resolver: 1.11.1 optionalDependencies: - eslint-plugin-import: 2.32.0(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.1(jiti@2.6.1)) + eslint-plugin-import: 2.32.0(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)) transitivePeerDependencies: - supports-color - eslint-module-utils@2.12.1(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.1(jiti@2.6.1)): + eslint-module-utils@2.12.1(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)): dependencies: debug: 3.2.7 optionalDependencies: eslint: 9.39.1(jiti@2.6.1) eslint-import-resolver-node: 0.3.9 - eslint-import-resolver-typescript: 3.10.1(eslint-plugin-import@2.32.0)(eslint@9.39.1(jiti@2.6.1)) + eslint-import-resolver-typescript: 3.10.1(eslint-plugin-import@2.32.0(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)) transitivePeerDependencies: - supports-color - eslint-plugin-import@2.32.0(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.1(jiti@2.6.1)): + eslint-plugin-import@2.32.0(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)): dependencies: '@rtsao/scc': 1.1.0 array-includes: 3.1.9 @@ -6239,7 +6258,7 @@ snapshots: doctrine: 2.1.0 eslint: 9.39.1(jiti@2.6.1) eslint-import-resolver-node: 0.3.9 - eslint-module-utils: 2.12.1(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.1(jiti@2.6.1)) + eslint-module-utils: 2.12.1(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)) hasown: 2.0.2 is-core-module: 2.16.1 is-glob: 4.0.3 @@ -6632,7 +6651,7 @@ snapshots: fs.realpath: 1.0.0 inflight: 1.0.6 inherits: 2.0.4 - minimatch: 10.2.2 + minimatch: 10.2.4 once: 1.4.0 path-is-absolute: 1.0.1 @@ -7091,6 +7110,13 @@ snapshots: optionalDependencies: fsevents: 2.3.3 + jest-junit@17.0.0: + dependencies: + mkdirp: 1.0.4 + strip-ansi: 6.0.1 + uuid: 14.0.1 + xml: 1.0.1 + jest-leak-detector@30.4.1: dependencies: '@jest/get-type': 30.1.0 @@ -7489,6 +7515,8 @@ snapshots: minipass@7.1.3: {} + mkdirp@1.0.4: {} + mlly@1.8.0: dependencies: acorn: 8.15.0 @@ -8439,6 +8467,8 @@ snapshots: escalade: 3.2.0 picocolors: 1.1.1 + uuid@14.0.1: {} + v8-to-istanbul@9.3.0: dependencies: '@jridgewell/trace-mapping': 0.3.31 @@ -8517,6 +8547,8 @@ snapshots: imurmurhash: 0.1.4 signal-exit: 4.1.0 + xml@1.0.1: {} + y18n@5.0.8: {} yallist@3.1.1: {} From 8b435efbc3719cf173b3335a754e928a4ce810c7 Mon Sep 17 00:00:00 2001 From: Marcin Kotlicki Date: Wed, 12 Aug 2026 15:22:18 +0200 Subject: [PATCH 06/11] Fixed setOnlyIfNotExists tests --- ...itial-cache-set-only-if-not-exists.spec.ts | 51 ++++++++++++++----- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/examples/redis-minimal/e2e/isolated/initial-cache-set-only-if-not-exists.spec.ts b/examples/redis-minimal/e2e/isolated/initial-cache-set-only-if-not-exists.spec.ts index 7cd2bbe..3430487 100644 --- a/examples/redis-minimal/e2e/isolated/initial-cache-set-only-if-not-exists.spec.ts +++ b/examples/redis-minimal/e2e/isolated/initial-cache-set-only-if-not-exists.spec.ts @@ -11,13 +11,26 @@ import { type IsolatedNextServer, } from "./helpers/isolated-server"; -const SEEDED_CACHE_PATH = "nextjs:/examples/default-cache"; -const SENTINEL = JSON.stringify({ - lastModified: 1, - lifespan: null, - tags: [], - value: { kind: "SENTINEL", __test: true }, -}); +// Two seeded paths deliberately span different `registerInitialCache` code +// paths (setPageCache(..., "app", ...) vs setPageCache(..., "pages", ...)) - +// setOnlyIfNotExists is a single global flag for the whole run (see +// register-initial-cache.ts), not something scoped to one specific +// cachePath, so this proves the NX guard is honored uniformly regardless of +// router kind, not just for one hardcoded App Router page. +const SEEDED_APP_ROUTER_CACHE_PATH = "nextjs:/examples/default-cache"; +const SEEDED_PAGES_ROUTER_CACHE_PATH = "nextjs:/posts/1"; + +function makeSentinel(label: string) { + return JSON.stringify({ + lastModified: 1, + lifespan: null, + tags: [], + value: { kind: "SENTINEL", __test: label }, + }); +} + +const APP_ROUTER_SENTINEL = makeSentinel("app-router"); +const PAGES_ROUTER_SENTINEL = makeSentinel("pages-router"); test.describe("registerInitialCache with setOnlyIfNotExists=true does not clobber existing keys", () => { let redis: IsolatedRedis; @@ -27,10 +40,11 @@ test.describe("registerInitialCache with setOnlyIfNotExists=true does not clobbe redis = await startEphemeralRedis(); await waitForRedisReady(redis.url); - // Pre-seed BEFORE starting next start, so registerInitialCache sees an existing key. + // Pre-seed BEFORE starting next start, so registerInitialCache sees existing keys. const seedClient = createClient({ url: redis.url }); await seedClient.connect(); - await seedClient.set(SEEDED_CACHE_PATH, SENTINEL); + await seedClient.set(SEEDED_APP_ROUTER_CACHE_PATH, APP_ROUTER_SENTINEL); + await seedClient.set(SEEDED_PAGES_ROUTER_CACHE_PATH, PAGES_ROUTER_SENTINEL); await seedClient.quit(); server = await startIsolatedNextServer({ @@ -43,7 +57,7 @@ test.describe("registerInitialCache with setOnlyIfNotExists=true does not clobbe await waitForRedisPopulated( redis.url, - ["/examples/default-cache", "/examples/fetch-tags"], + ["/examples/default-cache", "/examples/fetch-tags", "/posts/1"], { timeoutMs: 60_000 }, ); }); @@ -53,12 +67,23 @@ test.describe("registerInitialCache with setOnlyIfNotExists=true does not clobbe await stopEphemeralRedis(redis); }); - test("pre-existing value key is not overwritten", async () => { + test("pre-existing App Router value key is not overwritten", async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + const value = await client.get(SEEDED_APP_ROUTER_CACHE_PATH); + expect(value).toBe(APP_ROUTER_SENTINEL); + } finally { + await client.quit(); + } + }); + + test("pre-existing Pages Router value key is not overwritten", async () => { const client = createClient({ url: redis.url }); await client.connect(); try { - const value = await client.get(SEEDED_CACHE_PATH); - expect(value).toBe(SENTINEL); + const value = await client.get(SEEDED_PAGES_ROUTER_CACHE_PATH); + expect(value).toBe(PAGES_ROUTER_SENTINEL); } finally { await client.quit(); } From e4fce157be07163962bf2349ff560e0803f874c9 Mon Sep 17 00:00:00 2001 From: Marcin Kotlicki Date: Wed, 12 Aug 2026 15:24:38 +0200 Subject: [PATCH 07/11] Added extra coverage --- ...nitial-cache-overwrites-by-default.spec.ts | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 examples/redis-minimal/e2e/isolated/initial-cache-overwrites-by-default.spec.ts diff --git a/examples/redis-minimal/e2e/isolated/initial-cache-overwrites-by-default.spec.ts b/examples/redis-minimal/e2e/isolated/initial-cache-overwrites-by-default.spec.ts new file mode 100644 index 0000000..c8e90cd --- /dev/null +++ b/examples/redis-minimal/e2e/isolated/initial-cache-overwrites-by-default.spec.ts @@ -0,0 +1,92 @@ +import { test, expect } from "@playwright/test"; +import { createClient } from "redis"; +import { + startEphemeralRedis, + stopEphemeralRedis, + waitForRedisReady, + waitForRedisPopulated, + startIsolatedNextServer, + stopIsolatedNextServer, + type IsolatedRedis, + type IsolatedNextServer, +} from "./helpers/isolated-server"; + +// Control/counterfactual for initial-cache-set-only-if-not-exists.spec.ts: +// that spec proves a pre-seeded key SURVIVES when setOnlyIfNotExists=true. +// On its own, that's consistent with two different explanations - either +// (a) the NX guard specifically causes it to survive, or (b) +// registerInitialCache simply never touches a key that already exists, +// regardless of the flag. This spec seeds the exact same keys and runs with +// the flag left unset (default false), asserting they DO get overwritten - +// ruling out (b) and isolating the flag as the actual cause. +const SEEDED_APP_ROUTER_CACHE_PATH = "nextjs:/examples/default-cache"; +const SEEDED_PAGES_ROUTER_CACHE_PATH = "nextjs:/posts/1"; + +function makeSentinel(label: string) { + return JSON.stringify({ + lastModified: 1, + lifespan: null, + tags: [], + value: { kind: "SENTINEL", __test: label }, + }); +} + +const APP_ROUTER_SENTINEL = makeSentinel("app-router"); +const PAGES_ROUTER_SENTINEL = makeSentinel("pages-router"); + +test.describe("registerInitialCache with setOnlyIfNotExists left at its default (false) overwrites existing keys", () => { + let redis: IsolatedRedis; + let server: IsolatedNextServer; + + test.beforeAll(async () => { + redis = await startEphemeralRedis(); + await waitForRedisReady(redis.url); + + // Pre-seed the SAME keys and sentinels as initial-cache-set-only-if-not-exists.spec.ts, + // but this time boot next start WITHOUT INITIAL_CACHE_SET_ONLY_IF_NOT_EXISTS set. + const seedClient = createClient({ url: redis.url }); + await seedClient.connect(); + await seedClient.set(SEEDED_APP_ROUTER_CACHE_PATH, APP_ROUTER_SENTINEL); + await seedClient.set(SEEDED_PAGES_ROUTER_CACHE_PATH, PAGES_ROUTER_SENTINEL); + await seedClient.quit(); + + server = await startIsolatedNextServer({ + env: { REDIS_URL: redis.url, REDIS_TYPE: "redis" }, + }); + + await waitForRedisPopulated( + redis.url, + ["/examples/default-cache", "/examples/fetch-tags", "/posts/1"], + { timeoutMs: 60_000 }, + ); + }); + + test.afterAll(async () => { + await stopIsolatedNextServer(server); + await stopEphemeralRedis(redis); + }); + + test("pre-existing App Router value key IS overwritten", async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + const value = await client.get(SEEDED_APP_ROUTER_CACHE_PATH); + expect(value).not.toBeNull(); + expect(value).not.toBe(APP_ROUTER_SENTINEL); + } finally { + await client.quit(); + } + }); + + test("pre-existing Pages Router value key IS overwritten", async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + const value = await client.get(SEEDED_PAGES_ROUTER_CACHE_PATH); + expect(value).not.toBeNull(); + expect(value).not.toBe(PAGES_ROUTER_SENTINEL); + } finally { + await client.quit(); + } + }); +}); From 1ee5326828dbde2c6efaebeed20a20b869317270 Mon Sep 17 00:00:00 2001 From: Marcin Kotlicki Date: Wed, 12 Aug 2026 15:30:02 +0200 Subject: [PATCH 08/11] Added redis precheck --- .../e2e/isolated/initial-cache-population.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts b/examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts index 5293464..1aedc77 100644 --- a/examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts +++ b/examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts @@ -28,6 +28,18 @@ test.describe("registerInitialCache populates Redis on boot (setOnlyIfNotExists= redis = await startEphemeralRedis(); await waitForRedisReady(redis.url); + // Precondition: Redis must be genuinely empty before `next start` boots, + // so the population assertions below can only be explained by + // registerInitialCache - not by stale data left over in the container. + const preCheckClient = createClient({ url: redis.url }); + await preCheckClient.connect(); + try { + const dbSize = await preCheckClient.dbSize(); + expect(dbSize).toBe(0); + } finally { + await preCheckClient.quit(); + } + server = await startIsolatedNextServer({ env: { REDIS_URL: redis.url, REDIS_TYPE: "redis" }, }); From f33f9b30ae634af5bfedff20e916f785c70457aa Mon Sep 17 00:00:00 2001 From: Marcin Kotlicki Date: Wed, 12 Aug 2026 15:59:23 +0200 Subject: [PATCH 09/11] Added coverage for TTL and revalidate --- .../e2e/isolated/helpers/isolated-server.ts | 30 ++++ .../isolated/initial-cache-population.spec.ts | 22 +++ .../initial-cache-revalidate-tag-live.spec.ts | 83 +++++++++ .../redis-handler-revalidate-tag.spec.ts | 164 ++++++++++++++++++ .../redis-handler-tags-and-ttl.spec.ts | 163 +++++++++++++++++ 5 files changed, 462 insertions(+) create mode 100644 examples/redis-minimal/e2e/isolated/initial-cache-revalidate-tag-live.spec.ts create mode 100644 examples/redis-minimal/e2e/isolated/redis-handler-revalidate-tag.spec.ts create mode 100644 examples/redis-minimal/e2e/isolated/redis-handler-tags-and-ttl.spec.ts diff --git a/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts b/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts index 1efb7b9..cffbd91 100644 --- a/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts +++ b/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts @@ -156,6 +156,36 @@ export async function waitForRedisPopulated( ); } +/** + * Polls Redis until a value key is gone. Needed because Next.js's own + * `revalidateTag()`/`revalidatePath()` do not guarantee the underlying + * CacheHandler purge completes synchronously with the HTTP response that + * triggered it - the actual Redis deletion can land shortly after the + * response returns. Confirmed empirically: asserting immediately after a + * `POST /api/revalidate` response is flaky; polling is required. + */ +export async function waitForRedisKeyDeleted( + url: string, + key: string, + opts: { timeoutMs?: number; intervalMs?: number } = {}, +): Promise { + const timeoutMs = opts.timeoutMs ?? 10_000; + const intervalMs = opts.intervalMs ?? 200; + const deadline = Date.now() + timeoutMs; + + const client = await connectWithTimeout(url); + try { + while (Date.now() < deadline) { + const value = await client.get(key); + if (value === null) return; + await sleep(intervalMs); + } + throw new Error(`Redis key "${key}" was still present after ${timeoutMs}ms`); + } finally { + await client.quit().catch(() => {}); + } +} + export async function startIsolatedNextServer(opts: { port?: number; cwd?: string; diff --git a/examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts b/examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts index 1aedc77..174709c 100644 --- a/examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts +++ b/examples/redis-minimal/e2e/isolated/initial-cache-population.spec.ts @@ -110,4 +110,26 @@ test.describe("registerInitialCache populates Redis on boot (setOnlyIfNotExists= await client.quit(); } }); + + // Bridges confidence between this real-app-boot proof and the exhaustive, + // deterministic TTL matrix in redis-handler-tags-and-ttl.spec.ts: the real + // CacheHandler -> registerInitialCache -> redis-strings pipeline must + // produce internally-consistent state, not just "a TTL exists somewhere." + for (const path of ["/examples/fetch-tags", "/posts/1"]) { + test(`real Redis EXPIRETIME agrees with sharedTagsTtlKey for ${path}`, async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + const rawTtl = await client.hGet("nextjs:__sharedTagsTtl__", path); + expect(rawTtl).not.toBeNull(); + const expectedExpireAt = Number(rawTtl); + + const expireTime = await client.expireTime(`nextjs:${path}`); + expect(expireTime).toBe(expectedExpireAt); + expect(expireTime).toBeGreaterThan(Math.floor(Date.now() / 1000)); + } finally { + await client.quit(); + } + }); + } }); diff --git a/examples/redis-minimal/e2e/isolated/initial-cache-revalidate-tag-live.spec.ts b/examples/redis-minimal/e2e/isolated/initial-cache-revalidate-tag-live.spec.ts new file mode 100644 index 0000000..67713d6 --- /dev/null +++ b/examples/redis-minimal/e2e/isolated/initial-cache-revalidate-tag-live.spec.ts @@ -0,0 +1,83 @@ +import { test, expect } from "@playwright/test"; +import { createClient } from "redis"; +import { + startEphemeralRedis, + stopEphemeralRedis, + waitForRedisReady, + waitForRedisPopulated, + waitForRedisKeyDeleted, + waitForHttpReady, + startIsolatedNextServer, + stopIsolatedNextServer, + type IsolatedRedis, + type IsolatedNextServer, +} from "./helpers/isolated-server"; + +// Bridges the exhaustive, direct-handler-level revalidateTag coverage in +// redis-handler-revalidate-tag.spec.ts to the FULL real pipeline: Next.js's +// own revalidateTag() -> CacheHandler.revalidateTag() -> composite handler +// -> redis-strings handler. The shared e2e suite (examples/redis-minimal/e2e) +// already proves this indirectly via an HTTP response timestamp change; this +// spec asserts the real Redis deletion directly instead of inferring it. +test.describe("revalidateTag via the real /api/revalidate route deletes real Redis state", () => { + let redis: IsolatedRedis; + let server: IsolatedNextServer; + + test.beforeAll(async () => { + redis = await startEphemeralRedis(); + await waitForRedisReady(redis.url); + + server = await startIsolatedNextServer({ + env: { REDIS_URL: redis.url, REDIS_TYPE: "redis" }, + }); + + await waitForRedisPopulated(redis.url, ["/examples/fetch-tags"], { + timeoutMs: 60_000, + }); + await waitForHttpReady(server.baseURL); + }); + + test.afterAll(async () => { + await stopIsolatedNextServer(server); + await stopEphemeralRedis(redis); + }); + + test("POST /api/revalidate with a tag deletes the matching value key and tag hash entries", async () => { + const client = createClient({ url: redis.url }); + await client.connect(); + try { + // Precondition: confirm the entry is actually present before revalidating, + // so the deletion we assert afterward is unambiguously caused by this call. + expect(await client.get("nextjs:/examples/fetch-tags")).not.toBeNull(); + expect( + await client.hExists("nextjs:__sharedTags__", "/examples/fetch-tags"), + ).toBe(1); + + const response = await fetch(`${server.baseURL}/api/revalidate`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ tag: "futurama", cacheLife: "max" }), + }); + expect(response.ok).toBe(true); + + // Next's revalidateTag()/revalidatePath() don't guarantee the underlying + // CacheHandler purge completes synchronously with the HTTP response - + // poll rather than asserting immediately (confirmed empirically flaky + // otherwise). See waitForRedisKeyDeleted's doc comment. + await waitForRedisKeyDeleted(redis.url, "nextjs:/examples/fetch-tags"); + + expect(await client.get("nextjs:/examples/fetch-tags")).toBeNull(); + expect( + await client.hExists("nextjs:__sharedTags__", "/examples/fetch-tags"), + ).toBe(0); + expect( + await client.hExists( + "nextjs:__sharedTagsTtl__", + "/examples/fetch-tags", + ), + ).toBe(0); + } finally { + await client.quit(); + } + }); +}); diff --git a/examples/redis-minimal/e2e/isolated/redis-handler-revalidate-tag.spec.ts b/examples/redis-minimal/e2e/isolated/redis-handler-revalidate-tag.spec.ts new file mode 100644 index 0000000..45af23f --- /dev/null +++ b/examples/redis-minimal/e2e/isolated/redis-handler-revalidate-tag.spec.ts @@ -0,0 +1,164 @@ +import { test, expect } from "@playwright/test"; +import { createClient, type RedisClientType } from "redis"; +import createRedisHandler from "@fortedigital/nextjs-cache-handler/redis-strings"; +import { + startEphemeralRedis, + stopEphemeralRedis, + waitForRedisReady, + type IsolatedRedis, +} from "./helpers/isolated-server"; + +// Direct handler construction against a real, ephemeral Redis (see +// redis-handler-tags-and-ttl.spec.ts for rationale). One shared Redis for +// the whole file; each test uses its own keyPrefix so tests never interfere +// with each other despite sharing the container. + +let redis: IsolatedRedis; +let client: RedisClientType; + +test.beforeAll(async () => { + redis = await startEphemeralRedis(); + await waitForRedisReady(redis.url); + client = createClient({ url: redis.url }) as RedisClientType; + await client.connect(); +}); + +test.afterAll(async () => { + await client.quit(); + await stopEphemeralRedis(redis); +}); + +function makeLifespan(expireAt: number) { + return { + expireAge: 0, + expireAt, + lastModifiedAt: Math.floor(Date.now() / 1000), + revalidate: 0, + staleAge: 0, + staleAt: 0, + }; +} + +function makeEntry(tags: string[], expireAt = Math.floor(Date.now() / 1000) + 10_000) { + return { + lastModified: Date.now(), + lifespan: makeLifespan(expireAt), + tags, + value: { kind: "FETCH" }, + } as any; +} + +function handlerFor(keyPrefix: string) { + return createRedisHandler({ client: client as any, keyPrefix }); +} + +test("revalidateTag deletes only entries matching the tag, leaves unrelated entries fully intact", async () => { + const keyPrefix = "revtag-selective:"; + const handler = handlerFor(keyPrefix); + + await handler.set("a", makeEntry(["shared-tag", "only-a"]), {} as any); + await handler.set("b", makeEntry(["shared-tag", "only-b"]), {} as any); + await handler.set("c", makeEntry(["unrelated"]), {} as any); + + const cValueBefore = await client.get(`${keyPrefix}c`); + const cTagsBefore = await client.hGet(`${keyPrefix}__sharedTags__`, "c"); + const cTtlBefore = await client.hGet(`${keyPrefix}__sharedTagsTtl__`, "c"); + + await handler.revalidateTag("shared-tag"); + + expect(await client.get(`${keyPrefix}a`)).toBeNull(); + expect(await client.hExists(`${keyPrefix}__sharedTags__`, "a")).toBe(0); + expect(await client.hExists(`${keyPrefix}__sharedTagsTtl__`, "a")).toBe(0); + + expect(await client.get(`${keyPrefix}b`)).toBeNull(); + expect(await client.hExists(`${keyPrefix}__sharedTags__`, "b")).toBe(0); + expect(await client.hExists(`${keyPrefix}__sharedTagsTtl__`, "b")).toBe(0); + + // "c" never carried the revalidated tag - must be byte-for-byte unchanged. + expect(await client.get(`${keyPrefix}c`)).toBe(cValueBefore); + expect(await client.hGet(`${keyPrefix}__sharedTags__`, "c")).toBe(cTagsBefore); + expect(await client.hGet(`${keyPrefix}__sharedTagsTtl__`, "c")).toBe(cTtlBefore); +}); + +test("revalidateTag with no matching entries is a safe no-op", async () => { + const keyPrefix = "revtag-nomatch:"; + const handler = handlerFor(keyPrefix); + + await handler.set("x", makeEntry(["foo"]), {} as any); + await handler.set("y", makeEntry(["bar"]), {} as any); + + const before = { + x: await client.get(`${keyPrefix}x`), + y: await client.get(`${keyPrefix}y`), + xTags: await client.hGet(`${keyPrefix}__sharedTags__`, "x"), + yTags: await client.hGet(`${keyPrefix}__sharedTags__`, "y"), + }; + + await handler.revalidateTag("does-not-exist-anywhere"); + + expect(await client.get(`${keyPrefix}x`)).toBe(before.x); + expect(await client.get(`${keyPrefix}y`)).toBe(before.y); + expect(await client.hGet(`${keyPrefix}__sharedTags__`, "x")).toBe(before.xTags); + expect(await client.hGet(`${keyPrefix}__sharedTags__`, "y")).toBe(before.yTags); +}); + +test("an implicit tag matching an existing entry's tags is eagerly deleted, and bookkeeping is written", async () => { + const keyPrefix = "revtag-implicit-eager:"; + const handler = handlerFor(keyPrefix); + const implicitTag = "_N_T_/some/path"; + + // App/Pages Router entries genuinely carry implicit path tags like this one + // in their stored tags array (confirmed via the real-app-boot specs) - + // revalidateTag's eager hScan+match+delete logic does not special-case + // implicit tags, so a match here is deleted immediately, same as any tag. + await handler.set("some/path", makeEntry([implicitTag]), {} as any); + + await handler.revalidateTag(implicitTag); + + expect(await client.get(`${keyPrefix}some/path`)).toBeNull(); + expect(await client.hExists(`${keyPrefix}__sharedTags__`, "some/path")).toBe(0); + expect(await client.hExists(`${keyPrefix}__sharedTagsTtl__`, "some/path")).toBe(0); + + const bookkeeping = await client.hGet(`${keyPrefix}__revalidated_tags__`, implicitTag); + expect(bookkeeping).not.toBeNull(); +}); + +test("an implicit tag not present on any entry only writes bookkeeping (the genuinely lazy path) and deletes nothing", async () => { + const keyPrefix = "revtag-implicit-lazy:"; + const handler = handlerFor(keyPrefix); + const implicitTag = "_N_T_/never/cached"; + + await handler.set("other/path", makeEntry(["unrelated-tag"]), {} as any); + const before = await client.get(`${keyPrefix}other/path`); + + await handler.revalidateTag(implicitTag); + + // Nothing in the shared-tags hash contains this literal tag, so the + // eager scan+match+delete finds nothing to remove. + expect(await client.get(`${keyPrefix}other/path`)).toBe(before); + expect(await client.hExists(`${keyPrefix}__sharedTags__`, "other/path")).toBe(1); + + // But the bookkeeping write happens unconditionally for implicit tags, + // regardless of whether anything currently matches - this is the only + // effect in this scenario, and it's what makes later get() calls able to + // lazily invalidate a not-yet-cached entry once it does get cached. + const bookkeeping = await client.hGet(`${keyPrefix}__revalidated_tags__`, implicitTag); + expect(bookkeeping).not.toBeNull(); +}); + +test("revalidateTag also sweeps and removes independently TTL-expired hash bookkeeping as a side effect", async () => { + const keyPrefix = "revtag-ttl-sweep:"; + const handler = handlerFor(keyPrefix); + + const pastExpireAt = Math.floor(Date.now() / 1000) - 1000; + await handler.set("expired", makeEntry(["irrelevant-tag"], pastExpireAt), {} as any); + + // revalidateSharedKeys() runs as part of every revalidateTag() call + // (Promise.all([revalidateTag, revalidateSharedKeys])) - even a completely + // unrelated tag revalidation should still sweep up this independently + // expired entry's hash bookkeeping. + await handler.revalidateTag("completely-unrelated-tag"); + + expect(await client.hExists(`${keyPrefix}__sharedTagsTtl__`, "expired")).toBe(0); + expect(await client.hExists(`${keyPrefix}__sharedTags__`, "expired")).toBe(0); +}); diff --git a/examples/redis-minimal/e2e/isolated/redis-handler-tags-and-ttl.spec.ts b/examples/redis-minimal/e2e/isolated/redis-handler-tags-and-ttl.spec.ts new file mode 100644 index 0000000..92e925c --- /dev/null +++ b/examples/redis-minimal/e2e/isolated/redis-handler-tags-and-ttl.spec.ts @@ -0,0 +1,163 @@ +import { test, expect } from "@playwright/test"; +import { createClient, type RedisClientType } from "redis"; +import createRedisHandler from "@fortedigital/nextjs-cache-handler/redis-strings"; +import { + startEphemeralRedis, + stopEphemeralRedis, + waitForRedisReady, + type IsolatedRedis, +} from "./helpers/isolated-server"; + +// This spec constructs the redis-strings handler directly (the same public +// subpath export `examples/redis-minimal/cache-handler.mjs` uses in +// production) against a real, ephemeral Redis - no `next start` boot at all. +// That gives full deterministic control over arbitrary tags/lifespans, which +// is essential for a matrix of TTL/tag scenarios the fixed example-app +// routes can't cleanly provide, and it's fast (no Next.js process). +const KEY_PREFIX = "nextjs:"; + +function makeLifespan(expireAt: number) { + return { + expireAge: 0, + expireAt, + lastModifiedAt: Math.floor(Date.now() / 1000), + revalidate: 0, + staleAge: 0, + staleAt: 0, + }; +} + +test.describe("redis-strings handler: sharedTagsKey and sharedTagsTtlKey rules (direct construction, no next start)", () => { + let redis: IsolatedRedis; + let client: RedisClientType; + let handler: ReturnType; + + test.beforeAll(async () => { + redis = await startEphemeralRedis(); + await waitForRedisReady(redis.url); + client = createClient({ url: redis.url }) as RedisClientType; + await client.connect(); + // `redis` (this package's own dependency) and the internal `@redis/client` + // version pinned inside @fortedigital/nextjs-cache-handler are structurally + // identical at runtime but nominally distinct types (private class fields) - + // cast at the boundary, same as this repo's own cache-handler.mjs does implicitly + // via plain JS. + handler = createRedisHandler({ client: client as any, keyPrefix: KEY_PREFIX }); + }); + + test.afterAll(async () => { + await client.quit(); + await stopEphemeralRedis(redis); + }); + + test("sharedTagsKey stores an empty tags array verbatim", async () => { + await handler.set( + "tags-empty", + { + lastModified: Date.now(), + lifespan: makeLifespan(Math.floor(Date.now() / 1000) + 1000), + tags: [], + value: { kind: "FETCH" }, + } as any, + {} as any, + ); + + const raw = await client.hGet("nextjs:__sharedTags__", "tags-empty"); + expect(raw).toBe("[]"); + }); + + test("sharedTagsKey stores a multi-tag array verbatim, including duplicates (no dedup at this layer)", async () => { + await handler.set( + "tags-multi", + { + lastModified: Date.now(), + lifespan: makeLifespan(Math.floor(Date.now() / 1000) + 1000), + tags: ["a", "b", "a"], + value: { kind: "FETCH" }, + } as any, + {} as any, + ); + + const raw = await client.hGet("nextjs:__sharedTags__", "tags-multi"); + expect(raw).toBe(JSON.stringify(["a", "b", "a"])); + }); + + const ttlCases = [ + { label: "30s revalidate", offsetSeconds: 30 }, + { label: "3600s (1h) revalidate", offsetSeconds: 3600 }, + { label: "86400s (1d) revalidate", offsetSeconds: 86400 }, + ]; + + for (const { label, offsetSeconds } of ttlCases) { + test(`sharedTagsTtlKey and real Redis EXPIRETIME agree exactly for ${label}`, async () => { + const key = `ttl-${offsetSeconds}`; + const expireAt = Math.floor(Date.now() / 1000) + offsetSeconds; + + await handler.set( + key, + { + lastModified: Date.now(), + lifespan: makeLifespan(expireAt), + tags: [], + value: { kind: "FETCH" }, + } as any, + {} as any, + ); + + const rawTtl = await client.hGet("nextjs:__sharedTagsTtl__", key); + expect(rawTtl).toBe(String(expireAt)); + + const expireTime = await client.expireTime(`nextjs:${key}`); + expect(expireTime).toBe(expireAt); + }); + } + + test("lifespan: null produces a permanent key with no TTL and no sharedTagsTtlKey entry", async () => { + const key = "permanent-key"; + + await handler.set( + key, + { + lastModified: Date.now(), + lifespan: null, + tags: [], + value: { kind: "FETCH" }, + } as any, + {} as any, + ); + + const ttlEntryExists = await client.hExists("nextjs:__sharedTagsTtl__", key); + expect(ttlEntryExists).toBe(0); + + const ttl = await client.ttl(`nextjs:${key}`); + expect(ttl).toBe(-1); + + // "No TTL" must not mean "not written" - the value itself is still cached. + const value = await client.get(`nextjs:${key}`); + expect(value).not.toBeNull(); + }); + + test("keyExpirationStrategy=EXAT produces the same real Redis expiry as the default EXPIREAT", async () => { + const exatHandler = createRedisHandler({ + client: client as any, + keyPrefix: "nextjs-exat:", + keyExpirationStrategy: "EXAT", + }); + const key = "exat-check"; + const expireAt = Math.floor(Date.now() / 1000) + 500; + + await exatHandler.set( + key, + { + lastModified: Date.now(), + lifespan: makeLifespan(expireAt), + tags: [], + value: { kind: "FETCH" }, + } as any, + {} as any, + ); + + const expireTime = await client.expireTime(`nextjs-exat:${key}`); + expect(expireTime).toBe(expireAt); + }); +}); From dbfe6bab4afd74bdca9231196ac46b94cf6f07f6 Mon Sep 17 00:00:00 2001 From: Marcin Kotlicki Date: Wed, 12 Aug 2026 15:59:31 +0200 Subject: [PATCH 10/11] Skill improvements --- .claude/skills/new-e2e-test/SKILL.md | 39 +++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/.claude/skills/new-e2e-test/SKILL.md b/.claude/skills/new-e2e-test/SKILL.md index 249adbf..2156c40 100644 --- a/.claude/skills/new-e2e-test/SKILL.md +++ b/.claude/skills/new-e2e-test/SKILL.md @@ -121,10 +121,35 @@ there. See `e2e/isolated/` and `playwright.isolated.config.ts` instead: specs there self-manage a Docker Redis container and a `next start` child process via `e2e/isolated/helpers/isolated-server.ts` -(`startEphemeralRedis`/`waitForRedisReady`/`waitForRedisPopulated`/`startIsolatedNextServer`/ -`stopIsolatedNextServer`), run via `pnpm test:e2e:isolated` (requires Docker). Key lesson from -building this suite: don't synchronize on "the shared-tags hash has *any* entries" — routes are -written under `registerInitialCache`'s own internal concurrency limit, so an early partial write -can pass an `HLEN > 0` check while routes your test cares about are still being written. Wait for -the *specific* cache paths the test asserts on (`waitForRedisPopulated`'s `expectedCachePaths` -argument) rather than a generic non-empty check. +(`startEphemeralRedis`/`waitForRedisReady`/`waitForRedisPopulated`/`waitForRedisKeyDeleted`/ +`startIsolatedNextServer`/`stopIsolatedNextServer`), run via `pnpm test:e2e:isolated` (requires +Docker). Key lessons from building this suite: +- Don't synchronize on "the shared-tags hash has *any* entries" — routes are written under + `registerInitialCache`'s own internal concurrency limit, so an early partial write can pass an + `HLEN > 0` check while routes your test cares about are still being written. Wait for the + *specific* cache paths the test asserts on (`waitForRedisPopulated`'s `expectedCachePaths` + argument) rather than a generic non-empty check. +- Next's own `revalidateTag()`/`revalidatePath()` don't guarantee the underlying `CacheHandler` + purge completes synchronously with the HTTP response that triggered it — asserting on Redis + immediately after such a response is flaky (confirmed empirically). Poll with + `waitForRedisKeyDeleted` (or similar) instead of asserting once. + +### Two styles of isolated test — pick deliberately + +- **Boot `next start`** (`startIsolatedNextServer`) — needed to prove something about the *whole + pipeline*: `registerInitialCache`'s disk-to-Redis population, `setOnlyIfNotExists`, or that a + real HTTP route (`/api/revalidate`) actually mutates Redis. Slower (real build + boot), but the + only way to exercise the real `CacheHandler`'s kind-based tag computation + (`APP_PAGE`/`PAGES`/`FETCH`) against real Next.js-generated values. +- **Direct handler construction** (no `next start` at all) — for handler-level correctness that + doesn't depend on Next.js's runtime: import `createRedisHandler` from + `@fortedigital/nextjs-cache-handler/redis-strings` directly (the same public subpath export + `examples/redis-minimal/cache-handler.mjs` uses in production — not a private API) and call + `.set()`/`.revalidateTag()`/`.prepare()` on it against the ephemeral Redis client directly. Gives + full deterministic control over arbitrary tags/`lifespan`/`revalidate` shapes and entry counts — + essential for a *matrix* of scenarios the fixed example-app routes can't cleanly provide (e.g. + every `revalidate` value, a `lifespan: null` permanent-key case that's unreachable via any real + route under `next start`) — and it's fast, since there's no Next.js process to boot. See + `redis-handler-tags-and-ttl.spec.ts` and `redis-handler-revalidate-tag.spec.ts` for the pattern: + one shared ephemeral Redis per file (`beforeAll`/`afterAll`), a distinct `keyPrefix` per test to + keep them fully isolated from each other without paying for a separate container each time. From 263dcca398189316a362f316d20ddd4c14a59619 Mon Sep 17 00:00:00 2001 From: Marcin Kotlicki Date: Wed, 12 Aug 2026 16:18:09 +0200 Subject: [PATCH 11/11] Flaky test improvement --- .../e2e/isolated/helpers/isolated-server.ts | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts b/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts index cffbd91..0e4dcdc 100644 --- a/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts +++ b/examples/redis-minimal/e2e/isolated/helpers/isolated-server.ts @@ -1,6 +1,7 @@ import { spawn, exec as execCallback, type ChildProcess } from "node:child_process"; import { promisify } from "node:util"; import path from "node:path"; +import net from "node:net"; import { createClient } from "redis"; const exec = promisify(execCallback); @@ -26,6 +27,32 @@ function randomSuffix(): string { return Math.random().toString(36).slice(2, 10); } +/** + * Allocates a genuinely free OS-assigned port, rather than reusing a fixed + * default across every spec file. Sharing a fixed port (e.g. always 16380) + * caused a real flake: when one file's `beforeAll` starts a fresh container + * on that port at nearly the same moment the *previous* file's `afterAll` is + * `docker rm -f`-ing its own container off the same port, a client can land + * on the old container mid-teardown and get `ECONNRESET`. Unique ports per + * call eliminate this race entirely instead of trying to tighten the timing. + */ +function getFreePort(): Promise { + return new Promise((resolve, reject) => { + const probe = net.createServer(); + probe.unref(); + probe.on("error", reject); + probe.listen(0, () => { + const address = probe.address(); + if (address && typeof address === "object") { + const port = address.port; + probe.close(() => resolve(port)); + } else { + probe.close(() => reject(new Error("Could not determine a free port"))); + } + }); + }); +} + const DEFAULT_CONNECT_ATTEMPT_TIMEOUT_MS = 3_000; /** @@ -67,7 +94,7 @@ async function connectWithTimeout( export async function startEphemeralRedis( opts: { port?: number; image?: string } = {}, ): Promise { - const port = opts.port ?? 16380; + const port = opts.port ?? (await getFreePort()); const image = opts.image ?? "redis:7-alpine"; const containerName = `nca-e2e-redis-${randomSuffix()}`; @@ -191,7 +218,7 @@ export async function startIsolatedNextServer(opts: { cwd?: string; env: Record; }): Promise { - const port = opts.port ?? 3010; + const port = opts.port ?? (await getFreePort()); const cwd = opts.cwd ?? path.resolve(__dirname, "..", "..", ".."); const child = spawn(`pnpm exec next start --port ${port}`, [], {