Two async tests call `done` inside `p/with-redefs`, leaking stubs into the next namespace and failing the suite
Nobody has claimed this yet.
- Dominant language
- No language data
- Stars
- 28
- Forks
- 2
- PR merge metrics
- No merged PRs in 30d
Description
pnpm test fails on current master (5824434a74) with two failures. Both are in test namespaces added by 4b22fb7957, but neither is the faulty test — each passes in isolation. The cause is two older tests that call done inside p/with-redefs, so cljs.test advances to the next namespace while the redefs are still installed.
| Failing test | Broken by |
|---|---|
logseq.api.db-based.cli-test / upsert-nodes-imports-page |
frontend.handler.db-based.import-test |
logseq.api.db-based.tools-test / list-and-get-page-data |
frontend.handler.db-based.property-test |
Mechanism
p/with-redefs restores originals only after its body promise settles, but in both tests (p/finally done) is part of that body — so done fires first:
src/test/frontend/handler/db_based/import_test.cljs:61—state/<invoke-db-workerstays stubbed to(p/resolved nil). The next namespace's worker call returnsnil:No protocol method IDeref.-deref defined for type null.src/test/frontend/handler/db_based/property_test.cljs:55—state/get-current-repostays stubbed to"test". The next namespace'sstart-test-db!registers under the wrong repo,conn/get-dbreturnsnil:Assert failed: (db/db? db)(vialogseq.api.db-based.tools/list-pages,tools.cljs:90).
property_test.cljs contains both patterns — its first test already puts p/finally/done outside the redefs; only the second one wraps them inside.
Reproduce
node static/tests.js -n frontend.handler.db-based.import-test -n logseq.api.db-based.cli-test
node static/tests.js -n frontend.handler.db-based.property-test -n logseq.api.db-based.tools-test
Each fails deterministically; each namespace passes alone. Not timing-dependent in practice — shard membership comes from round-robin-batches in scripts/src/logseq/tasks/dev.clj and depends only on the namespace list, so CI hits it too.
Fix
Move (p/finally done) outside p/with-redefs in both tests, matching the working pattern already in property_test.cljs. Six lines ignoring reindentation; pnpm test then exits 0 (40/40 shards).
Environment
macOS 25.6.0 (arm64), master 5824434a74, Node via repo toolchain, pnpm 10.33.0.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with src/test/frontend/handler/db_based/import_test.cljs:61 and property_test.cljs:55, comparing the second test with the working first-test pattern in property_test.cljs. Run the two listed node static/tests.js reproductions, then pnpm test; done means both namespace pairs pass and the full suite exits 0.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- clojure, javascript
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 90/100