diff --git a/CLAUDE.md b/CLAUDE.md index 4c3bb79..a584727 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -113,7 +113,7 @@ When one is built, take it off this list and prove the behaviour with a test or - Tests in `test/shop` and `test/theme` run inside workerd against the site's own Worker. `test/shop/helpers.ts` brings a site up through Mallok's HTTP API (first request → admin → token → settings → plugin enabled); tables are created by Mallok's migrator, never by hand. Create content with `createContent`, not by inserting rows. - Mallok caches pages in `caches.default`. In a test, create all content before requesting any page. - A test for a fix must be seen failing without the fix. For new guards, break the guard and confirm the test goes red. -- A race is tested by running the calls with `Promise.all`, and such a test only counts once breaking the guard turns it red **every time**: that is the proof the two calls really interleave. Through `SELF.fetch` they do so about half the time, and where a request reads before it writes, the other half never reaches the guard. `holdWrites` in `test/shop/helpers.ts` holds each request at its write until the test has seen them all arrive; the inquiry's races use it. It waits by polling a flag: a promise settled by one request for another stops the Worker. +- A race is tested by running the calls with `Promise.all`, and such a test only counts once breaking the guard turns it red **every time**: that is the proof the two calls really interleave. Through `SELF.fetch` they do so about half the time, and where a request reads before it writes, the other half never reaches the guard. `holdWrites` in `test/shop/helpers.ts` holds each request at its write until the test has seen them all arrive; the inquiry's races use it. It waits by polling a flag: a promise settled by one request for another stops the Worker. Functions called side by side in one test are the other case: each runs as far as its reading before the next begins, every time. `atTheSameMoment` runs them and fails unless every call had read before any wrote, so that this is seen on every run and not assumed; the payment and fulfilment races use it. An order's write is stopped by several keys at once — the status it was read in, the event's keys, the ledger row's id, the outbox row's id — so taking one away turns no test red; taking them all away turns each race red every time. - `countD1Calls` in `test/shop/helpers.ts` counts round trips; use it wherever the number is a design constraint. `interceptBatches` runs a hook around each batch: it is how a test changes the data between a function's reading and its writing, or loses the answer to a write that committed. - `test/theme/pages.test.ts` renders every layout from content it creates itself, with everything set; `test/theme/bare.test.ts` does the same for a site that has filled in almost nothing, and fails on any empty element or `href=""`. Neither reads `content/`. A template that prints a wrapper has to check that there is something to put in it — and `content.html` is not a string, so capture it before comparing it with `blank`. The sample is checked by `test/content.test.ts` and `test/project.test.ts` (files only, no Worker) and by the smoke run. - `npm run smoke:shop` is the only place theme, plugin, content and the Mallok CLI run together; run it when touching any of them. It publishes the real sample, requests every kind of page, and follows every header and footer link in all four languages. diff --git a/docs/superpowers/specs/2026-09-30-nundar-on-mallok-design.md b/docs/superpowers/specs/2026-09-30-nundar-on-mallok-design.md index b90c30b..4ce433c 100644 --- a/docs/superpowers/specs/2026-09-30-nundar-on-mallok-design.md +++ b/docs/superpowers/specs/2026-09-30-nundar-on-mallok-design.md @@ -693,6 +693,6 @@ Not changed: the export still takes up to a thousand inquiries, as Mallok's own 4. *A job is queued beside a conditional write that stored nothing* in a true race, and finds nothing to send. Mallok's statement takes no condition. Written up. 5. *An amount is stored twice* because a table panel has no money column. Written up. 6. *Whether a quantity must be a multiple of the minimum order* (§17, left open, item 8) and *the rounding of a derived price* (item 3) were put to the owner with this work and are not settled in it. -7. *The older tests of a race* — a payment reported twice at once, two saves of one variant — are written the way the inquiry's first were, with `Promise.all` alone. They were seen red with their guards broken, once each. Whether they are red every time has not been looked at since the review showed that this harness interleaves only sometimes. +7. *The older tests of a race* — a payment reported twice at once, two refunds, a payment against a cancellation, nine in all — were looked at the same day, after the review. They call functions side by side rather than send requests, and such calls do overlap every time: twenty-five rounds of two payments went to the database in one and the same order, both readings before either write. So none had to be rewritten for that. They now run through `atTheSameMoment`, which fails unless the calls overlapped, so that it is seen on every run rather than taken on trust; and each of the nine, with everything that stops its second write taken away, was red eight times in eight. One thing that showed: an order's write is stopped by several keys at once — the status condition, the event's two keys, the ledger row's id, the outbox row's id — and removing the status condition alone turns no payment test red. That is the design, not a gap in the tests. 8. *A request with no connecting address.* Whether a zone that uses Cloudflare's "Remove visitor IP headers" transform still hands a Worker that header is not stated in the documentation that was read. If it does not, every visitor of such a site shares one limit of five an hour. diff --git a/test/shop/helpers.test.ts b/test/shop/helpers.test.ts new file mode 100644 index 0000000..dc506b6 --- /dev/null +++ b/test/shop/helpers.test.ts @@ -0,0 +1,59 @@ +import { beforeAll, describe, expect, it } from 'vitest'; +import { atTheSameMoment, db, ensureSite, fulfilledValues } from './helpers.js'; + +/** + * The helper the tests of a race stand on, tested itself: a check that the + * calls overlapped is worth nothing unless it fails when they did not. + */ +describe('calls made at the same moment', () => { + beforeAll(ensureSite); + + /** Reads, then writes: the shape of everything a race is tested on. */ + const readThenWrite = async (database: D1Database): Promise => { + await database.prepare('SELECT 1').first(); + await database.batch([database.prepare('SELECT 2')]); + return 'done'; + }; + + it('are let through when each has read before any writes', async () => { + const settled = await atTheSameMoment([readThenWrite, readThenWrite]); + + expect(fulfilledValues(settled)).toEqual(['done', 'done']); + }); + + it('are refused when one had finished before another began', async () => { + // The second waits on the database, with a database that is not the one + // it was given, before its own reading: a sequence dressed as a race. + const late = async (database: D1Database): Promise => { + await db().prepare('SELECT 0').first(); + await db().prepare('SELECT 0').first(); + return readThenWrite(database); + }; + + await expect(atTheSameMoment([readThenWrite, late])).rejects.toThrow( + /did not overlap.*0\.0 0\.1 1\.0 1\.1/, + ); + }); + + it('are refused when one never went to the database it was given', async () => { + await expect( + atTheSameMoment([readThenWrite, async () => 'elsewhere']), + ).rejects.toThrow(/did not overlap/); + }); + + it('hand back a call that failed as failed, and say so when none may', async () => { + const settled = await atTheSameMoment([ + readThenWrite, + async (database) => { + await database.prepare('SELECT 1').first(); + throw new Error('refused'); + }, + ]); + + expect(settled.map((result) => result.status)).toEqual([ + 'fulfilled', + 'rejected', + ]); + expect(() => fulfilledValues(settled)).toThrow('refused'); + }); +}); diff --git a/test/shop/helpers.ts b/test/shop/helpers.ts index 008f120..c145faa 100644 --- a/test/shop/helpers.ts +++ b/test/shop/helpers.ts @@ -556,6 +556,103 @@ export async function storeInquiry(input: { return inquiryNo; } +/** One call a database of {@link atTheSameMoment} saw: whose, and which. */ +interface NotedCall { + readonly call: number; + readonly nth: number; +} + +/** + * A database for one of several calls made at the same moment: the real one, + * noting in `order` each time this call goes to it. + */ +function notingDatabase(call: number, order: NotedCall[]): D1Database { + const real = db(); + let made = 0; + const note = (work: () => Promise): Promise => { + order.push({ call, nth: made }); + made += 1; + return work(); + }; + // A statement handed to `batch` has to be the real one again. + const reals = new WeakMap(); + const noting = (statement: D1PreparedStatement): D1PreparedStatement => { + const wrapped = { + bind: (...values: unknown[]) => noting(statement.bind(...values)), + first: (...columns: string[]) => + note(() => statement.first(...(columns as [string]))), + all: () => note(() => statement.all()), + run: () => note(() => statement.run()), + raw: (...options: unknown[]) => + note(() => statement.raw(...(options as []))), + } as unknown as D1PreparedStatement; + reals.set(wrapped, statement); + return wrapped; + }; + return { + prepare: (sql: string) => noting(real.prepare(sql)), + batch: (statements: D1PreparedStatement[]) => + note(() => + real.batch( + statements.map((statement) => reals.get(statement) ?? statement), + ), + ), + } as unknown as D1Database; +} + +/** + * Runs calls at the same moment, and fails unless they really overlapped: + * every call had gone to the database for the first time — its reading — + * before any of them went a second time — its write. + * + * Functions called side by side in one test do overlap that way, every + * time: each runs as far as its first wait, which is its reading, before + * the next begins. That is what makes a test of a race worth having, and it + * is taken on trust nowhere: a call that came to wait for something else + * before its reading would turn the race into a sequence, the guard under + * test would never be reached, and the test would go on passing. This says + * so instead. + * + * Each call is given a database of its own to use. For requests through + * `SELF.fetch`, which do not overlap of themselves, see {@link holdWrites}. + */ +export async function atTheSameMoment( + calls: readonly ((database: D1Database) => Promise)[], +): Promise[]> { + const order: NotedCall[] = []; + const settled = await Promise.allSettled( + calls.map((call, index) => call(notingDatabase(index, order))), + ); + const firstSecond = order.findIndex((noted) => noted.nth === 1); + const late = calls + .map((_, index) => + order.findIndex((noted) => noted.call === index && noted.nth === 0), + ) + .some( + (first) => first === -1 || (firstSecond !== -1 && first > firstSecond), + ); + if (late) { + throw new Error( + `The calls did not overlap. They went to the database in this order (call.nth): ${order + .map((noted) => `${noted.call}.${noted.nth}`) + .join(' ')}`, + ); + } + return settled; +} + +/** What calls made {@link atTheSameMoment} came to, when none may fail. */ +export function fulfilledValues( + settled: readonly PromiseSettledResult[], +): T[] { + return settled.map((result) => { + if (result.status === 'rejected') { + throw result.reason; + } + return result.value; + }); +} + /** What {@link holdWrites} gives a test to steer by. */ export interface WriteGate { /** diff --git a/test/shop/order-fulfilment.test.ts b/test/shop/order-fulfilment.test.ts index 7eda46a..b6d7056 100644 --- a/test/shop/order-fulfilment.test.ts +++ b/test/shop/order-fulfilment.test.ts @@ -14,6 +14,7 @@ import { OrderNotFoundError, } from '../../src/plugins/shop/lib/orders.js'; import { + atTheSameMoment, clearShopTables, countD1Calls, createProduct, @@ -96,9 +97,10 @@ async function owed(orderId: string): Promise { /** How many of several attempts went through, and why the rest did not. */ async function settle( - attempts: readonly Promise[], + attempts: readonly ((database: D1Database) => Promise)[], ): Promise<{ fulfilled: number; reasons: string[] }> { - const settled = await Promise.allSettled(attempts); + // Made at the same moment, and seen to have overlapped. + const settled = await atTheSameMoment(attempts); return { fulfilled: settled.filter((result) => result.status === 'fulfilled').length, reasons: settled.flatMap((result) => @@ -276,8 +278,8 @@ describe('cancelOrder', () => { const order = await makeOrder(); const outcome = await settle([ - cancelOrder(db(), { orderId: order.id, now: LATER }), - cancelOrder(db(), { orderId: order.id, now: LATER }), + (database) => cancelOrder(database, { orderId: order.id, now: LATER }), + (database) => cancelOrder(database, { orderId: order.id, now: LATER }), ]); expect(outcome.fulfilled).toBe(1); @@ -356,8 +358,8 @@ describe('refundOrder', () => { const order = await paidOrder(); const outcome = await settle([ - refundOrder(db(), { orderId: order.id, now: LATER }), - refundOrder(db(), { orderId: order.id, now: LATER }), + (database) => refundOrder(database, { orderId: order.id, now: LATER }), + (database) => refundOrder(database, { orderId: order.id, now: LATER }), ]); expect(outcome.fulfilled).toBe(1); @@ -370,8 +372,13 @@ describe('refundOrder', () => { const order = await paidOrder(); const outcome = await settle([ - shipOrder(db(), { orderId: order.id, trackingNo: 'T-1', now: LATER }), - refundOrder(db(), { orderId: order.id, now: LATER }), + (database) => + shipOrder(database, { + orderId: order.id, + trackingNo: 'T-1', + now: LATER, + }), + (database) => refundOrder(database, { orderId: order.id, now: LATER }), ]); expect(outcome.fulfilled).toBe(1); diff --git a/test/shop/orders.test.ts b/test/shop/orders.test.ts index 7fb0c76..88ca197 100644 --- a/test/shop/orders.test.ts +++ b/test/shop/orders.test.ts @@ -13,12 +13,14 @@ import { type ShippingAddress, } from '../../src/plugins/shop/lib/orders.js'; import { + atTheSameMoment, clearShopTables, countD1Calls, createProduct, createVariant, db, ensureSite, + fulfilledValues, interceptBatches, outboxRows, setPrice, @@ -517,10 +519,12 @@ describe('markOrderPaid', () => { // order still being pending, so it finds nothing to do. const order = await pendingOrder(); - const results = await Promise.all([ - pay(order.id, 'evt_race'), - pay(order.id, 'evt_race'), - ]); + const results = fulfilledValues( + await atTheSameMoment([ + (database) => pay(order.id, 'evt_race', 'pi_1', database), + (database) => pay(order.id, 'evt_race', 'pi_1', database), + ]), + ); expect(results.map((result) => result.outcome).sort()).toEqual([ 'duplicate', @@ -550,10 +554,12 @@ describe('markOrderPaid', () => { it('decrements once when two events for the same payment arrive at the same moment', async () => { const order = await pendingOrder(); - const results = await Promise.all([ - pay(order.id, 'evt_a', 'pi_same'), - pay(order.id, 'evt_b', 'pi_same'), - ]); + const results = fulfilledValues( + await atTheSameMoment([ + (database) => pay(order.id, 'evt_a', 'pi_same', database), + (database) => pay(order.id, 'evt_b', 'pi_same', database), + ]), + ); expect(results.map((result) => result.outcome).sort()).toEqual([ 'duplicate', @@ -568,10 +574,12 @@ describe('markOrderPaid', () => { // not vanish — it is money to give back. const order = await pendingOrder(); - const results = await Promise.all([ - pay(order.id, 'evt_a', 'pi_first'), - pay(order.id, 'evt_b', 'pi_second'), - ]); + const results = fulfilledValues( + await atTheSameMoment([ + (database) => pay(order.id, 'evt_a', 'pi_first', database), + (database) => pay(order.id, 'evt_b', 'pi_second', database), + ]), + ); expect(results.map((result) => result.outcome).sort()).toEqual([ 'paid', @@ -711,10 +719,12 @@ describe('markOrderPaid', () => { const first = await pendingOrder(); const second = await pendingOrder(); - const results = await Promise.all([ - pay(first.id, 'evt_first', 'pi_first'), - pay(second.id, 'evt_second', 'pi_second'), - ]); + const results = fulfilledValues( + await atTheSameMoment([ + (database) => pay(first.id, 'evt_first', 'pi_first', database), + (database) => pay(second.id, 'evt_second', 'pi_second', database), + ]), + ); expect(results.map((result) => result.outcome).sort()).toEqual([ 'oversold', @@ -785,11 +795,13 @@ describe('markOrderPaid', () => { const order = await pendingOrder(); await cancelOrder(db(), { orderId: order.id, now: NOW }); - const results = await Promise.all([ - pay(order.id, 'evt_late', 'pi_3'), - pay(order.id, 'evt_late', 'pi_3'), - pay(order.id, 'evt_other', 'pi_3'), - ]); + const results = fulfilledValues( + await atTheSameMoment([ + (database) => pay(order.id, 'evt_late', 'pi_3', database), + (database) => pay(order.id, 'evt_late', 'pi_3', database), + (database) => pay(order.id, 'evt_other', 'pi_3', database), + ]), + ); const again = await pay(order.id, 'evt_late', 'pi_3'); expect( @@ -805,22 +817,22 @@ describe('markOrderPaid', () => { it('pays or cancels, never both, when the two happen at the same moment', async () => { const order = await pendingOrder(); - const [payment, cancellation] = await Promise.allSettled([ - pay(order.id), - cancelOrder(db(), { orderId: order.id, now: LATER }), + const [payment, cancellation] = await atTheSameMoment([ + (database) => pay(order.id, 'evt_1', 'pi_1', database), + (database) => cancelOrder(database, { orderId: order.id, now: LATER }), ]); const status = (await orderRow(order.id)).status; if (status === 'paid') { // The payment won: the cancellation found the order moved and refused. expect(payment).toMatchObject({ value: { outcome: 'paid' } }); - expect(cancellation.status).toBe('rejected'); + expect(cancellation?.status).toBe('rejected'); expect(await stockOf('dn50')).toBe(90); } else { // The cancellation won: the payment is recorded for a refund. expect(status).toBe('cancelled'); expect(payment).toMatchObject({ value: { outcome: 'refused' } }); - expect(cancellation.status).toBe('fulfilled'); + expect(cancellation?.status).toBe('fulfilled'); expect(await stockOf('dn50')).toBe(100); } });