Unify async test assertions on a shared helper#641
Open
polRk wants to merge 1 commit into
Open
Conversation
- Add a shared track() promise-settlement helper next to each settle() in *.fixtures.ts (core, coordination, topic) and a new fsm promise.fixtures.ts - Replace hand-rolled `.then()` probes with track(), and liveness Promise.race timeouts with `await expect().resolves` - Swap reader-rebalance's local trackCommit for the shared helper - Document the one canonical vitest-expect assertion style in AGENTS.md - Clears the promise(always-return) and no-unmodified-loop-condition lint warnings (48 -> 46)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Standardize the test suite on a single async-assertion style — vitest
expectmatchers plus one shared promise-settlement helper — replacing scattered hand-rolled.then()idioms. Test-only; no product behavior changes.Why
The suite mixed several ad-hoc async-assertion idioms: bare
.then()flag probes for "still pending",Promise.race([p, setTimeout(...)])timeouts for liveness, and a per-file re-inventedtrackCommit. This was inconsistent and tripped two oxlint warnings (promise(always-return),no-unmodified-loop-condition). AGENTS.md already declares vitestexpectthe standard, but it wasn't enforced for the async cases.Changes
track(promise) → { settled, rejected, reason? }helper next to each package'ssettle()in*.fixtures.ts(core, coordination, topic writer + reader), plus a newpackages/fsm/src/promise.fixtures.ts.let f = false; p.then(() => (f = true))) withtrack()across endpoints, writer, session-runtime, session-registry, and fsm queue.Promise.race([p.then(...), setTimeout(...)])liveness checks withawait expect(p).resolves.*, bounded by the test timeout (coordination client, topic reader).trackCommitfor the sharedtrack().expectassertion style (rejects / resolves / pending-probe) in AGENTS.md Testing Rules.Not a breaking change. Intentionally left as-is (documented in review, not the target anti-pattern):
session-pooldual-outcome captures, theelection/mutexintegration probes with their justified heuristic sleeps, and the fsmsleep().then('label')race harness.Testing
npm run build— passes (fixtures are compiled, sotrack/Settlementare type-checked).npm run test:uni— 1023 passed, 1 skipped.reader-rebalanceintegration test — run live against local YDB, passes.npm run lint— exit 0, 46 warnings (down from 48;promise(always-return)andno-unmodified-loop-conditioncleared, none added).Checklist
Separate from the core-package refactor (#638).