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

Tests for .sol files #48

Open
marcelomorgado opened this issue May 13, 2020 · 9 comments
Open

Tests for .sol files #48

marcelomorgado opened this issue May 13, 2020 · 9 comments

Comments

@marcelomorgado
Copy link
Collaborator

Hi there, I've just pushed the Kyber interface fix. I've just noticed that was wrong because I've tried to use if for a project of mine.
I think that it would be better if contract errors could be caught by a test script. Do you think that make sense to have some truffle test cases for solidity files to have they covered?

Best,

@adrianmcli
Copy link
Member

That's a good idea to have Solidity test, I've been meaning to do that for a while. However, I would like to avoid Truffle's test runner if possible (especially with the way the repo is currently organized). I wonder if we can kind of roll our own testing harness.

@marcelomorgado
Copy link
Collaborator Author

Hi @adrianmcli, I've created this PoC here: #49 to have the solidity cover as well.
Some comments:

  • I didn't found a straightforward buidler-jest integration so I had to change to mocha;
  • Since Buidler EVM doesn't support fork from mainnet, I've setup ganache integration for that purpose (because of that console.log isn't available).

WDYT?

@adrianmcli
Copy link
Member

@marcelomorgado thanks for making this.

It looks like you're still running the tests from Javascript, so I was wondering why you needed to use Buidler and Waffle. They are great projects, but I'm not sure it's doing more than it was doing before since our tests already use Ganache.

@marcelomorgado
Copy link
Collaborator Author

marcelomorgado commented May 18, 2020

Yes, indeed it isn't changing too much, the reasons that I've done that were:

  • Buidler is flexible and supports the project's folders structure;
  • Better ethers.js integration;
  • Seems that will dry things a little bit (i.e. test-environment.js will be no longer needed);
  • A truffle alternative;

@adrianmcli
Copy link
Member

adrianmcli commented May 18, 2020

My concern is that the tests will take much longer since Mocha and the Ganache-buidler plugin does not run tests in parallel.

Currently, Jest allows us to create a Ganache instance for each test suite (i.e. test file) and run them in parallel. Also we don't use Truffle here either because this is not a dapp project. If I have a choice, I would always choose Buidler over Truffle.

@marcelomorgado
Copy link
Collaborator Author

I see, lose the parallel tests would be a regression here. So, the only thing that builder could help is to compile solidity code to see if there is an obvious error (like the one that I've found in the Kyber doc page sol code example that I've used in the PoC) but I'm not sure if worths.
I'll close my MR though (and open another one with the doc fix).
Thanks for the feedback @adrianmcli

@adrianmcli
Copy link
Member

@marcelomorgado no problem, thanks for making this, sorry we can't merge it in.

I'm actually working with Pato from Buidler, and maybe we can have something by next month that can actually help with this. I understand we need Buidler to compile Solidity contracts if we want to test Solidity usage. I'll keep this in mind, so we will leave this issue open.

@marcelomorgado
Copy link
Collaborator Author

Hi @adrianmcli !
WDYT to use hardhat to cover the .sol files and have faster tests?

@adrianmcli
Copy link
Member

@marcelomorgado I think that's a great idea! Hardhat forking is so much faster than Ganache.

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

No branches or pull requests

2 participants