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

[Bug]: Local snapshots do not match CI snapshots for fuzz tests #216

Closed
snreynolds opened this issue Jun 13, 2023 · 1 comment
Closed

[Bug]: Local snapshots do not match CI snapshots for fuzz tests #216

snreynolds opened this issue Jun 13, 2023 · 1 comment
Assignees
@snreynolds
Labels
bug Something isn't working p0 Very important to fix

Comments

@snreynolds
Copy link
Member

snreynolds commented Jun 13, 2023 •
edited
Loading

Describe the bug

There is an ongoing issue where forge snapshot locally does not match the CI snapshots. It is failing on on fuzz tests. I've checked the following which I thought could be related to this issue but are not:

  • Local and CI using same fuzz runs.
  • No fuzz seed is being set in either env, which defaults to none. But, forge snapshot should already be using a deterministic seed, so this shouldn't be a problem.

Expected Behavior

After updating the snapshot file locally, and pushing to your branch forge snapshot --check should pass in the linter.

To Reproduce

No response

Additional context

PR reverted here.
@snreynolds snreynolds added the bug Something isn't working label Jun 13, 2023
This was referenced Oct 12, 2023
Closed
Closed
@snreynolds snreynolds added the p0 Very important to fix label Oct 12, 2023
hyunchel added a commit to hyunchel/v4-core that referenced this issue Oct 16, 2023
@hyunchel
Compare gas snapshots in CI with a small tolerance
014ea88 
The previous attempt to assert identical gas usage in CI has been
reverted because a snapshot generated in CI was different from a
snapshot generated locally.

The diff still exist as of today, but amounts are minuscule. The amounts
range from 1 to tens. And these diffs exist in fuzz tests only.

This is a known bug in Foundry: foundry-rs/foundry#5689

This commit adds back gas snapshot, and check for a minimum diff in CI.
This way, snapshot diff tool can be used while waiting for the Foundry
issue to be fixed.

Resolves issue Uniswap#216
hyunchel added a commit to hyunchel/v4-core that referenced this issue Oct 16, 2023
@hyunchel
Compare gas snapshots in CI with a small tolerance
234fb76 
The previous attempt to assert identical gas usage in CI has been
reverted because a snapshot generated in CI was different from a
snapshot generated locally.

The diff still exist as of today, but amounts are minuscule. The amounts
range from 1 to tens. And these diffs exist in fuzz tests only.
This is a known bug in Foundry: foundry-rs/foundry#5689

This commit adds back gas snapshot, and check for a minimum diff in CI.
This way, snapshot diff tool can be used while waiting for the Foundry
issue to be fixed.

Resolves issue Uniswap#216
hyunchel added a commit to hyunchel/v4-core that referenced this issue Oct 16, 2023
@hyunchel
Compare gas snapshots in CI with a small tolerance
8d699dc 
The previous attempt to assert identical gas usage in CI has been
reverted because a snapshot generated in CI was different from a
snapshot generated locally.

The diff still exist as of today, but amounts are minuscule. The amounts
range from 1 to tens. And these diffs exist in fuzz tests only.
This is a known bug in Foundry: foundry-rs/foundry#5689

This commit adds back gas snapshot, and check for a small diff in CI.
By allowing just one percent of diff, forge snapshot diff tool can be
utilized while waiting for the Foundry issue to be fixed.

Resolves issue Uniswap#216
@hyunchel hyunchel mentioned this issue Oct 16, 2023
Closed
@zhongeric
Copy link
Contributor

Fixed by #401

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@snreynolds snreynolds

Labels
bug Something isn't working p0 Very important to fix
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@zhongeric @snreynolds