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

Datacap integration tests #735

Merged
merged 4 commits into from
Oct 7, 2022
Merged

Datacap integration tests #735

merged 4 commits into from
Oct 7, 2022

Conversation

ZenGround0
Copy link
Contributor

@ZenGround0 ZenGround0 commented Oct 4, 2022

Exercise the datacap actor directly and exercise error paths

Discovered the missing propagation of universal receiver hook errors back to data cap and modified the messenger trait implementation accordingly

@ZenGround0 ZenGround0 marked this pull request as ready for review October 5, 2022 20:57
@ZenGround0 ZenGround0 requested a review from anorth October 5, 2022 20:58
@ZenGround0 ZenGround0 force-pushed the test/datacap-token-itests branch 2 times, most recently from 29ddfbb to bf2892e Compare October 5, 2022 21:01
@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2022

Codecov Report

Merging #735 (0130c4d) into master (5fecc3a) will decrease coverage by 1.13%.
The diff coverage is 52.63%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #735      +/-   ##
==========================================
- Coverage   87.86%   86.73%   -1.14%     
==========================================
  Files          95       95              
  Lines       19796    19916     +120     
==========================================
- Hits        17394    17274     -120     
- Misses       2402     2642     +240     
Impacted Files Coverage Δ
actors/datacap/src/lib.rs 55.37% <52.63%> (-11.52%) ⬇️
test_vm/src/util.rs 84.23% <0.00%> (-15.68%) ⬇️
test_vm/src/lib.rs 89.31% <0.00%> (ø)
actors/miner/src/lib.rs 83.44% <0.00%> (+0.12%) ⬆️
actors/multisig/src/lib.rs 95.29% <0.00%> (+0.22%) ⬆️

Copy link
Member

@anorth anorth left a comment

Choose a reason for hiding this comment

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

These are good, thanks.

For #543 I think there is more required (as unit tests) to cover all of the non-generic logic of the datacap actor (e.g. the auto-allowance on mint, etc)

actors/datacap/src/lib.rs Show resolved Hide resolved
)
.unwrap(),
};
let clone_params = |x: &TransferFromParams| -> TransferFromParams {
Copy link
Member

Choose a reason for hiding this comment

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

Do you wanna file an issue in the token lib to derive Clone for all these types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks will do

* Propagate errors from universal receiver hook
* Test hitting many error cases and passing
@ZenGround0 ZenGround0 merged commit cf557e8 into master Oct 7, 2022
@ZenGround0 ZenGround0 deleted the test/datacap-token-itests branch October 7, 2022 18:29
shamb0 pushed a commit to shamb0/builtin-actors that referenced this pull request Jan 31, 2023
* Datacap Integration Test
* Propagate errors from universal receiver hook

Co-authored-by: zenground0 <ZenGround0@users.noreply.github.com>
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.

None yet

3 participants