docs(specs): specify clojure.test conformance, clojure.test.tap, and stack-trace primitives - #793
docs(specs): specify clojure.test conformance, clojure.test.tap, and stack-trace primitives#793nnunley wants to merge 1 commit into
Conversation
mparrett
left a comment
There was a problem hiding this comment.
I reviewed docs-only head ba70c59 against the current runtime and the Clojure 1.12.5 oracle. CI is green and the document is impressively comprehensive, but I found five blocking correctness/design issues:
-
The trace contract has no viable ownership model for arbitrary thrown values. Sections 5.2-5.3 require
ex-traceto work for every value, distinguish a never-thrown value, preserve a trace on rethrow, and assign a new trace to a newly thrown value. TodayThrownErrorowns only the rawValue, andhandleErrordiscards that wrapper when it pusheserrorToValue(err)into a catch. A side table keyed byValuecannot satisfy the contract: some values (for exampleArrayVector) are not Go-comparable, and equal scalars such as two independent throws of"s"have no identity with which to select the right trace. Please specify the trace-bearing representation/provenance that survives catch and rethrow while preserving the caught value's observable type/equality, and add an acceptance case for two equal scalar values thrown at different sites (including a nested catch). -
The specified
clojure.stacktraceoutput is not the 1.12.5 output and the differences are absent from Appendix A. The oracle prints ex-data on a new line, uses" at "only for the first frame and four spaces for later frames, and prints" at [empty stack trace]"for an empty trace. Sections 5.4 and 16.4 instead put ex-data on the header line, prefix every frame with" at ", omit the empty-stack line, and require twoatlines at depth 2. Please either match the tagged implementation or explicitly document/test each intentional deviation. Oracle: https://github.com/clojure/clojure/blob/clojure-1.12.5/src/clj/clojure/stacktrace.clj#L40-L70 -
The
*test-out*initialization order cannot produce the promised WASM behavior as written. Section 10.2 says bootstrap captures the root after an embedder installs its writer and citespkg/rt/wasm/rendermain.go, but compiler/runtime package initialization (pkg/compiler/init.go) finishes before generatedmain, while that template installsHostWriteronly insidemain. No hook or generated-main update is specified to refresh*test-out*after*out*.SetRoot. The proposed capture therefore freezes the old writer and contradicts the Section 16.9 WASM acceptance case. Please define an explicit post-host-install operation (and require every root-writer replacement path to use it), or have the generated host setup update both roots atomically. -
The report bridge protocol is not self-contained and can misattribute or deadlock valid runs.
ReportEventcarries no snapshot/rendered form of*testing-vars*,*testing-contexts*, or trace output, yetfail_text/error_textread those dynamic vars and invoke let-go formatting on the harness goroutine, contradicting the stated rule that only the runner goroutine touches let-go state. In addition, composed deftests are explicitly supported, butconsume_vartreats nestedBEGIN_TEST_VARas anELSEand returns on the nestedEND_TEST_VAR, so the rest of the outer test escapes its Go subtest. Finally, the goroutine pseudocode has no deferred close/recover/result send; an unexpected panic leavesconsume_eventsblocked forever despite the behavior paragraph promising propagation. Please make the envelope self-contained on the runner goroutine, define nesting/depth handling, and define a deferred terminal result that closes the event stream and transports panics/errors to the harness goroutine. -
print-tap-diagnosticpreserves a trailing empty field that Java's oracle drops. Section 11.1 explicitly useskeep_trailing_empty = true; Clojure 1.12.5 calls JavaString.split("\\n"), which discards trailing empty strings. For example, the oracle renders"a\\n"as only"# a\\n", whereas the specified algorithm adds a second"# \\n"line. This also affects multiline error text captured withwith-out-str. Please use the oracle split semantics and add trailing-newline coverage. Oracle: https://github.com/clojure/clojure/blob/clojure-1.12.5/src/clj/clojure/test/tap.clj#L51-L57
Once these contracts are resolved, the rest of the document gives a strong implementation and acceptance-test roadmap.
…stack-trace primitives Adds docs/specs/clojure-test-conformance.md, an NLSpec-style specification for porting clojure.test to Clojure 1.12.5 semantics so unmodified harnesses (kaocha, cognitect test-runner) can drive let-go: metadata-based test discovery, the report multimethod and do-report, *test-out* and with-test-out, per-namespace fixtures, test-var/test-vars/test-ns, runners that return the summary map, and a port of clojure.test.tap with the plan printed last as the JVM oracle does. It also specifies the runtime pieces the port needs: structured stack frames captured on every throw (including across the Go boundary), current-stack-trace / ex-trace / Throwable->map and a clojure.stacktrace namespace, namespace metadata, exposing the reader's FormSource positions through meta/&form/def, *file*, *out* accepting io/buffer, and a Go harness that first consumes the summary and then bridges report events into testing.T subtests. testing-and-conformance.md yields clojure-test-api-design to the new spec via superseded-by, and the docs README gains a topical-map row.
ba70c59 to
ed0edff
Compare
|
Updated and restacked in All five review blockers are addressed:
The source/oracle audit also corrected canonical Verification on the exact pushed head:
@mparrett, please re-review when convenient. |
mparrett
left a comment
There was a problem hiding this comment.
I reviewed the current restacked head (ed0edff4502e6679b1261c3c9583602e1edf8edc). The earlier five blockers appear substantially addressed, but I found three remaining correctness/design blockers:
-
[P1] Scalar
isforms can fail during macroexpansion. Section 6.2 sends non-sequential forms to:default, but the normative expansion then evaluatesfunction?(first(form))without first checkingsequential?. As written,(is true),(is false),(is :ok), and(is x)can error instead of going throughassert-any. Clojure 1.12.5 uses(and (sequential? form) (function? (first form))). Please restore that guard and add scalar acceptance cases. -
[P1] The trace design still lacks an execution-wide frame carrier.
current-stack-traceandcapture_live_frames(ec, err)require the caller's live frames, butDatumFnreceives onlyExecContext, whose current contract contains bindings and scope rather than a logical frame chain. Interpreter parent links exist only inside one VM run, native-to-Lisp callbacks start another run, and lowered Go has no VMFrameobjects. At the same time,ThrowOccurrenceis immutable and an existing occurrence is never normalized or captured again, so outer native/caller frames cannot be appended after a callback returns. That makes the cross-Go-boundary requirement in Section 5.2(4), and the parity cases in 16.4, unrealizable as specified. Please define an execution-wide logical-frame stack, including interpreter, native, callback, and generated-Go push/pop/finalization rules. -
[P1] The mandatory
no test namespacefailure breaks the existing corpus. The currentTestRunnerwalk includes 18.lgfiles with nonsform: all 16test/gold-aot/*.lgfixtures,test/top_level_do_test.lg, andtest/in_ns_auto_refer_test.lg. The last intentionally uses onlyin-ns, which Section 16.11 explicitly says must leavesaw_ns_formfalse. Implementing Sections 12.2 and 16.11 literally therefore makesgo test ./test/...fail, contradicting the Definition of Done. Please specify whether these are excluded/load-only or enumerate safe namespace migrations for them.
CI is green and git diff --check passes, but I do not think the specification is implementation-safe until these contracts are resolved.
|
Agent-drafted, posted by @mparrett. This continues the 2026-09-04 review on this PR (the five blockers). The 2026-09-08 review came from the same account through a different tool; this comment builds on it rather than restating it. Round-1 blockers: resolvedI re-read the restacked head
Three further findingsAll three are about Section 3.5, and they build on the frame-carrier finding in the 2026-09-08 review rather than competing with it. If the capture side gains a logical-frame stack, the transport side still needs these answered. [P1] The datum carrier widens every typed lowered value, with no stated budget. Section 3.5 makes [P2] Datum-carrying collections change the exported [P3] The datum-preserving set is illustrative rather than closed. Section 3.5 introduces the The existing verdict stands. The corpus finding in the 2026-09-08 review is a real blocker, and none of the three above changes that. |
Adds
docs/specs/clojure-test-conformance.md, an NLSpec-style specification for portingclojure.testto Clojure 1.12.5 semantics so unmodified harnesses (kaocha, cognitect test-runner, #738) can drive let-go: metadata-based test discovery, thereportmultimethod anddo-report,*test-out*/with-test-out, per-namespace fixtures,test-var/test-vars/test-ns, runners that return the summary map, and a port ofclojure.test.tapwith the plan printed last as the JVM oracle does.It also specifies the runtime pieces the port needs: structured stack frames captured on every throw (including across the Go boundary),
current-stack-trace/ex-trace/Throwable->mapand aclojure.stacktracenamespace, namespace metadata, exposing the reader'sFormSourcepositions throughmeta/&form/def,*file*,*out*acceptingio/buffer, and a Go harness that first consumes the summary and then bridgesreportevents intotesting.Tsubtests.It names #673, #671, #754 as the work whose behavior survives and whose mechanism (the registry) it replaces, relies on the typed-catch dispatch from #472/#476 as-is, and lists every deviation from the JVM oracle in an appendix with the oracle transcripts.
testing-and-conformance.mdyieldsclojure-test-api-designto the new spec viasuperseded-by, and the docs README gains a topical-map row. Docs only; no code.Pushed past the pre-push ratchet: it is red on current
mainindependent of this change, see #791.Refs #739 (fully implement the
clojure.test.*namespaces) and #738 (run the cognitect test-runner unmodified). This PR is the specification for both; it does not close them. The implementation lands in the three delivery slices the spec defines (traces; runtime extensions plus theclojure.test/clojure.test.tapport; the Go harness bridge), and the slice that makes the harnesses run will carry the closing reference.