test(app): cover RecipientEditor's duplicate-recipient warning - #411
Open
dchaudhari7177 wants to merge 2 commits into
Open
test(app): cover RecipientEditor's duplicate-recipient warning#411dchaudhari7177 wants to merge 2 commits into
dchaudhari7177 wants to merge 2 commits into
Conversation
export-csv.mjs had no tests. It is a script rather than a module -- it reads process.argv at import time, writes to stdout and calls process.exit -- so these drive it the way an operator does, as a subprocess over a fixture ndjson file, and assert on its stdout. That also covers the argv default and the exit code, which importing could not, and needs no refactor of the script. Eleven tests: the header row on an empty log, one row per record in column order, blank-line skipping, a missing field and an explicit null both becoming empty cells rather than "undefined"/"null", the three cell()-quoting cases (embedded comma, doubled quote per RFC 4180, embedded newline), unlisted record fields staying out of the output, the events.ndjson argv default, and a missing input exiting 1 with its message on stderr. No new dependencies: node:test plus the script itself. Verified: `node --test export-csv.test.mjs` -> 11 pass, 0 fail. Two pre-existing problems in indexer/ that this does not touch, both visible when running `node --test` across the directory: - index.test.mjs does not parse. At line 30 a second file's imports are pasted into the middle of the first test function, so `node --check` fails with "SyntaxError: Unexpected identifier 'test'" and none of that file's tests ever run. It survived because no workflow runs the indexer tests -- grep for "indexer" across .github/workflows/ finds nothing. Left alone here to keep this PR to one concern; happy to send the fix separately. - replay.test.mjs fails only for want of @stellar/stellar-sdk, which is not installed in this environment. Not a bug. The four src/**/*.test.ts files are vitest tests (see vitest.config.ts) and `node --test` is not meant to run them. Closes tributary-protocol#335 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
RecipientEditor.test.ts covers parseCsv/rowsTotal/rowsError but never renders the component, so the duplicate warning had no coverage. Nine rendered tests: the same address twice warns, both offending rows are marked (not just the second), a unique set is left alone, addresses differing only in whitespace count as the same recipient, repeated split ids do not warn, a triplicate marks all three rows under one note, two separate duplicated addresses are marked independently, and neither an empty editor nor two blank address rows warn. Two setup notes: - The jsdom environment is set with a per-file `@vitest-environment` docblock rather than in vite.config.ts, so the existing node-environment tests keep running exactly as before and no shared config changes. - cleanup() is registered explicitly. @testing-library/react only installs its automatic afterEach when vitest globals are enabled, and this project does not enable them, so without it renders pile up in document.body and any unscoped query sees every earlier test's DOM. That is what made the first version of the second test read 4 markers instead of 2. Assertions are on the warning elements (.dupe-note, .dupe-input, the aria-label="Duplicate recipient" marker), not on the rendered copy, because three of the i18n keys the component asks for -- duplicateRecipientNote, duplicateAddressHint and duplicateRecipientError -- are absent from the translations table in src/lib/i18n.tsx. t() falls through to returning the key, so the warning currently shows users the literal string "duplicateRecipientNote". Asserting on the copy would have pinned that in place. Not fixed here: adding the keys is a separate concern. Verified with `npx vitest run`: 105 passed, up from 96. The 5 failures in src/lib/tributary.test.ts (fromStroops formatting) are pre-existing and occur identically with this file removed. `npm run build` -- the stated CI gate for the app -- also fails on an unmodified tree, with 10 TS errors, all in src/components/StreamsCard.tsx (missing Client methods for the stream API, plus one implicit any). None are in the file added here. Closes tributary-protocol#328 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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 #328
Coverage
RecipientEditor.test.tscoversparseCsv/rowsTotal/rowsErrorbut never renders the component, so the duplicate warning had none. Nine rendered tests:.trim(), and pasted addresses carry spacesduplicateAddressesonly considers address rowsnpx vitest run→ 105 passed, up from 96.A bug this surfaced — the warning shows users a raw i18n key
Assertions are on the warning elements (
.dupe-note,.dupe-input, thearia-label="Duplicate recipient"marker) rather than on the rendered copy, and deliberately so:Three of the i18n keys
RecipientEditorasks for are missing from the translations table insrc/lib/i18n.tsx:duplicateRecipientNoteduplicateAddressHintduplicateRecipientErrort()falls through todict[key] || key, so the duplicate warning currently renders the literal stringduplicateRecipientNoteto the user. (For contrast, the adjacentsharesTotalWarnis present in all 5 languages.) Asserting on the copy would have pinned that bug in place, so I asserted on the elements — correct now and after the keys are added. Not fixed here, since adding them is a separate concern per CONTRIBUTING; happy to send it.Two setup notes
@vitest-environmentdocblock rather than invite.config.ts, so the existing node-environment tests keep running exactly as before and no shared config changes.cleanup()is registered explicitly.@testing-library/reactonly installs its automaticafterEachwhen vitest globals are enabled, and this project doesn't enable them — without it, renders pile up indocument.bodyand any unscoped query sees every earlier test's DOM. That's what made my first attempt read 4 duplicate markers instead of 2. Worth knowing before the next component test lands.Pre-existing failures, for the record
Both occur identically with this file removed — I checked:
src/lib/tributary.test.ts(fromStroopsformatting: thousands grouping,MAX_SAFE_INTEGERexactness, values above 2^53).npm run buildfails — the stated CI gate for the app — with 10 TS errors, all insrc/components/StreamsCard.tsx:get_stream,vested_of,create_stream,withdraw_vested,cancel_stream,top_upmissing onClient, plus one implicitany. Looks like the component is ahead of the generated SDK bindings. None are in the file added here.🤖 Generated with Claude Code