[162] Write React Testing Library assertions for network_sync_checker - #358
Open
kingeligma wants to merge 2 commits into
Conversation
Adds an interactive NetworkSyncChecker component exposing the network_sync_checker probe lifecycle (idle, checking, synced, cancelled, blocked, error) with React Testing Library assertions for network mismatch, missing-wallet setup instructions, signature approval, user rejection, unexpected signing failures and probe re-runs. Also repairs pre-existing blockers so the full suite and CI go green: - SignatureTimeoutAlert: derive parse failures with useMemo instead of setState-in-effect (fixes eslint react-hooks/set-state-in-effect). - vitest.setup: shim localStorage for Node 26's experimental global that shadows jsdom's. - freighter_multisig_hook: use real signed transaction envelopes instead of arbitrary bytes the hook's XDR parser rejects. - freighter component / signature timeout suites: stub WalletProvider's useWallet so the stellar-wallets-kit UMD graph no longer breaks module load; wrap loader-counter updates in act() for React 19. Closes 162
There was a problem hiding this comment.
Pull request overview
Adds a new NetworkSyncChecker UI component and a comprehensive React Testing Library suite that exercises the network_sync_checker lifecycle end-to-end, while also making a set of targeted test/lint fixes needed to get the existing suite green in CI.
Changes:
- Introduces
NetworkSyncCheckerplus new RTL assertions covering network mismatch, missing wallet, successful signing, user rejection, and unexpected errors. - Refactors
SignatureTimeoutAlertto compute parse errors as derived state (avoidingsetStateinuseEffect) and updates related tests/mocks for React 19 + ESM constraints. - Stabilizes the test environment and fixtures (Vitest
localStorageshim; multisig hook fixtures now use real signed envelopes).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
vitest.setup.ts |
Adds a Node-version-safe localStorage shim for jsdom-based tests. |
app/components/SignatureTimeoutAlert.tsx |
Replaces effect-driven parse error state with useMemo-derived state. |
app/components/NetworkSyncChecker.tsx |
New interactive component that runs the sync probe and renders probe status + warnings. |
__tests__/signature_timeout_alert.test.tsx |
Stubs useWallet to avoid UMD import issues; wraps loader-counter updates in act(). |
__tests__/network_sync_checker_component.test.tsx |
New RTL suite for the NetworkSyncChecker probe lifecycle. |
__tests__/freighter_multisig_hook.test.ts |
Builds valid signed transaction envelopes so XDR parsing under test succeeds. |
__tests__/freighter_connector.component.test.tsx |
Stubs useWallet to keep the Freighter connector suite loadable under Node ESM. |
Suppressed comments (1)
app/components/NetworkSyncChecker.tsx:114
- The blocked status copy says "resolve the warning below", but the warning UI is rendered above this status output (and in some blocked cases there may be no warning). Consider using location-agnostic wording to avoid confusing users.
{status === "blocked" &&
"Network sync blocked — resolve the warning below and try again."}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+64
to
+71
| const availabilityState = availability ?? checkWalletAvailability(detector); | ||
| if (!availabilityState.available || typeof onSign !== "function") { | ||
| if (showToast && availabilityState.warningMessage) { | ||
| showToast(availabilityState.warningMessage, "warning"); | ||
| } | ||
| setStatus("blocked"); | ||
| return; | ||
| } |
|
@kingeligma Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
…cing timers Three timeout tests attached promise.catch() only after awaiting timer advance, so the rejected outer promise could be snapshotted as unhandled by Node during the interleaved microtask gap, making vitest report 3 unhandled errors and exit non-zero despite all tests passing. Attach the handler immediately after runSignatureWithTimeout (mirroring the .rejects-based tests) to keep npm run test exiting cleanly.
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.
Closes #162
What
Adds a
NetworkSyncCheckercomponent and React Testing Library assertions that exercise the fullnetwork_sync_checkerprobe lifecycle:Repairs required to make the suite/CI green
The repo's test suite did not run green locally or in CI (pre-existing). This branch fixes each blocker so the PR's checks pass:
setState-in-useEffectwith auseMemo-derived parse result (fixes eslintreact-hooks/set-state-in-effect, which blocked CI's lint step).localStoragewhen Node's experimental global shadows jsdom's (Node 24+ ships alocalStoragethat isundefinedwithout--localstorage-file).useWallet(real provider default shape) so the stellar-wallets-kit UMD module graph no longer breaks suite load; wrap loader-counter updates inact()for React 19.Checks
npm run lint— passnpm run type-check— passnpm run test— 87 files / 1326 tests passnpm run build— pass