Page MenuHomePhabricator

[Cashtab] Update tests and patch bug in SendByUrlParams.test.js
ClosedPublic

Authored by bytesofman on Feb 28 2024, 04:32.

Details

Summary

T3444

Implement standardized cashtab test wrapper. Testing revealed bug where input fields were being set before balance had updated, leading to incorrect validation errors.

Test Plan

npm test

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Tail of the build log:

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm WARN deprecated @babel/plugin-proposal-private-methods@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-methods instead.
npm WARN deprecated @babel/plugin-proposal-numeric-separator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-numeric-separator instead.
npm WARN deprecated @babel/plugin-proposal-nullish-coalescing-operator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-nullish-coalescing-operator instead.
npm WARN deprecated rollup-plugin-terser@7.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-terser
npm WARN deprecated @babel/plugin-proposal-class-properties@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-class-properties instead.
npm WARN deprecated @babel/plugin-proposal-private-property-in-object@7.21.11: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-property-in-object instead.
npm WARN deprecated sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec instead
npm WARN deprecated domexception@4.0.0: Use your platform's native DOMException instead
npm WARN deprecated abab@2.0.6: Use your platform's native atob() and btoa() methods instead
npm WARN deprecated stable@0.1.8: Modern JS already guarantees Array#sort() is a stable sort, so this library is deprecated. See the compatibility table on MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#browser_compatibility
npm WARN deprecated @babel/plugin-proposal-optional-chaining@7.21.0: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-optional-chaining instead.
npm WARN deprecated workbox-cacheable-response@6.6.0: workbox-background-sync@6.6.0

added 1722 packages, and audited 1723 packages in 22s

263 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

> cashtab@1.4.4 build
> node scripts/build.js

Creating an optimized production build...
Failed to compile.

[eslint] 
src/components/Send/SendXec.js
  Line 168:9:  'cashtabCache' is assigned a value but never used  no-unused-vars

Search for the keywords to learn more about each error.


Build cashtab-tests failed with exit code 1

Update SendXec to update params when balance and params change, update tests, move error msgs to constants

remove unrelated change, version bump

remove unrelated newline change

bytesofman added inline comments.
cashtab/src/components/Send/SendXec.js
412 ↗(On Diff #45704)

This logic depends on balances being updated, as we will throw an "amount exceeds balance" error if we run it before cashtab has synced its utxos.

We don't see this in prod because it (usually) takes longer for this to be parsed than for balances to load. Shows up in testing though.

In react, if we have anything that should happen when a state variable changes, it should be done in a dependent useEffect.

remove unrelated cache bug fix

emack requested changes to this revision.Feb 28 2024, 13:26
emack added a subscriber: emack.
emack added inline comments.
cashtab/src/components/Send/SendXec.js
375 ↗(On Diff #45705)

isn't txInfoFromUrl already a dependency to this useEffect() block?

This revision now requires changes to proceed.Feb 28 2024, 13:26
cashtab/src/validation/index.js
127–130 ↗(On Diff #45705)

Are these meant to be retained for prod?

cashtab/src/components/Send/SendXec.js
375 ↗(On Diff #45705)

isn't txInfoFromUrl already a dependency to this useEffect() block?

ignore this, I realized it was for an initial render so it doesn't flow to the later references to props inside txInfoFromUrl.

bytesofman added inline comments.
cashtab/src/validation/index.js
127–130 ↗(On Diff #45705)

nope -- good catch. removed.

bytesofman marked an inline comment as done.

rebase, version bump, remove debug logs

This revision is now accepted and ready to land.Feb 28 2024, 23:52