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

add exception revert test case #345

Closed
wants to merge 3 commits into from
Closed

Conversation

yihuang
Copy link
Contributor

@yihuang yihuang commented Jul 24, 2021

#338

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)

@yihuang yihuang marked this pull request as draft July 24, 2021 04:37
@yihuang
Copy link
Contributor Author

yihuang commented Jul 24, 2021

not sure how to get it picked up in CI.

})
it('should revert', async () => {
await revert.try_set(10)
no = await revert.query()
Copy link
Contributor

Choose a reason for hiding this comment

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

This will just throw and end the execution. You should use the expectRevert function which is defined above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the test case actually success on ganache, only fail on ethermint.

Copy link
Contributor

Choose a reason for hiding this comment

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

await revert.set(10) this can success on ganache?

Copy link
Contributor

Choose a reason for hiding this comment

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

okay I saw the code. Yeah it should be success.

@yijiasu-crypto
Copy link
Contributor

not sure how to get it picked up in CI.

Once the #278 is merged this will be automatically executed.

@codecov
Copy link

codecov bot commented Jul 26, 2021

Codecov Report

Merging #345 (0775537) into main (f6dc80d) will increase coverage by 0.17%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #345      +/-   ##
==========================================
+ Coverage   52.42%   52.59%   +0.17%     
==========================================
  Files          46       46              
  Lines        4614     4620       +6     
==========================================
+ Hits         2419     2430      +11     
+ Misses       2093     2090       -3     
+ Partials      102      100       -2     
Impacted Files Coverage Δ
app/ante/ante.go 48.71% <0.00%> (-0.64%) ⬇️
x/evm/types/logs.go 36.47% <0.00%> (-0.44%) ⬇️
x/evm/types/key.go 0.00% <0.00%> (ø)
x/evm/keeper/statedb.go 84.65% <0.00%> (+0.03%) ⬆️
x/evm/keeper/keeper.go 69.46% <0.00%> (+4.82%) ⬆️

@yihuang
Copy link
Contributor Author

yihuang commented Aug 5, 2021

will open another PR to fix this issue directly, closing this one.

@yihuang yihuang closed this Aug 5, 2021
yihuang referenced this pull request in yihuang/ethermint Sep 13, 2023
* Problem: block number is not filled in eth logs

Solution:
- fill in both block hash and block number in json-rpc
- remove the block hash in events, to save some space in tx_indexer

* fix unit test

* fix unit test

* fill in tx hash to logs

* fix block number

* cleanup

* remove tx log event
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.

2 participants