Skip to content

refactor: remove deprecated snapshot test suite #9772

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

VolodymyrBg
Copy link
Contributor

Pull Request: Remove deprecated snapshot test suite

Motivation

The current implementation in StateSnapshots.t.sol contains a deprecated test suite DeprecatedStateSnapshotTest that uses the old snapshot* API. This code was marked with a TODO comment for removal after the old API would be deprecated in favor of the new snapshotState* API.

Having two test suites testing the same functionality creates:

  1. Code redundancy
  2. Additional maintenance overhead
  3. Potential confusion for new developers
  4. Risk of using deprecated API in new code

Solution

I removed the deprecated DeprecatedStateSnapshotTest test suite and its associated TODO comment, keeping only the modern implementation in the StateSnapshotTest contract.

Changes include:

  1. Removal of the DeprecatedStateSnapshotTest contract
  2. Removal of the TODO comment
  3. Preservation of all functionality tests in the modern StateSnapshotTest contract

Testing

All functionality is fully covered in the remaining StateSnapshotTest contract, which includes checks for:

  • Creating and restoring state snapshots
  • Deleting snapshots
  • Deleting all snapshots
  • Multiple snapshot operations
  • Proper block value restoration

Implementation Notes

  • Original import paths are preserved as they are standard for the Foundry project
  • Linter warnings about import paths can be ignored as they are related to Foundry's specific dependency resolution system

Checklist

  • Code follows project style
  • All tests pass
  • Documentation is updated
  • Changes are backward compatible
  • Code is cleaned from deprecated comments and unused code

Related Issues

Closes #6411 (referenced in test comments)

Remove the deprecated StateSnapshotTest test suite that was using the old
snapshot* API. The functionality is now fully covered by the new
snapshotState* API in the main StateSnapshotTest contract.

This change:
- Removes redundant test cases
- Cleans up the codebase by removing deprecated code
- Makes the testing suite more maintainable
- Encourages usage of the new snapshotState* API
@zerosnacks
Copy link
Member

Hi @VolodymyrBg

Whilst we do plan on removing the old snapshot cheatcode aliases they are currently still active and the tests should cover them

Thanks!

@zerosnacks zerosnacks closed this Jan 28, 2025
@VolodymyrBg VolodymyrBg deleted the Alaska branch March 26, 2025 18:54
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Memory leak in forge vm.snapshot and vm.revertTo
2 participants