Contracts: Use test helpers #608
Conversation
bingen
force-pushed
the
contract_helpers_test
branch
from
b8a13e8
to
d9e0d4b
Compare
| @@ -16,9 +14,11 @@ const ERCProxyMock = artifacts.require('ERCProxyMock') | |||
| const KernelSetAppMock = artifacts.require('KernelSetAppMock') | |||
|
|
|||
| const APP_ID = hash('stub.aragonpm.test') | |||
| const ZERO_ADDR = '0x0000000000000000000000000000000000000000' | |||
| const EMPTY_BYTES = '0x' | |||
There was a problem hiding this comment.
Wonder if we can replace EMPTY_BYTES everywhere for the one exported here: https://github.com/aragon/contract-helpers/blob/master/packages/test-helpers/src/bytes.js#L1
There was a problem hiding this comment.
Wasn't sure, but perhaps we forgot to do this? Although it is different as the one in the package is 0x00.
There was a problem hiding this comment.
All the tests using 0x00 as empty bytes were failing, we should change it in the helpers I think
| @@ -16,9 +14,11 @@ const ERCProxyMock = artifacts.require('ERCProxyMock') | |||
| const KernelSetAppMock = artifacts.require('KernelSetAppMock') | |||
|
|
|||
| const APP_ID = hash('stub.aragonpm.test') | |||
| const ZERO_ADDR = '0x0000000000000000000000000000000000000000' | |||
| const EMPTY_BYTES = '0x' | |||
There was a problem hiding this comment.
Wasn't sure, but perhaps we forgot to do this? Although it is different as the one in the package is 0x00.
| assert.equal((await tokenMock.balanceOf(owner)).valueOf(), initialBalance - approvedAmount, 'Balance of owner should be correct') | ||
| assert.equal((await tokenMock.balanceOf(receiver)).valueOf(), approvedAmount, 'Balance of receiver should be correct') | ||
| assert.equal((await tokenMock.balanceOf(safeERC20Mock.address)).valueOf(), 0, 'Balance of mock should stay the same') | ||
| assertBn(await tokenMock.balanceOf(owner), initialBalance - approvedAmount, 'Balance of owner should be correct') |
There was a problem hiding this comment.
Not sure, but are we able to do math on BNs like this? We don't need to use sub()?
There was a problem hiding this comment.
Yea, it shouldn't work this way, let me fix it anyway
| @@ -7,17 +9,18 @@ contract('TimeHelpers', () => { | |||
|
|
|||
| it('checks block number', async () => { | |||
| assert.equal((await timeHelpersMock.getBlockNumberExt()).toString(), (await timeHelpersMock.getBlockNumber64Ext()).toString(), "block numbers should match") | |||
| assert.equal((await timeHelpersMock.getBlockNumberExt()).toString(), (await timeHelpersMock.getBlockNumberDirect()).toString(), web3.eth.blockNumber, "block number should match with real one", "block number should match with real one") | |||
| assert.equal((await timeHelpersMock.getBlockNumberExt()).toString(), (await timeHelpersMock.getBlockNumberDirect()).toString(), web3.eth.getBlockNumber, "block number should match with real one", "block number should match with real one") | |||
There was a problem hiding this comment.
A bit confused with this (although it's more with the original test)—do we not need to call getBlockNumber()?
There was a problem hiding this comment.
I think the original was malformed, let me change it
| 'Kernel should match' | ||
| ) | ||
| assert.equal( | ||
| await web3.eth.getStorageAt(appStorage.address, (await appStorage.getKernelPosition())), | ||
| kernel.address, | ||
| kernel.address.toLowerCase(), |
There was a problem hiding this comment.
I'm starting to wonder if we should have some sort of assertAddressEquals() or etc. so we don't have to do this 😄
There was a problem hiding this comment.
Haha this one was a bit particular
Depends on aragon/contract-helpers#53