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

test: assert events emitted from contract #216

Merged
merged 36 commits into from
Sep 13, 2024

Conversation

chungquantin
Copy link
Collaborator

@chungquantin chungquantin commented Aug 19, 2024

Link to issue: #181
Refactor fungible integration-tests contract to emit events by default and catch those events in the integration test cases.

@codecov-commenter
Copy link

codecov-commenter commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.89%. Comparing base (7bbc04c) to head (c6d390d).
Report is 2 commits behind head on daan/api.

@@             Coverage Diff              @@
##           daan/api     #216      +/-   ##
============================================
+ Coverage     45.92%   48.89%   +2.97%     
============================================
  Files            47       47              
  Lines          4279     4532     +253     
  Branches       4279     4532     +253     
============================================
+ Hits           1965     2216     +251     
- Misses         2252     2254       +2     
  Partials         62       62              

see 10 files with indirect coverage changes

@chungquantin
Copy link
Collaborator Author

@Daanvdplas Could I receive review for this PR?

Copy link
Collaborator

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

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

Looking good! Left small nitpicks.

This PR is going to daan/api but I'm not sure whether we want this. I say we really focus on getting frank/extension merged and then rebase those small PRs.

pop-api/integration-tests/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pallets/api/src/fungibles/mod.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@evilrobot-01 evilrobot-01 left a comment

Choose a reason for hiding this comment

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

A few things need tweaking I'm afraid: return type of call not checked before emitting events, token terminology throughout (may want to get that PR merged first) and then it needs conflicts resolving.

pallets/api/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/Cargo.toml Outdated Show resolved Hide resolved
pop-api/integration-tests/contracts/fungibles/lib.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/src/fungibles/utils.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/src/fungibles/utils.rs Outdated Show resolved Hide resolved
pop-api/src/primitives.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/contracts/fungibles/lib.rs Outdated Show resolved Hide resolved
pop-api/src/v0/fungibles.rs Outdated Show resolved Hide resolved
Base automatically changed from daan/refactor-api to daan/api September 13, 2024 14:50
Copy link
Collaborator

@evilrobot-01 evilrobot-01 left a comment

Choose a reason for hiding this comment

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

Looks good, pleased with the event naming.

@Daanvdplas Daanvdplas merged commit bf27640 into daan/api Sep 13, 2024
10 checks passed
@Daanvdplas Daanvdplas deleted the chungquantin/test-api_events branch September 13, 2024 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants