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

refactor: tests for collection approval #419

Merged
merged 6 commits into from
Jan 7, 2025

Conversation

Daanvdplas
Copy link
Collaborator

@Daanvdplas Daanvdplas commented Jan 7, 2025

Refactors some of the test by separating the e.g. approve_collection_transfer and force_approve_collection_transfer, and while doing so found some small improvements.

@codecov-commenter
Copy link

codecov-commenter commented Jan 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.93%. Comparing base (97450fa) to head (866f128).
Report is 1 commits behind head on chungquantin/feat-nfts.

@@                    Coverage Diff                     @@
##           chungquantin/feat-nfts     #419      +/-   ##
==========================================================
+ Coverage                   70.48%   70.93%   +0.44%     
==========================================================
  Files                          72       72              
  Lines                       13119    13313     +194     
  Branches                    13119    13313     +194     
==========================================================
+ Hits                         9247     9443     +196     
  Misses                       3600     3600              
+ Partials                      272      270       -2     
Files with missing lines Coverage Δ
pallets/nfts/src/tests.rs 99.91% <100.00%> (+<0.01%) ⬆️

... and 1 file with indirect coverage changes

@@ -2120,6 +2069,64 @@ fn cancel_approval_works() {
});
}

#[test]
fn cancel_approval_ensures_no_active_collection_approval() {
Copy link
Collaborator Author

@Daanvdplas Daanvdplas Jan 7, 2025

Choose a reason for hiding this comment

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

Only removed the events test and another storage check as it was already checked in the cancel_approval_works test

),
Ok(Some(WeightOf::clear_collection_approvals(limit)).into())
);
approvals = approvals - limit;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added this approvals mutation and limit variable only

@chungquantin chungquantin changed the title refactor: approve_collection_transfer test refactor: tests for collection approval Jan 7, 2025
Copy link
Collaborator

@chungquantin chungquantin left a comment

Choose a reason for hiding this comment

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

LGTM! Very clean. Thank you.

@chungquantin chungquantin merged commit 881a04c into chungquantin/feat-nfts Jan 7, 2025
12 of 14 checks passed
@chungquantin chungquantin deleted the daan/refactor-tests_nfts branch January 7, 2025 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants