Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Integration tests. Move EVM tests to the generic framework #1801

Merged
merged 3 commits into from
Apr 16, 2024

Conversation

lemunozm
Copy link
Contributor

@lemunozm lemunozm commented Apr 8, 2024

Description

Fix partially #1790

Moving these tests to the generic framework:

  • Precompile test
  • Ethereum transaction test

NOTE: Blocked by polkadot-v1.1.0 READY

@lemunozm lemunozm added Q3-medium Can be done with good experience in the language, but little knowledge of the codebase. I4-tests Test needs fixing or improving. P5-soon Issue should be addressed soon. I6-refactoring Code needs refactoring. crcl-protocol Circle protocol related. labels Apr 8, 2024
@lemunozm lemunozm self-assigned this Apr 8, 2024
});
}

crate::test_for_runtimes!([development], call);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When trying with altair or centrifuge I obtain a BadOrigin error (when I call pallet_evm::call() or pallet_evm::create()). Do you know what could be the reason?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not from the top of my head unfortunately, no idea what might cause that.

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 guess some constant we use in the test does not match the runtime config... any idea @mustermeiszer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leaving this for a future PR

cdamian
cdamian previously approved these changes Apr 8, 2024
Copy link
Contributor

@cdamian cdamian left a comment

Choose a reason for hiding this comment

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

Thank you for cleaning these up @lemunozm!

Copy link

codecov bot commented Apr 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.53%. Comparing base (01178af) to head (cb221e0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1801      +/-   ##
==========================================
- Coverage   48.53%   48.53%   -0.01%     
==========================================
  Files         167      167              
  Lines       13311    13311              
==========================================
- Hits         6461     6460       -1     
- Misses       6850     6851       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lemunozm lemunozm requested a review from hieronx as a code owner April 15, 2024 10:25
@lemunozm lemunozm changed the base branch from polkadot-v1.1.0 to main April 15, 2024 10:26
@lemunozm lemunozm dismissed cdamian’s stale review April 15, 2024 10:26

The base branch was changed.

Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for moving this

@lemunozm lemunozm merged commit 79c5800 into main Apr 16, 2024
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crcl-protocol Circle protocol related. I4-tests Test needs fixing or improving. I6-refactoring Code needs refactoring. P5-soon Issue should be addressed soon. Q3-medium Can be done with good experience in the language, but little knowledge of the codebase.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants