Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added test file with initial tests. #6

Merged
merged 21 commits into from May 15, 2019

Conversation

willjgriff
Copy link
Member

@willjgriff willjgriff commented May 10, 2019

Added test file with tests for initialize() and addVaultToken(), still needs tests for removeVaultToken() and redeem(). It includes a helpers file and DaoDeployment file to extract some behaviour.

Added babel to enable JS ES6 in test files.

Added truffle compiler config to compile to Solidity v0.4.24.

Fixed a couple bugs preventing compilation.

Added openzeppelin-solidity library for ERC20 contract. (Don't forget to npm i).

I'm using truffle v5.0.14 not that it should matter but sometimes it does. You should be able to start ganache-cli then run tests in a separate terminal with truffle test.

@willjgriff willjgriff changed the title Redemptions contract changes Added test file with initial tests. May 10, 2019
@0xGabi
Copy link
Member

0xGabi commented May 11, 2019

@fabriziovigevani and me will continue tomorrow with tests left. Thanks for set it up for us 🙏🏻

@0xGabi
Copy link
Member

0xGabi commented May 13, 2019

Hey @willjgriff can you take a look to the progress we made over the weekend. Thanks

"args": ["test"]
}
]
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this .vscode file necessary? Can it be added to .gitignore?


require(redeemableToken.destroyTokens(msg.sender, _amount), ERROR_CANNOT_DESTROY_TOKENS);
// minime.destroyTokens() never returns false, only reverts on failure
redeemableToken.destroyTokens(msg.sender, _amount);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, we can remove the 'ERROR_CANNOT_DESTROY_TOKENS' error.


const redemptionAmount = 20000
const vaultToken0Ammount = 45231
const vaultToken1Ammount = 20001
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo, vaultToken1Amount


//mint redeemableTokens to first two accounts
await redeemableToken.generateTokens(redeemer,redemptionAmount)
await redeemableToken.generateTokens(rootAccount,80000)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constant for the 80000?


it('reverts if amount to redeem exceeds account\'s balance', async () => {
await assertRevert(redemptions.redeem(redemptionAmount + 1, { from:redeemer }),'REDEMPTIONS_INSUFFICIENT_BALANCE')
})
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could test the other require statements but it's up to you.

@0xGabi
Copy link
Member

0xGabi commented May 14, 2019

As @fabriziovigevani mention on his PR:

We thought it will be best to have a reference to the token manager instance of the redemption token instead of having a direct reference.
There's still two tests missing from the Redeem functionality that im not sure how to test.
require(msg.sender != address(vault),);
require(msg.sender != address(tokenManager),);

@willjgriff willjgriff merged commit beae869 into 1Hive:master May 15, 2019
@willjgriff willjgriff deleted the redemptions-contract-changes branch May 15, 2019 13:15
@0xGabi 0xGabi mentioned this pull request May 15, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants