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

evm: fixed commented out simulations, pubsub, and handler tests #655

Merged
merged 2 commits into from
Oct 11, 2021

Conversation

tomtau
Copy link
Contributor

@tomtau tomtau commented Oct 11, 2021

Closes: #640

Description

simulations are very basic: they can be built and executed,
but they don't generate any EVM-related transactions yet.
(It should be a matter of adding simulation-related code to the
modules + potentially extra helpers to the simulation.)
handler tests miss some extra assertions due to changes
in the return values snapshotting logic (ADR-001 and ADR-002).

Besides the test suites identified in the audit,
there's also "importer_test.go" which wasn't yet fixed.
(it'd require major rewriting + extra test resources)


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)

fixes evmos#640

simulations are very basic: they can be built and executed,
but they don't generate any EVM-related transactions yet.
(It should be a matter of adding simulation-related code to the
modules + potentially extra helpers to the simulation.)
handler tests miss some extra assertions due to changes
in the return values snapshotting logic (ADR-001 and ADR-002).

Besides the test suites identified in the audit,
there's also "importer_test.go" which wasn't yet fixed.
(it'd require major rewriting + extra test resources)
@codecov
Copy link

codecov bot commented Oct 11, 2021

Codecov Report

Merging #655 (5be9a6f) into main (bc8c87c) will increase coverage by 1.15%.
The diff coverage is 18.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #655      +/-   ##
==========================================
+ Coverage   55.88%   57.04%   +1.15%     
==========================================
  Files          63       63              
  Lines        5518     5534      +16     
==========================================
+ Hits         3084     3157      +73     
+ Misses       2271     2214      -57     
  Partials      163      163              
Impacted Files Coverage Δ
x/evm/types/genesis.go 80.76% <0.00%> (-19.24%) ⬇️
x/evm/module.go 54.68% <11.11%> (-7.14%) ⬇️
app/app.go 83.80% <100.00%> (+0.08%) ⬆️
x/evm/handler.go 75.00% <0.00%> (+66.66%) ⬆️
rpc/ethereum/pubsub/pubsub.go 78.48% <0.00%> (+78.48%) ⬆️

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, although the simulation pkg on the SDK doesn't work with EthAccounts and eth_secp256k1 keys, so it's not possible to simulate/fuzz EVM txs. Let's open a follow up issue to track that

@fedekunze fedekunze merged commit e91ec58 into evmos:main Oct 11, 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.

Commented out test suites
2 participants