Skip to content

Conversation

@zerosnacks
Copy link
Member

@zerosnacks zerosnacks commented Jan 30, 2025

Motivation

Closes: #9734

Unblocks: #9697 / #9710

Solution

Maintains backwards compatibility of FORGE_SNAPSHOT_CHECK with the added restriction of it requiring a boolean value (FORGE_SNAPSHOT_CHECK=true) in accordance to the other configuration items.

Evaluates in the following order:

Default is false

  • We check whether FORGE_SNAPSHOT_CHECK=true is set
  • We check whether gas_snapshot_check=true is set in the config
  • Last we check --gas-snapshot-check=<bool> which takes preference over all, both true and false

Marked as minor breaking due to FORGE_SNAPSHOT_CHECK now explicitly checking for truthiness, not just existence

@zerosnacks zerosnacks added the T-likely-breaking Type: requires changes that can be breaking label Jan 30, 2025
@zerosnacks zerosnacks changed the title feat: add gas_snapshot_check flag to config, update FORGE_SNAPSHOT_CHECK to feat: add gas_snapshot_check flag to config, fix FORGE_SNAPSHOT_CHECK behavior Jan 30, 2025
@zerosnacks zerosnacks marked this pull request as ready for review January 30, 2025 13:10
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

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

makes sense, have minor nit re snapshot check option

@zerosnacks zerosnacks requested a review from grandizzy January 30, 2025 13:58
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

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

lgtm

@zerosnacks zerosnacks enabled auto-merge (squash) January 30, 2025 14:54
@zerosnacks zerosnacks merged commit fe92e7e into master Jan 30, 2025
22 checks passed
@zerosnacks zerosnacks deleted the zerosnacks/add-snapshot-flag-to-config branch January 30, 2025 15:09
zerosnacks added a commit to foundry-rs/book that referenced this pull request Jan 30, 2025
@zerosnacks zerosnacks added the C-forge Command: forge label Jan 30, 2025
@zerosnacks zerosnacks self-assigned this Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-forge Command: forge T-likely-breaking Type: requires changes that can be breaking

Projects

Status: Completed

Development

Successfully merging this pull request may close these issues.

feat: improve forge test configuration for checking gas section snapshots

2 participants