Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Implements two gas reports, one for unit tests, and another for prod tests #805

Merged
merged 6 commits into from
Oct 16, 2020

Conversation

eternauta1337
Copy link
Contributor

@eternauta1337 eternauta1337 commented Oct 15, 2020

I requested this feature in eth-gas-reporter: cgewecke/eth-gas-reporter#217

This produces a new gas report in the "Checks" section and causes CI to fail if a method call consumes more than 25% than it did before.

Note that the "prod gas report" will only be produced on commits to the develop branch.

@codecov
Copy link

codecov bot commented Oct 15, 2020

Codecov Report

Merging #805 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #805   +/-   ##
========================================
  Coverage    98.13%   98.13%           
========================================
  Files           58       58           
  Lines         3919     3919           
  Branches       513      513           
========================================
  Hits          3846     3846           
  Misses          73       73           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32b3a10...32766b6. Read the comment docs.

@@ -306,6 +306,7 @@ module.exports = {
enabled: false,
showTimeSpent: true,
currency: 'USD',
maxMethodDiff: 25, // CI will fail if gas usage is > than this %
Copy link
Contributor

Choose a reason for hiding this comment

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

How can we override this failure if the expected gas usage is going to be more than 25% (something that can't be optimised for example the recent exchange updates for debt snapshot might increase it by more than 25% ?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suggest that in those cases, the percentage is incremented in the PR, but it has to be done consciously, and that's the whole point. After merge, another PR could lower the percentage again.

Copy link
Contributor

@jacko125 jacko125 left a comment

Choose a reason for hiding this comment

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

lgtm, will be helpful !

@eternauta1337 eternauta1337 merged commit 3baf30b into develop Oct 16, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants