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

gas snapshot values disagree on local (mac) vs CI (linux) #5199

Closed
1 of 2 tasks
thedavidmeister opened this issue Jun 22, 2023 · 11 comments · Fixed by #7951
Closed
1 of 2 tasks

gas snapshot values disagree on local (mac) vs CI (linux) #5199

thedavidmeister opened this issue Jun 22, 2023 · 11 comments · Fixed by #7951
Labels
T-bug Type: bug

Comments

@thedavidmeister
Copy link

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (a81d36f 2023-06-22T00:11:15.533732000Z)

What command(s) is the bug in?

forge snapshot

Operating System

macOS (Apple Silicon)

Describe the bug

Running forge snapshot locally produces a reliable output that i can verify with --check but when i run the same on CI the results disagree.

This leads to it often being impossible for me to get CI to pass without ignoring gas snapshotting.

An example is https://github.com/rainprotocol/rain.interpreter/actions/runs/5344581362/jobs/9689123930?pr=3

I just ran this locally and checked it, but CI reports changes.

Sometimes the difference is very small and sometimes quite large, which makes me think it has something to do with a seed, or the way the seed translates into fuzzed values.

@thedavidmeister thedavidmeister added the T-bug Type: bug label Jun 22, 2023
@mattsse
Copy link
Member

mattsse commented Jun 22, 2023

These look like fuzz tests?

could you try after setting a random seed value

for example 1000 as hex

[fuzz]
seed = '0x3e8'

this could still lead to some diffs, especially cross platform, windows in particular.

perhaps we need some additional settings for fuzz snapshots

@thedavidmeister
Copy link
Author

it did not help @mattsse

https://github.com/rainprotocol/rain.interpreter/actions/runs/5434768304/jobs/9883422715?pr=4

i added the seed you suggested and rebuilt the snapshot locally

still does not match CI

@mds1
Copy link
Collaborator

mds1 commented Jul 2, 2023

@snreynolds I recall from the foundry support telegram you also had this issue, wondering if you figured it out and it was user error or if there's a forge bug here?

@snreynolds
Copy link

HI @mds1 - No this was never resolved. Put up a draft here so you can see the replication. Its failing on the fuzz results. I read though that snapshots should already have a fixed seed automatically?

@thedavidmeister
Copy link
Author

as i understand, not only should snapshots already have a fixed seed, but setting a fixed seed for all tests would mean that you lose the benefit of additional coverage from new seeds each test run

@snreynolds
Copy link

as i understand, not only should snapshots already have a fixed seed, but setting a fixed seed for all tests would mean that you lose the benefit of additional coverage from new seeds each test run

Yeah, that is how I understand it as well. We don't want all our fuzz tests to run with a fixed seed, we just need the snapshots to run with a seed. But afaik they should already be running with a seed so I'm not sure why its breaking in CI.

@0xGuybrush
Copy link

I'm getting a version of this — weirder, is that I disabled CI fuzz tests runs when doing snapshot --check when this first cropped up for us a few months ago, but CI suddenly started failing, last week.

I've noticed when running forge snapshot locally on macOS that some fuzz tests run once more than what is defined by runs — so by default run 257 times, whereas other tests correctly run 256.

This happens too when I'm trying to suppress fuzz runs entirely (either via FOUNDRY_FUZZ_RUNS=0 or via the foundry.toml, runs=0).

I've just tried a forge init and can't reproduce on a fresh codebase so going to see if I can spot the difference…

@grandizzy
Copy link
Collaborator

grandizzy commented May 20, 2024

I'm getting a version of this — weirder, is that I disabled CI fuzz tests runs when doing snapshot --check when this first cropped up for us a few months ago, but CI suddenly started failing, last week.

I've noticed when running forge snapshot locally on macOS that some fuzz tests run once more than what is defined by runs — so by default run 257 times, whereas other tests correctly run 256.

This happens too when I'm trying to suppress fuzz runs entirely (either via FOUNDRY_FUZZ_RUNS=0 or via the foundry.toml, runs=0).

I've just tried a forge init and can't reproduce on a fresh codebase so going to see if I can spot the difference…

@0xGuybrush pls run forge clean if you had any test failing as we're now persisting failures and replay regressions (hence 257 instead 256)

@0xGuybrush
Copy link

0xGuybrush commented May 20, 2024

Hi @grandizzy, thanks very much! This resolved the N+1 difference in the test runs for me alright! Still seeing the original issue (that gas runs don't align across macOS locally and linux agent on Github CI) but at least we can do a forge snapshot --check against fuzzless tests again now.

@grandizzy
Copy link
Collaborator

Hi @grandizzy, thanks very much! This resolved the N+1 difference in the test runs for me alright! Still seeing the original issue (that gas runs don't align across macOS locally and linux agent on Github CI) but at least we can do a forge snapshot --check against fuzzless tests again now.

@0xGuybrush can you please try workaround here and lmk if consistent results? #7942 (comment)

@0xGuybrush
Copy link

Hi @grandizzy, thanks very much! This resolved the N+1 difference in the test runs for me alright! Still seeing the original issue (that gas runs don't align across macOS locally and linux agent on Github CI) but at least we can do a forge snapshot --check against fuzzless tests again now.

@0xGuybrush can you please try workaround here and lmk if consistent results? #7942 (comment)

this works! amazing, thanks very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants