OZ tests investigation

[jdevcs]

Following open zeppelin tests are not passing persistently in last few releases and should be investigated and fixed before 1.8.2:
#5577 (comment)

 253 passing (2m)
  2 failing

  1) Contract: balance
       current
         returns the current balance of an account as a BN in a specified unit:
     Error: Invalid character
      at assert (node_modules/web3-utils/node_modules/bn.js/lib/bn.js:6:21)
      at parseBase (node_modules/web3-utils/node_modules/bn.js/lib/bn.js:274:7)
      at BN._parseBase (node_modules/web3-utils/node_modules/bn.js/lib/bn.js:298:14)
      at BN.init [as _init] (node_modules/web3-utils/node_modules/bn.js/lib/bn.js:105:14)
      at new BN (node_modules/web3-utils/node_modules/bn.js/lib/bn.js:39:12)
      at new BNwrapped (node_modules/web3-utils/lib/utils.js:480:16)
      at Object.balanceCurrent [as current] (src/balance.js:33:10)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)
      at Context.<anonymous> (test/src/balance.test.js:20:19)

  2) expectRevert
       "before each" hook for "rejects if no revert occurs":
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/alexluu/openzeppelin-test-helpers/test/src/expectRevert.test.js)
      at listOnTimeout (node:internal/timers:559:17)
      at processTimers (node:internal/timers:502:7)
      ```

[imranmahmudshoiball]

Great project

[luu-alex]

Oz test is failing due to the update of bn.js on web3js. in the oz test suite they were testing web3.utils.bn on decimals and this would output the wrong value.

// oz tests before web3 1.7.2
new web3.utils.bn('99.1') // the value of this BN.toNumber would be 9908, which clearly is wrong, should be 99.1 

The bn.js library never handled decimals properly and always outputted wrong values, so in the update any invalid inputs would result in an error

// oz tests after web3 1.7.2
new web3.utils.bn('99.1') // this would result in an error, failing the test in oz and synthetix

I dont think theres anything we can change but just keep in mind of why these testcases are failing.
Testcase that fails in OZ:

1 recognized error

Contract: balance
`returns the current balance of an account as a BN in a specified unit // fails due to passing invalid value of decimals.`

Testcases that fail in synthetix.
5 recognized errors

Contract: EtherWrapper
       On deployment of Contract
         should have a default
           mintFeeRate of 50 bps  // fails due to passing invalid value - decimals.

Contract: EtherWrapper
       On deployment of Contract
         should have a default
           burnFeeRate of 50 bps:

totalIssuedSynths
         when mint(1 sETH) is called
           fees escrowed = 0.005: // fails due to passing invalid value - decimals.

totalIssuedSynths
         when mint(1 sETH) is called
           then burn(`reserves + fees` WETH) is called
             fees escrowed = 0.01: // fails due to passing invalid values - decimals
Contract: EtherWrapper
       totalIssuedSynths
         when mint(1 sETH) is called
           then burn(`reserves + fees` WETH) is called
             then distributeFees is called
               total issued sUSD = $15: // fails due to passing invalid values - decimals

[luu-alex]

For
2) expectRevert
"before each" hook for "rejects if no revert occurs":
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/alexluu/openzeppelin-test-helpers/test/src/expectRevert.test.js)
at listOnTimeout (node:internal/timers:559:17)
at processTimers (node:internal/timers:502:7)

I dont get this error, it seems to be an inconsistent testcase and may need to be run a few times for this test to complete.
Based on investigation it is good to note these comments for future releases or when running OZ testcases but this issue may be closed.