T3216
Update the alias lookup in sendToken.js component's address input field to use the alias api endpoint
Details
Details
- Reviewers
bytesofman Fabien - Group Reviewers
Restricted Project - Commits
- rABC8ba00ed0bdd9: [Cashtab][Alias] Update SendToken.js to use api endpoint in place of…
- npm test
- change the alias flag in Ticker.js to true
- npm start
- navigate to the Send Token page and click into any token
- input a pre-registered alias in the address input field and ensure it parses to the right address
- ensure the hyperlink underneath links correctly to the explorer page for this address
- send a token amount to this alias and ensure successful broadcast, and sanity check tx details in explorer
- input a valid eCash address and ensure it is accepted
- input a valid prefix-less eCash address and ensure it is accepted
- input an invalid eCash address and invalid alias and ensure it triggers validation error
- input an unregistered alias and ensure the eCash Alias does not exist or yet to receive 1 confirmation validation error is displayed
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- sendTokenAliasLookup
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 24541 Build 48679: Build Diff cashtab-tests Build 48678: arc lint + arc unit
Event Timeline
Comment Actions
Why is there no test change ?
When pushing this diff the aliasEnabled flag in Ticker.js is set to false, so that landing this diff does not prematurely publish this particular alias UI function. If the flag is enabled to true, then npm test will pick up on the UI changes and result in updates to the snapshots for this component.
What I'll do is add some additional unit tests that checks the state of this component.
| cashtab/src/components/Send/SendToken.js | ||
|---|---|---|
| 276 ↗ | (On Diff #41448) | This function is being deprecated after this diff (T3216) |
| 278 ↗ | (On Diff #41448) | Yup, alias cache deprecation is planned as one of the subsequent diffs (T3216) |
| 294 ↗ | (On Diff #41448) | yup  |
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.
Failed tests logs:
====== CashTab Unit Tests: Verify the SendToken component throws error on missing Wallet object ======
Error: expect(received).toBe(expected) // Object.is equality
Expected: "Cannot destructure property 'wallet' of '((cov_1erx5eord5(...).s[16]++) , _react.default.useContext(...))' as it is undefined."
Received: "Cannot destructure property 'wallet' of '((cov_2xkkrl9h1(...).s[16]++) , _react.default.useContext(...))' as it is undefined."
at Object.<anonymous> (/work/cashtab/src/components/Send/__tests__/SendToken.test.js:134:34)
at Promise.then.completed (/work/cashtab/node_modules/jest-circus/build/utils.js:391:28)
at new Promise (<anonymous>)
at callAsyncCircusFn (/work/cashtab/node_modules/jest-circus/build/utils.js:316:10)
at _callCircusTest (/work/cashtab/node_modules/jest-circus/build/run.js:218:40)
at _runTest (/work/cashtab/node_modules/jest-circus/build/run.js:155:3)
at _runTestsForDescribeBlock (/work/cashtab/node_modules/jest-circus/build/run.js:66:9)
at run (/work/cashtab/node_modules/jest-circus/build/run.js:25:3)
at runAndTransformResultsToJestFormat (/work/cashtab/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:170:21)
at jestAdapter (/work/cashtab/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:82:19)
at runTestInternal (/work/cashtab/node_modules/jest-runner/build/runTest.js:389:16)
at runTest (/work/cashtab/node_modules/jest-runner/build/runTest.js:475:34)
at Object.worker (/work/cashtab/node_modules/jest-runner/build/testWorker.js:133:12)Each failure log is accessible here:
CashTab Unit Tests: Verify the SendToken component throws error on missing Wallet object
Comment Actions
React/Jest dynamically generates the ((cov_1erx5eord5(...).s[16]++) portion of the expected error message Cannot destructure property 'wallet' of ((cov_1erx5eord5(...).s[16]++) _react.default.useContext(...)) as it is undefined. at the time of test execution, hence updating the error test case to check that the error message string contains Cannot destructure property 'wallet' as that is the core aspect being validated
| cashtab/src/components/Send/SendToken.js | ||
|---|---|---|
| 283 | I was talking about this one :) Maybe there is a test already but I couldn't find it ? | |
Comment Actions
Using unit tests to simulate frontend alias input into the address field, mocking the API response then validating the subsequent frontend error is proving to be a bit of a head scratcher, even with the react testing library. Will keep chipping away at this. So far the screen and renderhook approaches have not yielded any results.
Comment Actions
That's because you are not unit testing the feature. Your input is a giant handler, if you cut it into pieces that becomes much more manageable. You don't want to test user input or the query or the UI rendering, but the logic in between: if the requested alias doesn't exist, it doesn't return an address but an error.
Comment Actions
In that case there's already a unit test in aliasUtils.test.js that tests the case of an alias that isn't registered.
https://github.com/Bitcoin-ABC/bitcoin-abc/blob/c6b2bf9f979ba38e3d6ffd41d1257c6d0a3dda1e/cashtab/src/utils/__tests__/aliasUtils.test.js#L194-L212
But that is specifically validating the endpoint response is handled though. If you want to see automated testing of the UI throwing the error message above based on this endpoint response then that's the tricky part with React frontend testing where you have to simulate from the point of user input + mock the endpoint response + somehow catching the simulated UI response.
We've been manually testing these aspects thus far and it's definitely something we should be automating but may need a refactor of how the app and its various components are structured.
Comment Actions
^ @Fabien thoughts on comment above and whether we can explore this in a separate diff?
Comment Actions
I'm fine with running integration tests manually for now, at least it doesn't belong to this diff for sure