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

feat: Refactor to integration testing setup (SC-567) #7

Merged
merged 26 commits into from
Aug 13, 2024

Conversation

lucas-manuel
Copy link
Contributor

@lucas-manuel lucas-manuel commented Aug 8, 2024

No description provided.

Copy link

linear bot commented Aug 8, 2024

Copy link
Contributor

@hexonaut hexonaut left a comment

Choose a reason for hiding this comment

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

I would rename PsmCalls -> PSMCalls and SNstCalls -> SNSTCalls to match the casing elsewhere.

Comment on lines 212 to 213
deal(address(usdc), address(pocket), 100e6); // Gem is held in pocket
deal(address(nst), address(PSM), 100e18);
Copy link
Contributor

Choose a reason for hiding this comment

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

There is already liquidity in here. I suppose this needs the psm wrapper for nst. This is okay for now, but I would aim to eliminate this as it is an artificial deviation from mainnet.

Copy link
Contributor

@hexonaut hexonaut left a comment

Choose a reason for hiding this comment

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

Forgot to set to request changes. Also, I DMed you the NST PSM wrapper I would use that.

@lucas-manuel lucas-manuel requested a review from hexonaut August 12, 2024 17:54
Copy link
Contributor

@hexonaut hexonaut left a comment

Choose a reason for hiding this comment

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

A few issues. MockJug is unused. Also, I noticed ALMProxy is missing a way to send ETH to it. It should have a receive() special function.

Not sure why CI is failing, but tests work locally.

Copy link

Coverage after merging sc-567-refactor-to-integration into master will be

42.86%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   ALMProxy.sol100%100%100%100%
   EthereumController.sol27.91%0%10%35.48%102, 106, 115, 121, 129, 135, 147, 153, 161, 172, 175, 181, 187, 193, 200, 203, 209, 215, 221, 93, 93, 93

@lucas-manuel lucas-manuel merged commit d0f7441 into master Aug 13, 2024
2 of 3 checks passed
@lucas-manuel lucas-manuel deleted the sc-567-refactor-to-integration branch August 13, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants