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

add some unit tests for estimateGas #324

Merged
merged 3 commits into from
Jul 23, 2021
Merged

Conversation

yihuang
Copy link
Contributor

@yihuang yihuang commented Jul 20, 2021

Description


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Jul 20, 2021

Codecov Report

Merging #324 (29364cd) into main (e61594e) will increase coverage by 5.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #324      +/-   ##
==========================================
+ Coverage   47.38%   52.42%   +5.04%     
==========================================
  Files          46       46              
  Lines        4611     4614       +3     
==========================================
+ Hits         2185     2419     +234     
+ Misses       2348     2093     -255     
- Partials       78      102      +24     
Impacted Files Coverage Δ
x/evm/keeper/grpc_query.go 70.64% <100.00%> (+14.16%) ⬆️
x/evm/keeper/keeper.go 64.63% <0.00%> (+4.26%) ⬆️
x/evm/keeper/abci.go 26.66% <0.00%> (+26.66%) ⬆️
x/evm/keeper/state_transition.go 62.50% <0.00%> (+57.58%) ⬆️
x/evm/keeper/msg_server.go 69.69% <0.00%> (+69.69%) ⬆️

@yihuang
Copy link
Contributor Author

yihuang commented Jul 20, 2021

One of the test cases currently fail for estimated gas number different from the one returned by geth.

Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

ACK, one minor suggestion

@yihuang yihuang force-pushed the keepertest branch 2 times, most recently from 7cf3532 to 8396077 Compare July 21, 2021 04:08
@yihuang
Copy link
Contributor Author

yihuang commented Jul 21, 2021

One of the test cases currently fail for estimated gas number different from the one returned by geth.

the test case failure is due to unclean context, now added capability for test suite to commit and begin a new block.

@yihuang yihuang force-pushed the keepertest branch 4 times, most recently from 927ef00 to dba5786 Compare July 21, 2021 04:11
Also add some test environment setup, Closes #323

test estimateGas of erc20 token transfer

fix failed test case

the trick is to keep a clean transient store, by doing a commit

put artifacts to external file
@fedekunze
Copy link
Contributor

the test case failure is due to unclean context, now added capability for test suite to commit and begin a new block.

can you fix the failing test? Then we can merge this PR

@yihuang
Copy link
Contributor Author

yihuang commented Jul 23, 2021

the test case failure is due to unclean context, now added capability for test suite to commit and begin a new block.

can you fix the failing test? Then we can merge this PR

that one is already fixed, current failure seems caused by something else, I'm checking it now.

@yihuang
Copy link
Contributor Author

yihuang commented Jul 23, 2021

the test case failure is due to unclean context, now added capability for test suite to commit and begin a new block.

can you fix the failing test? Then we can merge this PR

fixed now, remove the commit in test suite setup.

@fedekunze fedekunze enabled auto-merge (squash) July 23, 2021 14:16
@fedekunze fedekunze merged commit f6dc80d into evmos:main Jul 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setup more sophisticated testing environment in unit test suite
2 participants