Skip to content
This repository has been archived by the owner on Mar 17, 2024. It is now read-only.

Ability to display two different checks #217

Closed
eternauta1337 opened this issue Sep 17, 2020 · 9 comments
Closed

Ability to display two different checks #217

eternauta1337 opened this issue Sep 17, 2020 · 9 comments

Comments

@eternauta1337
Copy link

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
Copy link
Owner

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
Copy link
Author

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

@cgewecke
Copy link
Owner

cgewecke commented Oct 1, 2020

@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
Copy link
Owner

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
Copy link
Owner

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
Copy link

Thanks Chris 🙏

Yep, the secret is in CircleCI.

@eternauta1337
Copy link
Author

Thanks @cgewecke
We'll be implementing this ASAP 🙏

@eternauta1337
Copy link
Author

@cgewecke FYI,

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

@cgewecke
Copy link
Owner

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

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants