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

remove testFail from tests and prefer vm.expectRevert #23

Merged
merged 7 commits into from
Jan 5, 2024

Conversation

fedealconada
Copy link
Contributor

@fedealconada fedealconada commented Jan 3, 2024

  • Refactors all testFail* tests to use expectRevert() or expectEmit() in the case of the tests that expect the UserOperationRevertReason event to be emitted.
  • Also changed some test function method names
  • Fixed very minor typos
  • Added some helpers

Copy link

github-actions bot commented Jan 3, 2024

Slither report

THIS CHECKLIST IS NOT COMPLETE. Use --show-ignored-findings to show all the results.
Summary

solc-version

Impact: Informational
Confidence: High

  • ID-0
    solc-0.8.17 is not recommended for deployment

@fedealconada fedealconada force-pushed the test-fail-refactor branch 6 times, most recently from 3990755 to 66b1e48 Compare January 5, 2024 12:06
@fedealconada fedealconada marked this pull request as ready for review January 5, 2024 12:09
@fedealconada fedealconada force-pushed the test-fail-refactor branch 2 times, most recently from ebec3f5 to 62f506a Compare January 5, 2024 12:17
Copy link

codecov bot commented Jan 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0cb5b1b) 67.31% compared to head (50904e5) 73.72%.
Report is 3 commits behind head on main.

❗ Current head 50904e5 differs from pull request most recent head 03ee289. Consider uploading reports for the commit 03ee289 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #23      +/-   ##
==========================================
+ Coverage   67.31%   73.72%   +6.41%     
==========================================
  Files          10       10              
  Lines         413      411       -2     
  Branches      114      113       -1     
==========================================
+ Hits          278      303      +25     
+ Misses         51       48       -3     
+ Partials       84       60      -24     
Files Coverage Δ
src/wallet/KintoWallet.sol 72.17% <100.00%> (+9.56%) ⬆️
src/wallet/KintoWalletFactory.sol 73.91% <ø> (+9.32%) ⬆️

... and 5 files with indirect coverage changes

@fedealconada fedealconada force-pushed the test-fail-refactor branch 2 times, most recently from bd6239c to 0340b48 Compare January 5, 2024 12:31
Bubble up errors from `UserOperationRevertReason`
@fedealconada fedealconada merged commit b9911f0 into main Jan 5, 2024
5 checks passed
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.

2 participants