search

input-output-hk / offchain-metadata-tools

Tools for creating, submitting, and managing off-chain metadata such as multi-asset token metadata
Apache License 2.0
46 stars 33 forks source link

Testing decimals #26

Closed piotr-iohk closed 3 years ago

piotr-iohk commented 3 years ago

ADP-915

Added in this PR:

@sevanspowell I've been adding some additional tests and stumbled upon two issues, see below:

Issues:

  1. Failing npm test: I've noticed that automatic npm tests are failing:
  21 passing (640ms)
  1 failing

  1) token-metadata-creator
       Minimal workflow
         Add required fields:

      AssertionError: expected { Object (sequenceNumber, value, ...) } to deeply equal { Object (sequenceNumber, signatures, ...) }
      + expected - actual

       {
         "sequenceNumber": 0
         "signatures": []
      -  "value": "������"
      +  "value": "ギル"
       }

This is actually something I'm able to reproduce manually, i.e. creating metadata with token name: "ギル" results in "������" being stored in metadata file. I believe this used to work fine...

  1. Error messages

I think the error message could be improved a little bit. The message when someone is using decimal value that's out of range is OK:

$ token-metadata-creator entry 6b1b1cc20a8d37d1d7bea2ca5427335c7f4c586e70692333153ef38354455354444543494d414c5330   --name "DECIMALSCOIN"   --description "Test Decimals"   --policy /home/piotr/wb/tokens_minter/testnet/nft4_test/policy.script   --decimals 256
option --decimals: Error in $: Decimal value must be in the range [0, 255] (inclusive)

But looking at some other examples:

$ token-metadata-creator entry ... --decimals "potato"
option --decimals: Error in $: Failed reading: not a valid json value

$ token-metadata-creator entry ... --decimals 0x
option --decimals: Error in $: endOfInput

$ token-metadata-creator entry ... --decimals 0.x
option --decimals: Error in $: Failed reading: takeWhile1

I think it would be much nicer if we display "Decimal value must be in the range [0, 255] (inclusive)" for every case when decimals are invalid. What do you think?

sevanspowell commented 3 years ago

Regarding the error messages, I think that's a great idea. Aeson's (JSON handling library) default error messages are not good enough, even for those familiar with Aeson, let alone end-users.