diff --git a/changelog/unreleased/1749.md b/changelog/unreleased/1749.md new file mode 100644 index 000000000..a96aaff7d --- /dev/null +++ b/changelog/unreleased/1749.md @@ -0,0 +1,4 @@ +- **BREAKING CHANGES:** `emergent_testing`: `Reporter.result` now receives the + normalized `TestResult` and `Reporter.summary` one `RunTotals` record; the + new `addResult` and `zeroTotals` fold leaf results into every runner's + totals. Printed output and the browser report are unchanged diff --git a/fjs/emergent_testing/browser.mjs b/fjs/emergent_testing/browser.mjs index 192d3318f..6a7e82f6d 100644 --- a/fjs/emergent_testing/browser.mjs +++ b/fjs/emergent_testing/browser.mjs @@ -19,7 +19,7 @@ * @import { Result } from '../types/result/types.ts' */ -import { collectTests, testResult } from './module.f.mjs' +import { addResult, collectTests, testResult, zeroTotals } from './module.f.mjs' import { error as errorResult, invert, ok } from '../types/result/module.f.mjs' /** @type {(value: unknown) => string} */ @@ -200,13 +200,24 @@ const runOne = (module, path, throws, fn, result) => { return Promise.resolve().then(() => [fn()]).then(([value]) => settled(value), failed) } -/** @type {(status: string, duration: number, results: readonly _BrowserTestResult[]) => BrowserTestReport} */ -const reportOf = (status, duration, results) => { - const failed = results.filter(result => result.status === 'failed').length +/** + * The run-ended event, as the page reports it. The counts — and with them the + * run's own pass/fail status — come from folding the results with the same + * `addResult` that decides `fjs t`'s summary and exit code, so "did the run + * pass" has one answer across the runners. `duration` stays the page's own + * wall clock: leaves run concurrently here, so the fold's summed duration is + * not how long the run took (see `RunTotals`). + * + * `status` overrides the folded decision when the run never got to its leaves + * — module loading failed — which no leaf result can express. + * + * @type {(duration: number, results: readonly _BrowserTestResult[], status?: string) => BrowserTestReport} */ +const reportOf = (duration, results, status = undefined) => { + const { passed, failed } = results.reduce(addResult, zeroTotals) return { - status, + status: status ?? (failed !== 0 ? 'failed' : 'passed'), browser: navigator.userAgent, - totals: { tests: results.length, passed: results.length - failed, failed }, + totals: { tests: results.length, passed, failed }, duration, results, } @@ -215,6 +226,11 @@ const reportOf = (status, duration, results) => { /** * Runs named proof exports and returns the serializable browser report. * + * `result` is the page's subscription to the leaf-landed event — the same + * event `fjs t`'s `Reporter.result` carries, a shared `TestResult` plus the + * browser's own `message`/`stack` part — and the resolved report is its + * run-ended event, with totals folded by the shared `addResult`. + * * @type {(modules: readonly (readonly [string, unknown])[], result?: (result: _BrowserTestResult) => void) => Promise} */ export const runBrowserProofs = (modules, result = () => undefined) => { @@ -259,11 +275,7 @@ export const runBrowserProofs = (modules, result = () => undefined) => { ).then(next => runBatch(index + batchSize, next)) } const completed = runBatch(0, []) - return completed.then(results => reportOf( - results.some(result => result.status === 'failed') ? 'failed' : 'passed', - performance.now() - start, - results, - )) + return completed.then(results => reportOf(performance.now() - start, results)) } /** @typedef {(source: string) => Promise<{ readonly proof?: unknown }>} _BrowserImporter */ @@ -337,11 +349,12 @@ export const startBrowserTestSources = (root, sources, importer) => { // that disagreed with `results` would tell an automated consumer // the suite was empty rather than broken. const duration = performance.now() - start - return publish(root, Promise.resolve(reportOf('infrastructure-error', duration, + return publish(root, Promise.resolve(reportOf(duration, rejected.map(({ source, error }) => { const [message, stack] = errorDetails(error) return moduleFailure(source, duration, message, stack) - })))) + }), + 'infrastructure-error'))) } return startBrowserTests(root, loadedModules.flatMap(module => module.status === 'loaded' diff --git a/fjs/emergent_testing/module.f.mjs b/fjs/emergent_testing/module.f.mjs index 9421738ef..06cef624a 100644 --- a/fjs/emergent_testing/module.f.mjs +++ b/fjs/emergent_testing/module.f.mjs @@ -3,7 +3,7 @@ * * Two parallel execution paths: * - `runModule` / `Reporter` — self-hosted Effects runner used by `fjs t`; - * sandboxes each leaf call individually and accumulates `TestState`. + * sandboxes each leaf call individually and accumulates `RunTotals`. * - `registerModule` / `TestContext` — registers tests with an external * framework (Node `--test`, Bun, Deno) at import time; the framework owns * scheduling and pass/fail counting. @@ -13,7 +13,7 @@ * @import { Operation } from '../effects/types.ts' * @import { Effect, NotImplemented } from '../effects/types.ts' * @import { LoadModuleOperations, ModuleMap } from '../dev/types.ts' - * @import { TestFn, TestEntry, TestSet, Path, Reporter, TestResult, _TestState, _TestAndPath } from './types.ts' + * @import { TestFn, TestEntry, TestSet, Path, Reporter, RunTotals, TestResult, _TestAndPath } from './types.ts' * @import { All, Await, Env, IoChannel, NodeProgram, NodeProgramOptions, Program, Sandbox, SandboxResult, Test, TestContext, Write, WriteConsoles } from '../effects/node/types.ts' */ @@ -26,13 +26,28 @@ import { loadModuleMap } from '../dev/module.f.mjs' import { invert } from '../types/result/module.f.mjs' import { definedEntries } from '../types/object/module.f.mjs' -/** @type {(delta: number) => (ts: _TestState) => _TestState} */ -const addPass = delta => ts => - ({ ...ts, time: ts.time + delta, pass: ts.pass + 1 }) +/** + * The empty {@link RunTotals}: what a run's totals are before any leaf lands. + * + * @type {RunTotals} + */ +export const zeroTotals = { passed: 0, failed: 0, duration: 0 } -/** @type {(delta: number) => (ts: _TestState) => _TestState} */ -const addFail = delta => ts => - ({ ...ts, time: ts.time + delta, fail: ts.fail + 1 }) +/** + * Folds one leaf-landed event into a run's totals. + * + * This is where "did the run pass" is decided, for every runner: the counts + * come from each result's shared `status`, so the summary line, the exit code + * and the browser report's totals all read the same fold of the same events + * rather than each counting their own way. + * + * @type {(totals: RunTotals, r: TestResult) => RunTotals} + */ +export const addResult = (totals, r) => ({ + passed: totals.passed + (r.status === 'passed' ? 1 : 0), + failed: totals.failed + (r.status === 'failed' ? 1 : 0), + duration: totals.duration + r.duration, +}) /** @type {(a: number) => string} */ const timeFormat = a => { @@ -154,49 +169,49 @@ export const registerModule = (ctx, k, v, star) => { return mapStep(allOk(...tests.map(e => registerOne(ctx, e))), () => undefined) } -/** @type {(a: _TestState, b: _TestState) => _TestState} */ -const mergeState = (a, b) => - ({ time: a.time + b.time, pass: a.pass + b.pass, fail: a.fail + b.fail }) - -/** @type {_TestState} */ -const zero = { time: 0, pass: 0, fail: 0 } +/** @type {(a: RunTotals, b: RunTotals) => RunTotals} */ +const mergeTotals = (a, b) => + ({ passed: a.passed + b.passed, failed: a.failed + b.failed, duration: a.duration + b.duration }) /** * @template {Operation} O * @param {Reporter} reporter - * @returns {(k: string, v: unknown) => (ts: _TestState) => Effect} + * @returns {(k: string, v: unknown) => (ts: RunTotals) => Effect} */ const runModule = ({ result, test }) => (k, v) => ts => { - /** @type {(entry: _TestAndPath) => Effect} */ + /** @type {(entry: _TestAndPath) => Effect} */ const one = ([testPath, set]) => { - // The sandbox result is still needed after it has been reported, so the - // reporting call is captured rather than nested inside its own step. + // The leaf's shared record is built here, next to the sandbox result it + // is read from, so the leaf-landed event carries the value already + // decided — a reporter renders `t`, it does not derive its own. + const evaluated = mapStep( + test(k, testPath, set), + sr => /** @type {const} */ ([testResult(k, testPath, sr), sr])) + // Both are still needed after they have been reported, so the reporting + // call is captured rather than nested inside its own step. const reported = historyStep( - history(test(k, testPath, set)), - sr => result(k, testPath, sr, set.throws)) + history(evaluated), + ([t, sr]) => result(t, sr, set.throws)) return step( reported, - ([, sr]) => { - const { result: [s, r], duration } = sr - if (s !== 'ok') { - return pureOk(addFail(duration)(zero)) - } - if (set.throws) { - return pureOk(addPass(duration)(zero)) + ([, [t, sr]]) => { + const total = addResult(zeroTotals, t) + if (t.status !== 'passed' || set.throws) { + return pureOk(total) } // Walk return-value sub-tree; null marks the call boundary so // paths render as e.g. `outer().inner`. throws resets to false. return mapStep( - walk([...testPath, null], false, r), - sub => mergeState(addPass(duration)(zero), sub)) + walk([...testPath, null], false, sr.result[1]), + sub => mergeTotals(total, sub)) }) } - /** @type {(path: Path, throws: boolean, v: unknown) => Effect} */ + /** @type {(path: Path, throws: boolean, v: unknown) => Effect} */ const walk = (path, throws, v) => { const effects = collectTests(path, throws, v).map(one) - return mapStep(allOk(...effects), states => states.reduce(mergeState, zero)) + return mapStep(allOk(...effects), states => states.reduce(mergeTotals, zeroTotals)) } - return mapStep(walk([], false, v), delta => mergeState(ts, delta)) + return mapStep(walk([], false, v), delta => mergeTotals(ts, delta)) } /** @type {(moduleMap: ModuleMap) => readonly (readonly [string, unknown])[]} */ @@ -217,15 +232,15 @@ export const runModuleMap = reporter => moduleMap => { const { summary } = reporter const modules = proofEntries(moduleMap) const total = mapStep( - allOk(...modules.map(([k, v]) => runModule(reporter)(k, v)(zero))), - m => m.reduce(mergeState, zero)) + allOk(...modules.map(([k, v]) => runModule(reporter)(k, v)(zeroTotals))), + m => m.reduce(mergeTotals, zeroTotals)) // The totals are still needed after the summary has been printed, so they // are carried forward in a history rather than closed over by a nested // continuation. const reported = historyStep( history(total), - ts => summary(ts.pass, ts.fail, ts.time)) - return mapStep(reported, ([, ts]) => ts.fail !== 0 ? 1 : 0) + summary) + return mapStep(reported, ([, ts]) => ts.failed !== 0 ? 1 : 0) } /** @@ -423,13 +438,12 @@ export const defaultReporter = options => { const isGitHub = options.env['GITHUB_ACTIONS'] !== undefined return { // https://github.com/OndraM/ci-detector/blob/main/src/Ci/GitHubActions.php - result: (file, path, r, throws) => { - const t = testResult(file, path, r) + result: (t, r, throws) => { const v = r.result[1] return t.status === 'passed' ? csiLog(fmtResultLine(t, fgGreen, 'ok') + (throws ? ' # EXPECTED TO THROW' : '')) : isGitHub - ? csiError(`::error file=${file},line=1,title=${ghEscape(t.name)}::${ghEscape(String(v))}`) + ? csiError(`::error file=${t.module},line=1,title=${ghEscape(t.name)}::${ghEscape(String(v))}`) // `step`, so the detail line is attempted only when the // header line was written: two halves of one report, and // half of it is worse than none. @@ -437,11 +451,11 @@ export const defaultReporter = options => { csiError(fmtResultLine(t, fgRed, 'error')), () => csiError(`${fgRed}${v}${reset}`)) }, - summary: (pass, fail, time) => { - const fgFail = fail === 0 ? fgGreen : fgRed + summary: ({ passed, failed, duration }) => { + const fgFail = failed === 0 ? fgGreen : fgRed return step( - csiLog(`${bold}Number of tests: pass: ${fgGreen}${pass}${reset}${bold}, fail: ${fgFail}${fail}${reset}${bold}, total: ${pass + fail}${reset}`), - () => csiLog(`${bold}Time: ${timeFormat(time)}${reset}`)) + csiLog(`${bold}Number of tests: pass: ${fgGreen}${passed}${reset}${bold}, fail: ${fgFail}${failed}${reset}${bold}, total: ${passed + failed}${reset}`), + () => csiLog(`${bold}Time: ${timeFormat(duration)}${reset}`)) }, test: defaultTest, } diff --git a/fjs/emergent_testing/proof.f.mjs b/fjs/emergent_testing/proof.f.mjs index a6abb8539..55c1b7694 100644 --- a/fjs/emergent_testing/proof.f.mjs +++ b/fjs/emergent_testing/proof.f.mjs @@ -15,12 +15,12 @@ import { assert, assertEq, todo } from '../asserts/module.f.mjs' import { testAll, fmtPath, fmtTerm, fmtImport, ghEscape, isInteger, isIdentifier, registerModule, parseTestSet, - defaultTest, main, register, testResult, + addResult, defaultTest, main, register, testResult, zeroTotals, } from './module.f.mjs' import { run as mockRun } from '../effects/mock/module.f.mjs' import { shouldLoad } from '../dev/module.f.mjs' import { parse as parseJson } from '../media/json/module.f.mjs' -import { array, number as rttiNumber, or, string as rttiString } from '../rtti/module.f.mjs' +import { number as rttiNumber, or, string as rttiString } from '../rtti/module.f.mjs' import { parse as rttiParse } from '../rtti/parse/module.f.mjs' import { error, ok, unwrap } from '../types/result/module.f.mjs' @@ -35,7 +35,7 @@ import { error, ok, unwrap } from '../types/result/module.f.mjs' * JSON representation to round-trip through anyway. */ const event = or( - /** @type {const} */ (['result', rttiString, array(or(rttiString, null))]), + /** @type {const} */ (['result', rttiString, rttiString]), /** @type {const} */ (['summary', rttiNumber, rttiNumber, rttiNumber]), ) @@ -55,8 +55,12 @@ const parseEvents = stdout => /** @type {() => _TestReporter} */ const makeReporter = () => ({ - result: (file, path, _r, _throws) => writeEvent(['result', file, [...path]]), - summary: (pass, fail, time) => writeEvent(['summary', pass, fail, time]), + // The leaf-landed event arrives with the shared `TestResult` already + // built, so what this writes — and what the proofs below assert on — is + // the record's own `module` and formatted `path`, not a spelling of the + // mock's own. + result: (t, _r, _throws) => writeEvent(['result', t.module, t.path]), + summary: ({ passed, failed, duration }) => writeEvent(['summary', passed, failed, duration]), test: defaultTest, }) @@ -98,8 +102,8 @@ export const flat = () => { }) assertEq(exit, 0) const [e0, e1, e2] = events - assert(e0[0] === 'result' && e0[2][0] === 'a') - assert(e1[0] === 'result' && e1[2][0] === 'b') + assert(e0[0] === 'result' && e0[2] === '.a') + assert(e1[0] === 'result' && e1[2] === '.b') assert(e2[0] === 'summary') const [, pass, fail] = e2 assertEq(pass, 2) @@ -113,8 +117,8 @@ export const nested = () => { }) assertEq(exit, 0) const [e0, e1, e2] = events - assert(e0[0] === 'result' && e0[2][1] === 'add') - assert(e1[0] === 'result' && e1[2][1] === 'sub') + assert(e0[0] === 'result' && e0[2] === '.math.add') + assert(e1[0] === 'result' && e1[2] === '.math.sub') assert(e2[0] === 'summary') const [, pass, fail] = e2 assertEq(pass, 2) @@ -128,7 +132,7 @@ export const throwKey = () => { }) assertEq(exit, 0) const [e0, e1] = events - assert(e0[0] === 'result' && e0[2][0] === 'throw' && e0[2][1] === 'a') + assert(e0[0] === 'result' && e0[2] === '.throw.a') assert(e1[0] === 'summary') const [, pass, fail] = e1 assertEq(pass, 1) @@ -180,8 +184,8 @@ export const returnValueSubTree = () => { const passEvents = events.filter(e => e[0] === 'result') assertEq(passEvents.length, 2) const [p0, p1] = passEvents - assertEq(p0[2][0], 'outer') - assertEq(p1[2][2], 'inner') + assertEq(p0[2], '.outer') + assertEq(p1[2], '.outer().inner') } // integer-indexed array keys appear as numeric path segments @@ -192,8 +196,8 @@ export const arrayKeys = () => { assertEq(exit, 0) const passEvents = events.filter(e => e[0] === 'result') assertEq(passEvents.length, 2) - assertEq(passEvents[0][2][1], '0') - assertEq(passEvents[1][2][1], '1') + assertEq(passEvents[0][2], '.arr[0]') + assertEq(passEvents[1][2], '.arr[1]') } // non-proof files are skipped: plain `.ts` is not loaded; `.f.ts` without @@ -235,7 +239,7 @@ export const throwByFunctionName = () => { assertEq(exit, 0) const passEvents = events.filter(e => e[0] === 'result') assertEq(passEvents.length, 1) - assertEq(passEvents[0][2][0], 'here') + assertEq(passEvents[0][2], '.here') } // only the `proof` export is used; other module properties are ignored @@ -246,8 +250,8 @@ export const namedExports = () => { assertEq(exit, 0) const passEvents = events.filter(e => e[0] === 'result') assertEq(passEvents.length, 2) // `other` is ignored - assertEq(passEvents[0][2][0], 'a') - assertEq(passEvents[1][2][0], 'b') + assertEq(passEvents[0][2], '.a') + assertEq(passEvents[1][2], '.b') } // the default (non-GitHub) reporter formats module/pass/summary lines on stdout @@ -667,8 +671,31 @@ const testResultProofs = { }, } +/** + * `addResult` is where every runner turns a stream of leaf results into the + * run's totals — the summary line, the exit code and the browser report's + * counts all read this fold — so the fold itself is pinned here, not only its + * end-to-end effects. + */ +const runTotalsProofs = { + startsEmpty: () => { + assertEq(zeroTotals.passed, 0) + assertEq(zeroTotals.failed, 0) + assertEq(zeroTotals.duration, 0) + }, + countsByTheSharedStatus: () => { + const pass = testResult('./a.f.mjs', ['x'], { result: ok(1), duration: 0.5 }) + const fail = testResult('./a.f.mjs', ['y'], { result: error('boom'), duration: 2 }) + const totals = [pass, fail, pass].reduce(addResult, zeroTotals) + assertEq(totals.passed, 2) + assertEq(totals.failed, 1) + assertEq(totals.duration, 3) + }, +} + export const proof = { testResult: testResultProofs, + runTotals: runTotalsProofs, throw: { registerBodyPanicsOnUndispatchableEffect, }, diff --git a/fjs/emergent_testing/todo/65z-tf-test-tree-walker.md b/fjs/emergent_testing/todo/65z-tf-test-tree-walker.md index 8b47befad..622b07be3 100644 --- a/fjs/emergent_testing/todo/65z-tf-test-tree-walker.md +++ b/fjs/emergent_testing/todo/65z-tf-test-tree-walker.md @@ -23,25 +23,23 @@ const registerOne = (ctx: TestContext, [path, { fn, throws }]: TestAndPath) => return all(...sub.map(e => registerOne(t, e))).step(() => pure(undefined)) })) -// runModule (./fjs/emergent_testing/module.f.mjs:167) -const one = ([testPath, set]: TestAndPath): Effect => +// runModule (./fjs/emergent_testing/module.f.mjs) +const one = ([testPath, set]: TestAndPath): Effect => test(k, testPath, set) .step(sr => { - const { result: [s, r], duration } = sr - return result(k, testPath, sr) - .step((): Effect => { - if (s === 'ok') { - if (set.throws) { return pure(addPass(duration)(zero)) } - return walk([...testPath, null], false, r) - .step(sub => pure(mergeState(addPass(duration)(zero), sub))) - } - return pure(addFail(duration)(zero)) + const t = testResult(k, testPath, sr) + return result(t, sr, set.throws) + .step((): Effect => { + const total = addResult(zeroTotals, t) + if (t.status !== 'passed' || set.throws) { return pure(total) } + return walk([...testPath, null], false, sr.result[1]) + .step(sub => pure(mergeTotals(total, sub))) }) }) -const walk = (path: Path, throws: boolean, v: unknown): Effect => { +const walk = (path: Path, throws: boolean, v: unknown): Effect => { const effects = collectTests(path, throws, v).map(one) return all(...effects) - .step(states => pure(states.reduce(mergeState, zero))) + .step(states => pure(states.reduce(mergeTotals, zeroTotals))) } ``` @@ -93,7 +91,7 @@ export const walkTests = (w: Walker) => { } ``` -`runModule` instantiates `S = TestState`, threads `Sandbox`/`Reporter` effects +`runModule` instantiates `S = RunTotals`, threads `Sandbox`/`Reporter` effects in `onLeaf`, and returns the sub-tree value on success-without-`throws`. `registerModule` instantiates `S = void` for surviving process adapters, registers through @@ -136,8 +134,8 @@ shares the semantics rather than the obsolete Playwright registration path. `onLeaf` may need to return a "child context" alongside the accumulator. This may complicate the signature enough that the abstraction stops feeling like a win; a small spike will tell. -- `runModule` measures per-leaf `duration` from `SandboxResult` and folds it - into `TestState`; `registerModule` doesn't care. The walker must not +- `runModule` builds each leaf's `TestResult` and folds it into `RunTotals` + with `addResult`; `registerModule` doesn't care. The walker must not pretend to own this — it stays inside `onLeaf`. - Browser execution has no `TestContext` and must not import the Node effect runner. Share browser-compatible code only when it keeps the page independent from Node and diff --git a/fjs/emergent_testing/todo/66a-emergent-add-result.md b/fjs/emergent_testing/todo/66a-emergent-add-result.md deleted file mode 100644 index 043b98cd6..000000000 --- a/fjs/emergent_testing/todo/66a-emergent-add-result.md +++ /dev/null @@ -1,79 +0,0 @@ -## 66A-emergent-add-result. Merge `addPass` / `addFail` into one `TestState` updater - -**Priority:** P5 -**Status:** open - -### Problem - -`fjs/emergent_testing/module.f.mjs` defines two `TestState` updaters that are -identical except for the counter field they increment: - -```ts -// fjs/emergent_testing/module.f.mjs:40-46 -const addPass = (delta: number) => (ts: TestState): TestState => - ({ ...ts, time: ts.time + delta, pass: ts.pass + 1 }) - -const addFail = (delta: number) => (ts: TestState): TestState => - ({ ...ts, time: ts.time + delta, fail: ts.fail + 1 }) -``` - -where - -```ts -// :37-41 -type TestState = { - readonly time: number, - readonly pass: number, - readonly fail: number, -} -``` - -The two bodies share the spread, the `time: ts.time + delta` accumulation, and -the `+ 1` increment; they differ only in whether `pass` or `fail` is the -incremented key. This is the "same algorithm, one varying constant" shape that -DRY targets — and if a third outcome counter were ever added (e.g. `skip`), the -copy would multiply. - -Both helpers are real, exercised code: `addPass(duration)(zero)` / -`addFail(duration)(zero)` feed the `runModule` walk -(`fjs/emergent_testing/module.f.mjs:180-190`). - -### Proposal - -Parameterize over the counter key with a typed computed property, keeping the -type checker's exhaustiveness (`'pass' | 'fail'` is a closed union, so a typo -is a compile error): - -```ts -const addResult = (key: 'pass' | 'fail') => (delta: number) => (ts: TestState): TestState => - ({ ...ts, time: ts.time + delta, [key]: ts[key] + 1 }) - -const addPass = addResult('pass') -const addFail = addResult('fail') -``` - -The two named helpers are kept as point-free derivations so every call site -(`addPass(duration)(zero)`, `addFail(duration)(zero)`) is unchanged and still -reads at the grammar level. No `as` cast is needed — `ts[key]` is `number` for -both members of the union, and the computed-key literal is checked against the -`TestState` shape. - -This is a small, single-module change. It is borderline against the AGENTS.md -DRY-vs-readability guidance (the originals are short and clear), which is why -it is filed at **P5** — worth doing if the file is being touched anyway, or as -a prerequisite if a third counter is introduced, but not on its own. - -### Tasks - -- [ ] Replace `addPass` / `addFail` with the `addResult` factory + two - derivations in `fjs/emergent_testing/module.f.mjs`. -- [ ] Confirm `fjs/emergent_testing` proofs still pass (`fjs t`) with full - branch coverage and `npx tsc` is clean. - -### Related - -- [i65Z-tf-test-tree-walker](./65z-tf-test-tree-walker.md) — adjacent - `fjs/emergent_testing` DRY cleanup (sharing the dynamic test-tree walk between - `runModule` and `registerModule`). Same module; independent change. Note that - walker also consumes `addPass`/`mergeState`, so landing this first keeps the - updater surface stable for that refactor. diff --git a/fjs/emergent_testing/todo/report-before-running.md b/fjs/emergent_testing/todo/report-before-running.md index 37d8fff98..1a0be4351 100644 --- a/fjs/emergent_testing/todo/report-before-running.md +++ b/fjs/emergent_testing/todo/report-before-running.md @@ -24,8 +24,11 @@ Three things follow from that, and the third is the one that matters: the case where a name is worth more than a result, and it is the case where the current design has none. -No reporter has an event for it: `result` is called with a `SandboxResult`, so it -cannot be called before there is one. +No reporter has an event for it: `result` is called with a finished +`TestResult` and the `SandboxResult` it was read from, so it cannot be called +before there is one. The seam it would travel through does exist now — both +runners report through the same leaf-landed and run-ended events — so adding a +start event is adding a third event kind, not building the stream first. ### Preliminary design diff --git a/fjs/emergent_testing/todo/share-browser-console-runner.md b/fjs/emergent_testing/todo/share-browser-console-runner.md index c1297405e..0bb159d66 100644 --- a/fjs/emergent_testing/todo/share-browser-console-runner.md +++ b/fjs/emergent_testing/todo/share-browser-console-runner.md @@ -131,13 +131,29 @@ and is reviewable without the next one. move with them. - [ ] **5. A browser interpreter** for exactly those operations, with no scheduling policy of its own. -- [ ] **6. One reporter.** The event stream — a leaf landed, a run ended — - that both hosts subscribe to. Step 2 gave them the *value*; this gives - them the seam it travels through, and it is what +- [x] **6. One reporter.** The event stream — a leaf landed, a run ended — + that both hosts subscribe to. Step 2 gave them the *value*; this gave + them the seam it travels through. `Reporter.result` now receives the + shared `TestResult` built by the runner instead of raw material every + reporter normalized for itself, and the run-ended event is `RunTotals`, + folded from the leaf results by one `addResult` — the summary line, the + exit code and the browser report's counts and pass/fail status all read + that same fold. This is what [report a test's name before running it](report-before-running.md) - needs before a start event can exist. + needed before a start event could exist: adding one is now a third event + kind on an existing stream. What stayed each host's own, deliberately: + the raw `SandboxResult` still travels next to the `TestResult`, because + describing a *thrown value* is each host's part (step 2's finding); and + the browser report's own `duration` stays wall-clock rather than the + fold's summed durations, because its leaves run concurrently and the sum + only means "how long the run took" for a sequential runner — + `RunTotals` documents that. - [ ] **7. One skeleton.** The page's proof-tree walk is deleted and the shared - traversal runs it. + traversal runs it. The walk's `batchSize = 25` batching goes onto the + table with it: that is a scheduling policy of the page's own — the same + kind the reverted attempt was faulted for inventing, though this one + predates it in `browser.mjs` — and step 7 is where it gets decided + rather than silently inherited. - [ ] **8. The layout move**, and the website preparation program. Steps 3 and 7 are the ones that change behaviour, so they are the ones to keep @@ -160,12 +176,25 @@ alone on purpose: step 2 shares; they disagree on the message, which belongs with the point above. -Note also that `testResult` now sits inside `fjs t`'s own reporting path, so a -defect in it can mislabel the very failures it causes — a mutation forcing every -status to `passed` prints `ok` on failing lines. The pass/fail counts come from -the walk's state rather than from the reporter, so they stay honest and the -summary still reports the failures. Worth remembering when reading output while -changing this function. +Note also that `testResult` and `addResult` now sit inside `fjs t`'s own +reporting path: since step 6 the result lines, the summary counts and the exit +code all read them, so a defect there can mislabel or miscount the very +failures it causes — and **`fjs t` alone cannot see that**. The direct proofs +that pin both functions are themselves reported through the functions they +test: mutate `testResult` to answer `passed` for everything and the proof that +asserts `failed` does fail, but its failure is relabelled `ok` on the way out — +measured, the mutated suite prints 3480 pass, exit 0. Mutate the fold to never +count a failure and the gate (`failed !== 0`) reads the fold it is gating — +exit 0 again, with the total quietly short. That is not a duplicate-decision +problem to fix with a second count (the second count is what step 6 removed); +it is a runner auditing itself, which no arrangement of its own proofs escapes. +What actually holds the line is the *other* execution path: `all.test.mjs` +registers every proof with an external framework (`register`, which consults +neither `testResult` nor `addResult` — a deliberate independence, worth +keeping), and CI runs it under node, bun and deno. Both mutants above fail +there — 16 and 18 failures, exit 1. So a reporter defect shows up as `fjs t` +disagreeing with the external runners, never as every gate lying together — +and `fjs t`'s own exit code is trustworthy only in that company. ### Why the remaining steps are worth taking diff --git a/fjs/emergent_testing/todo/skip-property.md b/fjs/emergent_testing/todo/skip-property.md index 7578993ce..806a62e47 100644 --- a/fjs/emergent_testing/todo/skip-property.md +++ b/fjs/emergent_testing/todo/skip-property.md @@ -110,7 +110,7 @@ option on the test effect used by the surviving process-based adapters: zero-arg function is still a leaf; generators are not expanded (they are leaves that never run). - **`runModule`** — when `entry.skip`, do not call `test`; report a skipped - result and increment the `skip` counter in `TestState` (no `pass`/`fail` + result and increment the `skip` counter in `RunTotals` (no `passed`/`failed` change, no return-value walk). - **`registerModule`** — register skipped leaves with a `skip` flag instead of a test body; no subtest registration, no ` ...` star suffix. @@ -150,7 +150,7 @@ Playwright execution obtains skip results from the shared browser application. - [ ] Add `skip` to `TestEntry`; inherit it in `parseTestSet` / `collectTests` like `throws`. - [ ] Short-circuit skipped leaves in `runModule` (no execution, no walk) and - count them in a new `TestState.skip`. + count them in a new `RunTotals.skip` (folded in `addResult`). - [ ] Extend the test-effect options with `skip`; map it to Node, Deno, and Bun without adding or restoring a Playwright branch in the Node effect runner. - [ ] Implement equivalent skip collection and reporting in the shared diff --git a/fjs/emergent_testing/todo/todo-property.md b/fjs/emergent_testing/todo/todo-property.md index fa2b9f4d1..b59a811db 100644 --- a/fjs/emergent_testing/todo/todo-property.md +++ b/fjs/emergent_testing/todo/todo-property.md @@ -186,12 +186,13 @@ docs, mirroring the existing `throws` plumbing: instead of a bare `throws` boolean) so `defaultReporter` can annotate a passing leaf with `# TODO` when `todo` is set, otherwise keep `# EXPECTED TO THROW`. The GitHub/error path is unchanged. -- **`TestState` / summary (`fjs t` only)** — add a `todo` counter to `TestState`, - incremented for every `todo` leaf in `runModule` (independently of whether it - passed or failed; its pass/fail still increments `pass`/`fail` as usual). - Extend `Reporter.summary` to receive the `todo` count and have `defaultReporter` - print it (`pass / fail / todo`). The `register` path is unchanged — no - cross-test tally there. +- **`RunTotals` / summary (`fjs t` only)** — add a `todo` counter to + `RunTotals`, folded in `addResult` for every `todo` leaf (independently of + whether it passed or failed; its pass/fail still increments + `passed`/`failed` as usual). `Reporter.summary` already receives the whole + record, so no signature change — `defaultReporter` just prints the new field + (`pass / fail / todo`). The `register` path is unchanged — no cross-test + tally there. - **No `fn.name === 'todo'` check** — `todo` is structural-key-only, matching the guidance around the legacy `fn.name === 'throw'` path. @@ -228,7 +229,7 @@ out from under `throw`) as part of landing this change. skipping + star suffix to `leafOnly` (`runModule`, `registerModule`). - [ ] Update `Reporter.result` to receive the entry flags and annotate passing `todo` leaves with `# TODO`. -- [ ] Add a `todo` counter to `TestState`/`Reporter.summary` and print +- [ ] Add a `todo` counter to `RunTotals` (folded in `addResult`) and print `pass / fail / todo` in `defaultReporter` (`fjs t` only; `register` unchanged). - [ ] Migrate `fjs/emergent_testing/example.f.mjs` off `throw: { todo }`. diff --git a/fjs/emergent_testing/types.ts b/fjs/emergent_testing/types.ts index ed5888d7d..20ac2f6c9 100644 --- a/fjs/emergent_testing/types.ts +++ b/fjs/emergent_testing/types.ts @@ -96,13 +96,31 @@ export type TestResult = { readonly duration: number } +/** + * A run's outcome, folded from its leaf results: how many passed, how many + * failed, and how long they took together. + * + * It is built one `TestResult` at a time with `addResult`, starting from + * `zeroTotals`, so a stream of leaf-landed events and a finished totals record + * are the same information at two moments — which is what lets both runners + * answer "did the run pass" (`failed !== 0`) from the same fold. + * + * `duration` is the *sum* of the folded results' durations. For `fjs t`, which + * runs leaves sequentially, that is also the run's time and is what its + * `Time:` line prints. The browser runs leaves concurrently, so the sum stops + * meaning "how long the run took" there; its wire report keeps its own + * wall-clock `duration` and takes only the counts from the fold. + */ +export type RunTotals = { + readonly passed: number + readonly failed: number + readonly duration: number +} + /** * Receives semantic test-run events. Each method is the runner's notification * of an event; the reporter decides how to render it (terminal, GitHub - * annotations, JSON, node `--test`, etc.). `path` is the chain of object keys - * leading to the current location; `null` marks a function-call boundary, e.g. - * `['outer', null, 'inner']` means `outer` was invoked and its return value - * contained `inner`. + * annotations, JSON, node `--test`, etc.). * * **Every method is fallible**, because reporting is IO and IO can fail: a * write to a closed pipe, a runner that cannot dispatch `write` at all. The @@ -123,17 +141,19 @@ export type TestResult = { * through unchanged. */ export type Reporter = { - readonly result: (file: string, path: Path, r: SandboxResult, throws: boolean) => Effect - readonly summary: (pass: number, fail: number, time: number) => Effect + /** + * A leaf landed. The first argument is the shared {@link TestResult} — the + * runner builds it with `testResult` before notifying, so a reporter + * receives the leaf's identity and status rather than deriving its own. + * The raw `SandboxResult` and the throw expectation travel with it because + * describing a *thrown value* is each host's part (see {@link TestResult}), + * and the description needs the value. + */ + readonly result: (t: TestResult, r: SandboxResult, throws: boolean) => Effect + /** The run ended, with the totals folded from every leaf that landed. */ + readonly summary: (totals: RunTotals) => Effect readonly test: (file: string, path: Path, set: TestEntry) => Effect, IoChannel> } -/** @internal */ -export type _TestState = { - readonly time: number, - readonly pass: number, - readonly fail: number, -} - /** @internal */ export type _TestAndPath = readonly [Path, TestEntry] diff --git a/fjs/types/todo/66b-sorted-list-cmp-reduce-factory.md b/fjs/types/todo/66b-sorted-list-cmp-reduce-factory.md index d781f8e23..a148ddabc 100644 --- a/fjs/types/todo/66b-sorted-list-cmp-reduce-factory.md +++ b/fjs/types/todo/66b-sorted-list-cmp-reduce-factory.md @@ -66,10 +66,12 @@ between "merge" and "intersect". This is borderline against the `AGENTS.md` "readability over DRY for short, clear functions" guidance — the originals are three lines each and already readable, and the `select` callback adds an indirection a reader must follow. It is the -same caliber as [i66A-emergent-add-result](../../emergent_testing/todo/66a-emergent-add-result.md) (two -near-identical updaters differing in one slot), filed at the same low priority: +"two near-identical updaters differing in one slot" caliber, filed low for it: worth doing if the file is being touched anyway, or as a prerequisite if a third sign-driven merge reducer is added (e.g. set difference), but not on its own. +(`emergent_testing` had the same shape in its `addPass`/`addFail` pair until a +runner change dissolved the pair into one fold rather than parameterizing it — +which is also a way this kind of issue resolves.) ### Tasks @@ -81,5 +83,3 @@ sign-driven merge reducer is added (e.g. set difference), but not on its own. - i180-sorted-set-intersect-symmetry — adjacent sorted-collection merge/intersect cleanup. -- [i66A-emergent-add-result](../../emergent_testing/todo/66a-emergent-add-result.md) — the same - "two updaters differing in one slot" pattern, filed at the same priority.