Update unit test to handle arbitrary token decimals

OpenST / mosaic-contracts

Mosaic-0: Gateways and anchors on top of Ethereum to scale DApps

https://discuss.openst.org/c/mosaic

MIT License

114 stars 31 forks source link

Update unit test to handle arbitrary token decimals #755

Closed schemar closed 5 years ago

schemar commented 5 years ago

Added config in tests to get configured decimals (default or env override).
Updated MockToken to take decimals as constructor argument.
Updated all instantiations of various tokens in unit tests to use decimals from config.
Updated proof generator to use 18 decimals (it uses mock token).
Added new script test:range. Example: npm run test:range -- 16 18

Fixes #735

schemar commented 5 years ago

Ready for review 🦑

schemar commented 5 years ago

Should MosaicCoreConfig be updated?

I don't understand. Can you please elaborate?

Should the range script end with the first value to have failing tests (current behavior) or handle failing tests with each value in the range?

I will change the script to run all the tests and change the exit code based on whether a test fails.

jasonklein commented 5 years ago

Should MosaicCoreConfig be updated?

I don't understand. Can you please elaborate?

@schemar: apologies for not being specific. From MosaicCoreConfig:

/** The number of decimals of the base token. */
uint256 public constant DECIMALSFACTOR = 10 ** uint256(18);

I asked because of the changes to base token references in this PR. On the other hand, perhaps those references shouldn't be dynamic and should always have 18 decimals?

schemar commented 5 years ago

Should MosaicCoreConfig be updated?

I don't understand. Can you please elaborate?

@schemar: apologies for not being specific. From MosaicCoreConfig:
/** The number of decimals of the base token. */
uint256 public constant DECIMALSFACTOR = 10 ** uint256(18);
I asked because of the changes to base token references in this PR. On the other hand, perhaps those references shouldn't be dynamic and should always have 18 decimals?

Hm 🤔 Good point. Mosaic core is way outdated by now. It's probably not worth it to mingle around in the core code in the context of this PR.

schemar commented 5 years ago

Ready for review again. Now accepts one and two arguments and continues after failed decimal (prints all failed decimals at the end).