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.

Sign up for GitHub

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

Jump to bottom

Add tests for CirclesBackingFactory and CirclesBacking contracts #3

Merged
merged 6 commits into from
Jan 18, 2025
Merged

Add tests for CirclesBackingFactory and CirclesBacking contracts #3

merged 6 commits into from
Jan 18, 2025

Conversation

web3skeptic
Copy link
Contributor

Todo:

  • research if it's possible to reenter onERC1155Received function on CirclesBackingFactory.sol contract, no trivial ways have been found so far.
  • research whether CirclesBackingDeploymentFailed validation is needed. Normally the revert occurs when attempting to deploy a contract to the same address twice and CirclesBackingDeploymentFailed branch is unreachable.

Results:
forge coverage --no-match-coverage "src/prototype|test|script"

╭--------------------------------------------+------------------+------------------+----------------+-----------------╮
| File                                       | % Lines          | % Statements     | % Branches     | % Funcs         |
+=====================================================================================================================+
| src/CirclesBacking.sol                     | 100.00% (38/38)  | 100.00% (43/43)  | 100.00% (7/7)  | 100.00% (5/5)   |
|--------------------------------------------+------------------+------------------+----------------+-----------------|
| src/factory/CirclesBackingFactory.sol      | 99.18% (121/122) | 98.57% (138/140) | 80.00% (8/10)  | 100.00% (21/21) |
|--------------------------------------------+------------------+------------------+----------------+-----------------|
| src/helpers/RedeemHelper.sol               | 0.00% (0/4)      | 0.00% (0/3)      | 100.00% (0/0)  | 0.00% (0/1)     |
|--------------------------------------------+------------------+------------------+----------------+-----------------|
| src/proxy/CreateTestProxyLBPMintPolicy.sol | 100.00% (8/8)    | 100.00% (8/8)    | 100.00% (1/1)  | 100.00% (2/2)   |
|--------------------------------------------+------------------+------------------+----------------+-----------------|
| Total                                      | 97.09% (167/172) | 97.42% (189/194) | 88.89% (16/18) | 96.55% (28/29)  |
╰--------------------------------------------+------------------+------------------+----------------+-----------------╯

@roleengineer roleengineer changed the title Add tests for CirclesBackingFactory and CireclesBacking contracts Add tests for CirclesBackingFactory and CirclesBacking contracts Jan 16, 2025
roleengineer
roleengineer reviewed Jan 16, 2025
View reviewed changes
Copy link
Collaborator

@roleengineer roleengineer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great work! I've completed the review and i think once comments are resolved we will be able to merge.

benjaminbollen
benjaminbollen approved these changes Jan 17, 2025
View reviewed changes
Copy link
Member

@benjaminbollen benjaminbollen left a comment

Choose a reason for hiding this comment

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

overall looks great! (I havent gone in depth through the many tests). Great work!

@roleengineer roleengineer marked this pull request as ready for review January 18, 2025 15:48
roleengineer
roleengineer approved these changes Jan 18, 2025
View reviewed changes
Copy link
Collaborator

@roleengineer roleengineer left a comment

Choose a reason for hiding this comment

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

LGTM! I'll fix smth later.

@roleengineer roleengineer merged commit 99cb8cd into metri/testing Jan 18, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjaminbollen benjaminbollen benjaminbollen approved these changes

@roleengineer roleengineer roleengineer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@web3skeptic @benjaminbollen @roleengineer