Page MenuHomePhabricator

[Cashtab] Add unit tests for send BCHA amount validation
ClosedPublic

Authored by bytesofman on Jan 21 2021, 22:01.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC048b9b0884bc: [Cashtab] Add unit tests for send BCHA amount validation
Summary

Add a validation.js file in utils to split form validation out from functional component and allow efficient unit tests. Unit tests for covered error cases.

Test Plan

npm test

Diff Detail

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

Event Timeline

Fabien requested changes to this revision.Jan 22 2021, 08:31
Fabien added a subscriber: Fabien.
Fabien added inline comments.
web/cashtab/src/utils/__tests__/validation.test.js
57 ↗(On Diff #27214)

Why is this one not a string like the others ?

web/cashtab/src/utils/validation.js
4 ↗(On Diff #27214)

The API here is backwards. If validateCashAmount() == false then validation is successful.
If you want to keep the same API to keep it move-only, then pick a name that represents what it's doing.
Maybe shouldRejectAmountInput() ? There might be better names but you get the idea.

Side note: this validation seems to work just as well on tokens, so the cash wording (cashAmount) is not really necessary.
It is always possible to add cash or token specific validation later if needed.

This revision now requires changes to proceed.Jan 22 2021, 08:31

Make all test inputs strings, better function name for validation method

Tail of the build log:

added 2927 packages, and audited 2928 packages in 39s

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

6 vulnerabilities (4 low, 2 high)

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.
npm notice 
npm notice New minor version of npm available! 7.3.0 -> 7.4.3
npm notice Changelog: <https://github.com/npm/cli/releases/tag/v7.4.3>
npm notice Run `npm install -g npm@7.4.3` to update!
npm notice 

> cashtab@0.1.8 prebuild
> npm run prepare-fbts


> cashtab@0.1.8 prepare-fbts
> NODE_ENV=production npm run manifest-fbts && npm run collect-fbts && npm run fbt-generate-translations && npm run translate-fbts


> cashtab@0.1.8 manifest-fbts
> NODE_ENV=production node node_modules/babel-plugin-fbt/bin/manifest --src src


> cashtab@0.1.8 collect-fbts
> NODE_ENV=production node node_modules/babel-plugin-fbt/bin/collectFBT --pretty --manifest < .src_manifest.json > .source_strings.json


> cashtab@0.1.8 fbt-generate-translations
> NODE_ENV=production node node_modules/fbt-generate-translations --locales src/i18n/locales.js --multi-files translations


> cashtab@0.1.8 translate-fbts
> NODE_ENV=production node node_modules/babel-plugin-fbt/bin/translate.js --pretty --translations translations/*.json --jenkins > src/translatedFbts.json


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

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

./src/components/Send/Send.js
Attempted import error: 'validateCashAmount' is not exported from '@utils/validation'.


npm ERR! code 1
npm ERR! path /work/web/cashtab
npm ERR! command failed
npm ERR! command sh -c node scripts/build.js

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2021-01-22T11_53_00_442Z-debug.log
Build cashtab-tests failed with exit code 1
Fabien requested changes to this revision.Jan 22 2021, 11:54
Fabien added inline comments.
web/cashtab/src/components/Send/Send.js
25 ↗(On Diff #27257)

You missed this one

This revision now requires changes to proceed.Jan 22 2021, 11:54
This revision is now accepted and ready to land.Jan 22 2021, 15:42