Skip to content

Commit f7d61b7

Browse files
committed
Document known cljs.test implementation gaps
Inventory of behavior our built-in cljs.test diverges from canonical clojure.test/cljs.test. Grouped by impact (correctness, extensibility, coverage, polish) with a fix sketch per item so they can be tackled incrementally.
1 parent f34f358 commit f7d61b7

1 file changed

Lines changed: 124 additions & 0 deletions

File tree

doc/cljs-test-todo.md

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
# cljs.test / clojure.test: known gaps in squint's implementation
2+
3+
This list captures behavior the squint (and cherry) cljs.test built-ins
4+
don't yet match against the canonical `clojure.test` / `cljs.test`. The
5+
ordering is rough priority — high-impact correctness gaps first, then
6+
extensibility gaps, then polish.
7+
8+
## Correctness — likely to bite real users
9+
10+
### 1. Fixtures are global, not per-namespace
11+
`set-each-fixtures!` / `set-once-fixtures!` write into the single global
12+
env. Real `clojure.test` keys fixtures by namespace (`(alter-meta! ns ...)`).
13+
Today, two test namespaces each calling `(use-fixtures :each ...)` will
14+
clobber each other; whichever loaded last wins for *every* test.
15+
16+
**Fix sketch:** key `:each-fixtures` / `:once-fixtures` by ns string in the
17+
env; `core-use-fixtures` macro captures `(&env :ns :name str)` and
18+
includes it in the `set-*-fixtures!` call; `test-var` looks the test fn's
19+
ns up via the same metadata `register-test!` already attaches, then
20+
fetches the right fixture vector.
21+
22+
### 2. run-tests doesn't reset counters per call
23+
`(run-tests)` auto-inits the env only when `*current-env*` is `nil`. A
24+
second `(run-tests)` in the same module reuses (and inflates) the
25+
existing counters. Tests that themselves invoke `run-tests` (squint and
26+
cherry's own runtime tests do this) emit summaries showing cumulative
27+
counts, not per-run counts.
28+
29+
**Fix sketch:** snapshot counters at start, restore (or delta) before
30+
emitting summary. Or always init a fresh env at the top of `run-tests`
31+
unless the caller opts out.
32+
33+
### 3. Quoted-symbol emission breaks `is` reporting
34+
The shared macro `(:name '~name)` would expand to `cljs.core.symbol(...)`
35+
under squint, which doesn't exist at runtime. We worked around it by
36+
storing `:name` as a string (and `assert-expr` uses `(pr-str form)`).
37+
Result: `(:name (meta test-fn))` is a string, not a symbol — diverges
38+
from `clojure.test`'s expectations and breaks code that calls
39+
`namespace`/`name` on it.
40+
41+
**Fix sketch:** fix squint's quote emission to route through
42+
`squint_core.symbol` (already a real fn) instead of the literal
43+
`cljs.core.symbol`. Then revert macros to symbol form for parity with
44+
cherry/cljs.test.
45+
46+
## Extensibility — currently impossible to extend
47+
48+
### 4. `assert-expr` is hardcoded
49+
Real `clojure.test/assert-expr` is a multimethod keyed on the head of
50+
the form inside `(is ...)`. Users add new assertions by `defmethod`-ing
51+
it. Ours is a `case` over `=`, `thrown?`, `thrown-with-msg?`. No
52+
extension point.
53+
54+
**Fix sketch:** make `assert-expr` a multimethod (or a registry of
55+
`{op-sym (fn [msg form] ...)}`) that the macro consults. Squint can ship
56+
the existing four cases as default methods.
57+
58+
### 5. `report` is hardcoded too
59+
`clojure.test/report` is a multimethod keyed on `:type`; users add
60+
methods to extend reporting (e.g. cljs-test-display). Ours is a single
61+
`case` defn.
62+
63+
**Fix sketch:** convert to a multimethod (works in squint's runtime; we
64+
already use `(use-fixtures ...)` etc.).
65+
66+
### 6. `test-var` is a plain fn
67+
`clojure.test/test-var` is a multimethod (`:default` impl is what
68+
everyone uses, but it's overridable). Same idea as `report`.
69+
70+
## Coverage gaps — features that exist but are partial
71+
72+
### 7. `:begin-test-ns` / `:end-test-ns` are never emitted
73+
We support the report types in `report`'s `case`, but `run-tests` never
74+
fires them. Real `clojure.test` brackets each ns it processes with these
75+
events; reporters use them to group output.
76+
77+
### 8. No `(t/async done body)` form
78+
Real `cljs.test` async tests use `(async done (do ... (done)))`. We use
79+
`^:async` on the deftest plus a Promise-returning body. Functionally
80+
equivalent but different surface — code copied from a CLJS project
81+
won't run.
82+
83+
### 9. Test discovery is registration-based, not metadata-based
84+
`clojure.test/run-tests` walks `ns-publics` and filters by
85+
`(:test (meta var))`. We use a separate `test-registry` because squint
86+
has no var metadata at runtime. That works for tests defined via our
87+
`deftest`, but a user who manually attaches `:test true` to a defn
88+
won't be picked up.
89+
90+
**Note:** unclear this is fixable without a var registry — probably
91+
accept the divergence and document.
92+
93+
### 10. `successful?` reads global counters
94+
`(successful? results)` is meant to take the *return value* of
95+
`run-tests` (a counters map) and tell you if it passed. Today many of
96+
our internal call sites pass `(:report-counters (get-current-env))`
97+
which is global state. After our run-tests rewrite returns counters
98+
properly this is mostly fine, but the helper still reads the env when
99+
called with no/incompatible args.
100+
101+
## Polish — cosmetic but worth fixing
102+
103+
### 11. Inner `run-tests` calls produce double summary output
104+
Two of cherry's `cross_platform_test` cases call `run-tests` inside test
105+
bodies (testing the runtime itself). Each emits its own `:summary` line,
106+
on top of the outer test run's summary. Visual noise; not a correctness
107+
issue.
108+
109+
**Fix sketch:** add a `{:summary? false}` opts arg to `run-tests`, or
110+
drop the auto-summary and require an explicit `(report {:type :summary})`
111+
at the call site (matching the very first version we had).
112+
113+
### 12. `:report-counters :summary` increments needlessly
114+
`report` calls `(inc-report-counter! :type)` unconditionally, including
115+
for `:type :summary`. Adds a meaningless counter to the env map.
116+
117+
### 13. No `*test-out*` redirection
118+
Output goes hard-wired to `js/console.log`/`js/console.error`. Real
119+
`cljs.test` lets users rebind via `*report-out*` or per-method.
120+
121+
### 14. Cherry runtime and squint runtime drifted in subtle ways
122+
Both implement the same surface but are separate `.cljs` files. Easy to
123+
fix one and forget the other. Could share via a `.cljc` if we factor
124+
out the few JS-vs-CLJS differences.

0 commit comments

Comments
 (0)