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

add snapshot to CI #331

Closed
wants to merge 25 commits into from
Closed

add snapshot to CI #331

wants to merge 25 commits into from

Conversation

snreynolds
Copy link
Member

Related Issue

Which issue does this pull request resolve?

We should be running forge snapshots on all PRs locally & on CI. There was an issue with getting the fuzz snapshots locally to match the CI. Hopefully we can resolve it in this PR.

Description of changes

@snreynolds
Copy link
Member Author

snreynolds commented Oct 12, 2023

trying to fix #216

@marktoda marktoda marked this pull request as ready for review October 31, 2023 18:20
[FAIL. Reason: The `vm.assume` cheatcode rejected too many inputs (65536 allowed)] testHookWithdrawFeeProtocolWithdrawFee(uint16,uint16) (runs: 5904, μ: 533782, ~: 538058)

TODO: fix above error
@marktoda
Copy link
Contributor

marktoda commented Oct 31, 2023

tl;dr had to set the fuzz seed in the environment because forge snapshot does not respect it from foundry.toml currently (issue below). So now we should use the justfile to run with the proper seed i.e. just snapshots. Can also source the .env file and run forge snapshot

Reference to foundry issue foundry-rs/foundry#6179

@snreynolds
Copy link
Member Author

closing this in favor of this #401 just squashes these, rebases main, and adds a tolerance

@snreynolds snreynolds closed this Nov 13, 2023
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