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

Migrated to Open Zeppelin Test Environment and improved tests #97

Closed
wants to merge 5 commits into from

Conversation

nkuba
Copy link
Member

@nkuba nkuba commented Oct 2, 2020

In this PR we migrate to Open Zeppelin's Test Environment as we already did in keep-core (keep-network/keep-core#1528) and keep-ecdsa (keep-network/keep-ecdsa#501).

We also had to set long timeout for test runner to be able to complete workDistributionTest.js. To balance long time execution we added a command that let's a developer to run quick test suite, excluding the distribution test: npm run test:quick.

Closes #88

nkuba added 5 commits October 2, 2020 17:53
…nvironment

Dependencies to run tests with open zeppelin test environemnt.
OZ expects tests stubs in contracts directory.
Updated tests to be able to run on OZ test env. Also included couple of
improvements to tests.
There are work distribution tests that require longer timeout to be set.
It may be quite cumbersome for local development, so we also added a
command that will execute quick tests excluding the work distribution
test:
`npm run test:quick`
@pdyraga
Copy link
Member

pdyraga commented Oct 26, 2020

In #93 we added a possibility to console.log when debugging contract issues.

Is there any way to make it work with OZ test environment? My understanding is that it's not. In this context, I am not sure what's better - OpenZeppelin test environment or buidler-vm with logging. Don't have any data regarding performance and stability to compare them though.

Comment on lines 17 to 19
"ganache": "echo 'GANACHE IS NO LONGER USED; use buidler-vm script instead'",
"buidler-vm": "buidler node",
"buidler-vm:log": "buidler node | grep '^ \\|console.log'",
Copy link
Member

Choose a reason for hiding this comment

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

If we decide to sacrifice console.log functionality and move to OpenZeppelin test environment, we probably no longer need those lines.

@pdyraga
Copy link
Member

pdyraga commented Oct 27, 2020

We were able to merge #96 and #89 without this change. I looked at the performance comparison locally and it takes 13 minutes to execute tests on master with builder-vm and 27 minutes to execute tests on this branch. Looking at Circle, it also takes longer here - 11 minutes vs 16 minutes. Given that, I am not sure if we should proceed with OpenZeppelin test environment for this repository. Consistency is nice but being able to console.log for debugging is even nicer.

@pdyraga
Copy link
Member

pdyraga commented Nov 3, 2021

We are now on Hardhat test env, see #127. Closing without merging.

@pdyraga pdyraga closed this Nov 3, 2021
@pdyraga pdyraga deleted the oz branch November 3, 2021 08:55
@pdyraga pdyraga added this to the v2.0.0 milestone Sep 29, 2022
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.

Migrate to OpenZeppelin test environment
2 participants