search

cgewecke / eth-gas-reporter

Gas usage per unit test. Average gas usage per method. A mocha reporter.
MIT License
603 stars 94 forks source link

Ability to display two different checks #217

Closed eternauta1337 closed 4 years ago

eternauta1337 commented 4 years ago

We're using gas reports currently in Synthetix, e.g. https://github.com/Synthetixio/synthetix/pull/728/checks?check_run_id=1125774785

As you can see in that PR, we're working with forks. Basically, what we're trying to do is to run a bunch of production tests on top of a fork of mainnet.

I would like to be able to have two different gas reports, one for unit tests (as shown in the link above), and a different one for production tests. Right now, we're outputting 2 files, test-gas-used.log, and test-gas-used-prod.log, but we're not sure how to get this to an actual report.

Also, while I'm here, is there a way to cause the CI to fail given a diff in gas usage above a certain threshold?

cgewecke commented 4 years ago

Hi @ajsantander. Thanks for opening. Some initial thoughts...

I would like to be able to have two different gas reports, one for unit tests (as shown in the link above), and a different one for production tests. Right now, we're outputting 2 files, test-gas-used.log, and test-gas-used-prod.log, but we're not sure how to get this to an actual report.

Will look into this...it's possible Codechecks already supports it.

In the most recent Synthetix runs it seems like the diff is still not being reported.

Recent PRs here using Travis show the expected output, so it doesn't seem to be a problem with the Codechecks service as a whole or anything. However, will need to figure out how to get it working correctly with CircleCI...

Also, while I'm here, is there a way to cause the CI to fail given a diff in gas usage above a certain threshold?

That's a good idea. Would have to add some logic here but this should be simple to implement. Will open as a separate issue.

eternauta1337 commented 4 years ago

@cgewecke any progress with this? Pls let us know how we can help.

cgewecke commented 4 years ago

@ajsantander Apologies, I should be able to look into this and other issues at buidler-gas-reporter in the next couple days...

Thanks for pinging.

cgewecke commented 4 years ago

There's support for this now in eth-gas-reporter 0.2.18 / buidler-gas-reporter 0.1.4.

(At synthetix you'll need to upgrade buidler-gas-reporter since you're using a beta)

An example with two reports, one failing a threshold setting looks like:

Screen Shot 2020-10-13 at 6 10 43 PM

@ajsantander

If you have a chance, could you double-check that the Codechecks api token, CC_SECRET was transferred to CircleCI? When you go to https://app.codechecks.io/, you should see something like this where the token is available for copying...

Screen Shot 2020-10-13 at 5 55 15 PM

Please just lmk about any problems you run into, will leave this open for now...

cgewecke commented 4 years ago

Another note: ganache-core 625 reports that there's a difference between forked and non-forked gas costs for SSTORE which looks like a bug.

jjgonecrypto commented 4 years ago

Thanks Chris :pray:

Yep, the secret is in CircleCI.

eternauta1337 commented 4 years ago

Thanks @cgewecke We'll be implementing this ASAP 🙏

eternauta1337 commented 4 years ago

@cgewecke FYI,

This is where we're implementing this https://github.com/Synthetixio/synthetix/pull/805 So far, it appears to work seamlessly. We'll let you know if we encounter any problems in the future.

cgewecke commented 4 years ago

@ajsantander Ok awesome! Will close here and keep an eye out over there. Thanks!