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

Improve ERC20 tests coverage #712

Conversation

facuspagnuolo
Copy link
Contributor

Fixes #686 & #616

@facuspagnuolo facuspagnuolo force-pushed the enhancement/provide_more_erc20_tests branch from d0d6599 to 4018821 Compare January 28, 2018 22:06
@facuspagnuolo facuspagnuolo self-assigned this Jan 28, 2018
@facuspagnuolo facuspagnuolo force-pushed the enhancement/provide_more_erc20_tests branch from 4018821 to 02f8f5a Compare January 28, 2018 22:13
@facuspagnuolo facuspagnuolo force-pushed the enhancement/provide_more_erc20_tests branch from 4bf6092 to 94fc8ed Compare January 28, 2018 22:35
@facuspagnuolo
Copy link
Contributor Author

Moreover, if we get this PR merged, it fixes #548

describe('when the sender has enough balance', function () {
const amount = 100;

it('transfer the requested amount', async function () {

Choose a reason for hiding this comment

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

should be transfers instead of transfer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice catch! thx @gertjanleemans

@federicobond
Copy link
Contributor

federicobond commented Feb 8, 2018

This one became a bit of a hassle to review. I would have preferred 5 different pull requests that are easier to review and merge.

Copy link
Contributor

@federicobond federicobond left a comment

Choose a reason for hiding this comment

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

LGTM

@facuspagnuolo
Copy link
Contributor Author

totally agree @federicobond... sorry for that and thanks a lot for reviewing it :)

@facuspagnuolo facuspagnuolo merged commit 6ad275b into OpenZeppelin:master Feb 8, 2018
ProphetDaniel pushed a commit to classicdelta/Smart-Contracts that referenced this pull request Mar 9, 2018
* Improve StandardToken tests coverage
* Improve BasicToken test coverage
* Improve MintableToken test coverage
* Improve BurnableToken test coverage
* Improve PausableToken tests coverage
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

3 participants