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

Updated OpenZeppelin to v2.0. #462

Closed
wants to merge 1 commit into from
Closed

Conversation

@nventuro
Copy link
Member

nventuro commented Dec 6, 2018

Fixes #565

OpenZeppelin 2.0 has been audited and has a stable API, so migrating early to the 2.x branch will prevent future upgrade issues.

@nventuro

This comment has been minimized.

Copy link
Member Author

nventuro commented Dec 6, 2018

This is failing because the App tests use OpenZeppelin's expectEvent test helper, which changed behavior from 1.10 to 2.0: the 2.0 version actually checks the values emitted in the event, whereas in 1.10 only the event name was compared.

The test suit has a mistake in that it expects the version field in the PackageChanged event to be a string (it is an array). Sadly, since expectEvent doesn't support checking arrays of BigNumber instances, the new test helper cannot be used to test this. I've opened an issue in the newly created openzeppelin-test-helpers package to track this - once implemented, I'll update this PR and fix the test.

@facuspagnuolo facuspagnuolo self-requested a review Dec 10, 2018
@facuspagnuolo facuspagnuolo self-assigned this Dec 10, 2018
@facuspagnuolo facuspagnuolo removed their assignment Jan 10, 2019
@spalladino

This comment has been minimized.

Copy link
Member

spalladino commented Feb 25, 2019

Closing in favour of #739

@spalladino spalladino closed this Feb 25, 2019
@nventuro nventuro deleted the nventuro:oz-2.0 branch Feb 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.