You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
test(anchors,settlements): cover AbortSignal forwarding to fetch
Description
Add test coverage verifying that the signal-accepting API functions in
src/lib/anchorsApi.ts and src/lib/settlementsApi.ts actually forward the
caller's AbortSignal to the underlying fetch call, mirroring
metricsApi.test.ts's existing "passes the abort signal through" pattern.
These functions (fetchAnchors, fetchAnchor, fetchSettlements,
fetchSettlement, and exportSettlementsCsv) are consumed through useAsync's
abortable loader (e.g. AnchorDetail, AnchorsPanel, SettlementDetail), so
correct signal forwarding is what makes in-flight requests cancel on
unmount/reload. Until now, neither suite asserted the signal actually
reached fetch, so a regression that silently broke cancellation would have
gone unnoticed.
registerAnchor and deregisterAnchor do not accept an AbortSignal, so they
are not applicable to this coverage task.
Implementation note
The existing template test asserted that fetch's signal is the SAME OBJECT
as the caller's (expect(fn.mock.calls[0][1].signal).toBe(controller.signal)).
That test was in fact FAILING on main, because apiRequest/apiTextRequest
compose the caller's signal with an internal timeout controller
(composeSignals in api.ts), so fetch receives a derived signal rather than
the caller's object. Because this issue is explicitly test-only (no
production changes), the literal same-object assertion is unachievable.
With maintainer direction, the new coverage instead asserts signal
forwarding FUNCTIONALLY: aborting the caller's AbortController causes the
signal handed to fetch to abort, and the in-flight request to reject with an
AbortError. This catches exactly the regression described in the issue
("a request no longer aborting on unmount/reload"), and is
implementation-agnostic with respect to the timeout-composition layer. The
broken metricsApi.test.ts template was repaired to the same correct
assertion so the suite is green and continues to serve as a valid template.
Changes
src/lib/anchorsApi.test.ts: added a mockAbortableFetch helper (a fetch mock
that, like real fetch, rejects with an AbortError when its signal aborts)
and a "abort signal forwarding" block with tests for fetchAnchors and
fetchAnchor.
src/lib/settlementsApi.test.ts: added the same helper and tests for
fetchSettlements, fetchSettlement, and exportSettlementsCsv (which also
accepts options.signal); imported exportSettlementsCsv at the top.
src/lib/metricsApi.test.ts: repaired the previously failing
"passes the abort signal through" test to use the correct functional
propagation assertion.
No production code changes; api.ts is untouched (zero diff).
Validation
All 24 tests across the three suites pass.
Mutation-tested: temporarily severing signal forwarding in api.ts makes all
six signal-forwarding tests fail (change then reverted), proving the tests
genuinely detect the regression.
ESLint clean on all changed files.
Coverage: anchorsApi.ts 100%, metricsApi.ts 100%, settlementsApi.ts 100%
lines/functions (the single uncovered branch is a pre-existing unreachable
empty-params fallback in exportSettlementsCsv, unrelated to signals).
next build: the only build errors (PoolsPanel.tsx duplicate declarations of
totalLiquidity/positions/filteredPools) are pre-existing and unrelated;
verified on a clean baseline with these changes stashed. This test-only PR
introduces no new build conflicts.
I added a CHANGELOG.md entry under the next ## [x.y.z] section (see the
Format note at the top of CHANGELOG.md), or this PR is docs-only /
test-only / internal tooling and doesn't change user-facing behavior.
-> TEST-ONLY: no CHANGELOG entry needed per the format note.
Hi! Quick note on the failing CI check on PR #413 (Closes #322) — I dug into
it and it's pre-existing on main, not caused by this change.
This PR is test-only. It touches just 3 files (anchorsApi.test.ts,
settlementsApi.test.ts, metricsApi.test.ts), and all three are lint-clean
(0 errors, 0 warnings) with their tests passing.
I reproduced the CI job locally and it fails fast at the repo-wide lint step
(npm run lint = eslint .) because of pre-existing errors in files this PR
doesn't touch: AnchorDetail.test.tsx, MetricsBar.test.tsx,
SettlementDetail.test.tsx, SortAnnouncer.tsx, ToastProvider.test.tsx, and
vitest.setup.ts — 10 errors total, none in my files. That's also why it dies
after ~29s (npm ci + lint), long before the much slower test step would run.
The same lint errors are on main, so the base branch's CI is already red too.
FYI, even past lint, npm run build is red on main (duplicate const
declarations in PoolsPanel.tsx) and npm run test has 74 pre-existing
failures in unrelated suites — so main's CI is currently failing end-to-end.
How would you like to handle it? I'm happy to either (1) leave this PR as-is
since it adds nothing to the breakage, or (2) open a separate PR to clear the
pre-existing lint/build/test issues so CI can go green. I just didn't want to
mix unrelated production/config fixes into a test-coverage PR without
checking with you first. Thanks!
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
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.
test(anchors,settlements): cover AbortSignal forwarding to fetch
Description
Add test coverage verifying that the signal-accepting API functions in
src/lib/anchorsApi.ts and src/lib/settlementsApi.ts actually forward the
caller's AbortSignal to the underlying fetch call, mirroring
metricsApi.test.ts's existing "passes the abort signal through" pattern.
These functions (fetchAnchors, fetchAnchor, fetchSettlements,
fetchSettlement, and exportSettlementsCsv) are consumed through useAsync's
abortable loader (e.g. AnchorDetail, AnchorsPanel, SettlementDetail), so
correct signal forwarding is what makes in-flight requests cancel on
unmount/reload. Until now, neither suite asserted the signal actually
reached fetch, so a regression that silently broke cancellation would have
gone unnoticed.
registerAnchor and deregisterAnchor do not accept an AbortSignal, so they
are not applicable to this coverage task.
Implementation note
The existing template test asserted that fetch's signal is the SAME OBJECT
as the caller's (expect(fn.mock.calls[0][1].signal).toBe(controller.signal)).
That test was in fact FAILING on main, because apiRequest/apiTextRequest
compose the caller's signal with an internal timeout controller
(composeSignals in api.ts), so fetch receives a derived signal rather than
the caller's object. Because this issue is explicitly test-only (no
production changes), the literal same-object assertion is unachievable.
With maintainer direction, the new coverage instead asserts signal
forwarding FUNCTIONALLY: aborting the caller's AbortController causes the
signal handed to fetch to abort, and the in-flight request to reject with an
AbortError. This catches exactly the regression described in the issue
("a request no longer aborting on unmount/reload"), and is
implementation-agnostic with respect to the timeout-composition layer. The
broken metricsApi.test.ts template was repaired to the same correct
assertion so the suite is green and continues to serve as a valid template.
Changes
that, like real fetch, rejects with an AbortError when its signal aborts)
and a "abort signal forwarding" block with tests for fetchAnchors and
fetchAnchor.
fetchSettlements, fetchSettlement, and exportSettlementsCsv (which also
accepts options.signal); imported exportSettlementsCsv at the top.
"passes the abort signal through" test to use the correct functional
propagation assertion.
No production code changes; api.ts is untouched (zero diff).
Validation
six signal-forwarding tests fail (change then reverted), proving the tests
genuinely detect the regression.
lines/functions (the single uncovered branch is a pre-existing unreachable
empty-params fallback in exportSettlementsCsv, unrelated to signals).
totalLiquidity/positions/filteredPools) are pre-existing and unrelated;
verified on a clean baseline with these changes stashed. This test-only PR
introduces no new build conflicts.
Closes #322
Checklist
Format note at the top of CHANGELOG.md), or this PR is docs-only /
test-only / internal tooling and doesn't change user-facing behavior.
-> TEST-ONLY: no CHANGELOG entry needed per the format note.
Closes #322