test(dada-client): characterize the untested surface (LIVE-35224) - #20276
Conversation
Web Tools Build Status
|
There was a problem hiding this comment.
Pull request overview
Adds a comprehensive characterization-test suite around libs/ledger-live-common/src/dada-client to pin current behavior ahead of the planned DDD relocation/migration, without modifying production runtime code.
Changes:
- Add characterization tests for selector cache-walk behavior, API transforms, and RTK Query endpoints under
src/dada-client. - Add hook-level tests for query-arg shaping, loading/error semantics, memoization/stability guarantees, and edge cases.
- Add a patch changeset for
@ledgerhq/live-commonreflecting the test-only addition.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| libs/ledger-live-common/src/dada-client/utils/test/mergeAssetsDataPages.test.ts | Characterizes merging semantics for paged assets data (shallow merge, last-write wins, pagination/currenciesOrder behavior). |
| libs/ledger-live-common/src/dada-client/state-manager/tests/apiTransforms.test.ts | Exercises public RTK Query endpoints to characterize private transform/conversion behaviors and chunked fetching semantics. |
| libs/ledger-live-common/src/dada-client/hooks/tests/useStockAssetIds.test.ts | Pins query args + Set-wrapping/memoization behavior for stock asset IDs hook. |
| libs/ledger-live-common/src/dada-client/hooks/tests/useMarketByCurrencies.test.tsx | Pins selection/filtering + rounding behavior for market data hook and reference stability. |
| libs/ledger-live-common/src/dada-client/hooks/tests/useInterestRatesByCurrencies.test.tsx | Pins allowlist/type filtering, zero-is-value semantics, and reference stability for interest rate hook. |
| libs/ledger-live-common/src/dada-client/hooks/tests/useDrawerConfiguration.test.ts | Characterizes feature-flag-driven drawer configuration precedence/merging and callback stability. |
| libs/ledger-live-common/src/dada-client/hooks/tests/useChunkedAssetsData.test.ts | Pins param shaping (skip separation), loading-state semantics, and ErrorInfo parsing. |
| libs/ledger-live-common/src/dada-client/hooks/tests/useAssetData.test.ts | Pins param narrowing behavior and `isLoading |
| libs/ledger-live-common/src/dada-client/entities/tests/selectorUtils.test.ts | Characterizes RTK Query cache-shape walk semantics and cross-query “first match wins” behavior. |
| libs/ledger-live-common/src/dada-client/entities/tests/marketSelectors.test.ts | Pins selector reads-only-from-markets behavior and empty/partial handling. |
| libs/ledger-live-common/src/dada-client/entities/tests/interestRateSelectors.test.ts | Pins selector reads-only-from-interestRates behavior and empty/field-preservation semantics. |
| .changeset/lucky-mirrors-observe.md | Patch changeset noting added characterization tests in @ledgerhq/live-common. |
Rsdoctor Bundle Diff AnalysisFound 7 projects in monorepo, 7 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 desktop-mainPath:
📁 desktop-preloaderPath:
📁 desktop-rendererPath:
📁 desktop-webviewDappPreloaderPath:
📁 desktop-webviewPreloaderPath:
📁 desktop-workersPath:
📁 mobilePath:
Generated by Rsdoctor GitHub Action |
Pin current behaviour of the dada-client code that had no test coverage, as the safety net for the upcoming DDD relocation (LIVE-35223). No production code changed. Covers the cache-shape selectors, the RTK Query wrapper hooks and the API transforms. convertApiAssets, transformAssetsResponse and emptyAssetsData are module-private, so they are characterized through the getAssetData and getChunkedAssetsData endpoints with a real store and a stubbed fetch. These tests must keep passing unmodified through the migration: one that needs editing to accommodate a later change signals broken behaviour, not a wrong test.
develop added networkIds to GetAssetsDataParams after this branch was written (perf(modular-drawer): forward network filters to dada). buildAssetsQueryParams is covered by that change's own tests, but the chunked endpoint's forwarding was not: networkIds must be repeated in full on every chunk while currencyIds are split. Pin it so the DDD relocation cannot silently drop it.
3bbb92b to
dd341f2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (3)
libs/ledger-live-common/src/dada-client/state-manager/tests/apiTransforms.test.ts:100
- Avoid
jest.restoreAllMocks()here: it restores all spies/mocks (including those installed by global jest setup), which can break unrelated tests when running the suite in parallel. Prefer restoring only the localfetchSpyfor this file.
afterEach(() => {
jest.restoreAllMocks();
});
libs/ledger-live-common/src/dada-client/state-manager/tests/apiTransforms.test.ts:96
jest.spyOn(globalThis, "fetch")will call the real undicifetchwhen a test forgets to stub a response, which can accidentally hit the network and make failures slow/flaky. It’s safer for this suite to defaultfetchto throwing unless a test explicitlymockResolvedValueOnce/mockRejectedValueOnce.
beforeEach(() => {
fetchSpy = jest.spyOn(globalThis, "fetch");
});
libs/ledger-live-common/src/dada-client/hooks/tests/useDrawerConfiguration.test.ts:15
- This test mocks
useFeature, butuseFeatureis a Redux selector hook (it usesuseSelector). Repo testing guidance prefers exercising feature flags via a store state (e.g., a Provider +featureFlags.resolved.ptxEarnDrawerConfiguration) rather than mocking the hook, so the test better reflects real behavior and avoids mock drift.
import { useFeature } from "@features/platform-feature-flags";
import { useDrawerConfiguration } from "../useDrawerConfiguration";
jest.mock("@features/platform-feature-flags", () => ({ useFeature: jest.fn() }));
const mockUseFeature = jest.mocked(useFeature);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const flag = (value: unknown) => mockUseFeature.mockReturnValue(value as any);
|



✅ Checklist
npx changesetwas attached.- No runtime impact — no production code was changed. The diff is 11 new test files plus a changeset.
- CI signal only: the
src/dada-clientsuite goes from 10 suites / 96 tests to 21 suites / 245 tests.- No QA pass needed. Nothing to test in the apps.
📝 Description
Problem. 14 source files under
libs/ledger-live-common/src/dada-clienthad no colocated test, and the one existing api test covered onlybuildAssetsQueryParamsand the twofetchAll*ByCategoryhelpers — not the RTK Query endpoints,getChunkedAssetsData,convertApiAssets, ortransformAssetsResponse. The package is about to be relocated into DDD packages under LIVE-35223, and a green CI run would not have proven that relocation behaviour-neutral. The riskiest code in the package is also the least typed:createCurrencyDataSelectorwalks RTK Query's internal cache layout (queries[*].data.pages[*][key][id]) onRecord<string, unknown>, so no refactor there can ever produce a type error.Solution. 149 characterization tests pinning current behaviour, with no production change. These become the contract for the rest of the migration epic: they must keep passing unmodified, and a test that needs editing to accommodate a later change signals broken behaviour rather than a wrong test.
state-manager/__tests__/apiTransforms.test.tsconvertApiAssets,transformAssetsResponse,getChunkedAssetsDatahooks/__tests__/useMarketByCurrencies.test.tsxentities/__tests__/selectorUtils.test.tsutils/__test__/mergeAssetsDataPages.test.tshooks/__tests__/useInterestRatesByCurrencies.test.tsxisEqualstabilityhooks/__tests__/useStockAssetIds.test.tshooks/__tests__/useDrawerConfiguration.test.tshooks/__tests__/useAssetData.test.tsisLoading ‖ isFetchinghooks/__tests__/useChunkedAssetsData.test.tsErrorInfoshapeentities/__tests__/interestRateSelectors.test.tsinterestRatesonlyentities/__tests__/marketSelectors.test.tsmarketsonlyFour invariants that break silently are now pinned executably, since none of them can produce a type error:
assetsDataApi.reducerPathis the literal string"assetsDataApi"— read by string inselectorUtils.ts:25and preloaded by three Storybook files.convertApiAssetsis deliberately lenient: it silently drops tokens whose parent chain is unknown, and synthesises a currency (color: "#999999",explorerViews: []) for cryptos missing from the local registry rather than dropping them.getChunkedAssetsDatasucceeds if any chunk resolves.The contract already proved itself
This branch was rebased onto 59 new
developcommits, two of which changed dada-client production code (perf(modular-drawer): forward network filters to dada, addingnetworkIdstoGetAssetsDataParamsandbuildAssetsQueryParams). Every characterization test passed unmodified through that rebase.That change did open one gap, closed by the second commit here:
networkIdsalso flows throughgetChunkedAssetsData, where — unlikecurrencyIds, which is split across chunks — it must be repeated in full on every request.buildAssetsQueryParamsis covered by that change's own tests, but the chunked path's forwarding was not. Exactly the kind of detail a relocation drops silently.Notes for reviewers
convertApiAssets,transformAssetsResponseandemptyAssetsDataare module-private and this task must not touch production code, so they are driven through thegetAssetData/getChunkedAssetsDataendpoints with a real store and a stubbedfetch. That pins the observable contract rather than internals, which is what the migration has to preserve anyway.getAssetDatagoes throughfetchBaseQuery, sofetchreceives aRequest; the chunked path callsfetchwith a string.requestedUrl()normalises both.id: ""makesCryptoCurrencyIdSchema.parsethrow and fails the entire query, not just that asset. Pinned as current behaviour inapiTransforms.test.ts; the follow-up validation ticket (LIVE-35233) must not make it worse.useMarketByCurrenciesrounding table was verified against the implementation rather than assumed. Rounding isMath.round(x * 100) / 100, so results follow float representation, not decimal intuition:-2.345 → -2.35,-1.005 → -1, and an exact.5rounds toward +∞ (0.125 → 0.13but-0.125 → -0.12).useDrawerConfigurationis tested here although it is not DADA code — it only reads a feature flag. It is misfiled in this directory and gets relocated later in the epic, so it needs the same protection.apiTransforms.test.tsleaves a worker to be force-exited (A worker process has failed to exit gracefully). Several endpoints setkeepUnusedDataFor: 86400, which parks a 24-hour timer. Releasing the subscription viaunsubscribe()was tried and was not sufficient; the real fix is fake timers or aresetApiStateteardown. Left out of scope on purpose so this PR stays tests-only.patchon@ledgerhq/live-common— internal-only change, no runtime behaviour affected.❓ Context
🧐 Checklist for the PR Reviewers