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

[NO-3155] Fix usage of pranks in functions to make suite work with foundry@nightly #677

Merged
merged 2 commits into from
Aug 11, 2023

Conversation

jaycenhorton
Copy link
Contributor

@jaycenhorton jaycenhorton commented Aug 11, 2023

These changes fixes our integration with the latest versions of foundry. It seems something changed within the foundry stack that broke some of our test suites. The problematic test suites were all starting pranks in setUp functions without also calling stopPrank before the setUp function ended. It's unclear exactly why this broke (i suspect it's related to the changes in the revm dependency that was recently updated, but that's just a guess). Nonetheless it's a lot cleaner and easier to debug if we are using prank in more limited contexts (otherwise it can be hard to figure out what address is the currently active address, and this is especially true given that setup and test functions are initiated from two separate contexts IIRC).

The primary fix is outlined in this comment

I created a new issue in the foundry repo here

@swarmia
Copy link

swarmia bot commented Aug 11, 2023

test/Market.t.sol Show resolved Hide resolved
test/Market.t.sol Show resolved Hide resolved
test/Market.t.sol Show resolved Hide resolved
test/Market.t.sol Show resolved Hide resolved
test/Market.t.sol Show resolved Hide resolved
Copy link
Contributor

@amiecorso amiecorso left a comment

Choose a reason for hiding this comment

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

nice work figuring this out

@jaycenhorton jaycenhorton merged commit 8f666e2 into master Aug 11, 2023
6 checks passed
@jaycenhorton jaycenhorton deleted the jaycen-fix-pranks branch August 11, 2023 16:04
@amiecorso
Copy link
Contributor

I resolved all the conversations because I wanted this to automerge so I can pull it in to my priceMultiple PR but I left some responses

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