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

2. refactor: increase coverage and refactor tests structure #46

Merged
merged 5 commits into from
Jan 23, 2024

Conversation

fedealconada
Copy link
Contributor

No description provided.

Copy link

Slither report

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

Copy link

codecov bot commented Jan 17, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (6f73fbc) 82.00% compared to head (b9b2cee) 83.01%.
Report is 10 commits behind head on main.

❗ Current head b9b2cee differs from pull request most recent head a2e8c7d. Consider uploading reports for the commit a2e8c7d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #46      +/-   ##
==========================================
+ Coverage   82.00%   83.01%   +1.00%     
==========================================
  Files           8        8              
  Lines         428      424       -4     
  Branches      114      113       -1     
==========================================
+ Hits          351      352       +1     
- Misses         25       26       +1     
+ Partials       52       46       -6     
Files Coverage Δ
src/apps/KintoAppRegistry.sol 84.09% <100.00%> (ø)
src/paymasters/SponsorPaymaster.sol 87.14% <100.00%> (ø)
src/wallet/KintoWallet.sol 78.35% <0.00%> (+5.15%) ⬆️
src/wallet/KintoWalletFactory.sol 64.00% <0.00%> (-2.67%) ⬇️

@fedealconada fedealconada requested a review from rrecuero January 17, 2024 20:02
@fedealconada fedealconada marked this pull request as ready for review January 17, 2024 20:02
@fedealconada fedealconada changed the title refactor: add some refactors and renames refactor: increase coverage and add some refactors and renames Jan 17, 2024
Copy link
Contributor

@rrecuero rrecuero left a comment

Choose a reason for hiding this comment

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

The PR looks good but I would only feel comfortable changing contract method names if we deploy and upgrade and include it in the PR. otherwise its a nightmare because our code doesnt reflect what is deployed and we can get crazy

@fedealconada fedealconada changed the base branch from main to modify-wallet-test-structure January 18, 2024 19:36
@fedealconada fedealconada force-pushed the increase-coverage-2 branch 2 times, most recently from b41be85 to 2f69616 Compare January 22, 2024 15:52
@fedealconada fedealconada changed the title refactor: increase coverage and add some refactors and renames refactor: increase coverage and refactor tests structure Jan 22, 2024
@fedealconada fedealconada changed the title refactor: increase coverage and refactor tests structure 2. refactor: increase coverage and refactor tests structure Jan 22, 2024
@fedealconada fedealconada changed the base branch from modify-wallet-test-structure to main January 22, 2024 22:16
@@ -270,8 +273,13 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto
bytes32 hash = userOpHash.toEthSignedMessageHash();
// If there is only one signature and there is an app Key, check it
address app = _getAppContract(userOp.callData);
console.log("APP", app);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove console logs

Copy link
Contributor

@rrecuero rrecuero 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 after removing the console logs

@fedealconada fedealconada merged commit 0cff01c into main Jan 23, 2024
1 check 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