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

test: remove testFail* tests #643

Merged
merged 1 commit into from
Jan 31, 2025
Merged

Conversation

zerosnacks
Copy link
Member

@zerosnacks zerosnacks commented Jan 9, 2025

In preparation to deprecate testFail*: foundry-rs/foundry#9574

  • Removes deployMockERC721 and deployMockERC20 from StdUtils as their tests includes large amount of testFail* tests and is generally outdated. I haven't been able to find the deploy* methods being used in any public project on Github.
  • Implements workaround for other cases

Alternatively we could upstream Solady's mocks and tests (which have been updated to not use testFail* but this feels out of place and would be incomplete (ERC4626, ERC6909, etc.. https://github.com/Vectorized/solady/tree/main/test/utils/mocks). I think it is most common for people to rely on the mocks their library comes with (be it Solmate, Solady or OpenZeppelin) or write their own wrapper around the token implementation.

@zerosnacks
Copy link
Member Author

zerosnacks commented Jan 9, 2025

cc @mds1 if you would like to retain the deploy utility methods I could port over Solady's test suite which does not have testFail* methods and is largely similar as it also initially forked Solmate's test suite

Related: #469 (comment)

@grandizzy
Copy link
Contributor

ping @mds1 could you please check comment above? thank you!

@DaniPopes DaniPopes changed the title chore: deprecate testFail* in forge-std tests test: remove testFail* tests Jan 27, 2025
@zerosnacks zerosnacks merged commit 4645871 into master Jan 31, 2025
3 checks passed
@zerosnacks zerosnacks deleted the zerosnacks/deprecate-fail branch January 31, 2025 11:23
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