foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
8.33k stars 1.76k forks source link

Test feature for Internal and private tests #1040

Closed catmaxi closed 3 months ago

catmaxi commented 2 years ago

Component

Forge

Describe the feature you would like

It would be very neat to have a way to expose internal/private functions and states through perhaps the same syntax or perhaps a slightly different syntax automatically when using ds-test (perhaps I should make this PR in dapptools?) instead of using the current not ideal ways of testing them which from my knowledge are only:

  1. Test only the external/public functions that uses these internal/private fields and if those work correctly you can assume these also work.
  2. Make these internal/private fields public when you do test and then make them private/internal again once you're done (which defeats the purpose a bit).
  3. Make a new contract which inherits your contract which provides public/external APIs to the internal functions to test them. (this gets very complicated to manage when you have tons of inheritance and multiple overrides of the same function from different parents and multiple virtual functions which tends to happen when you use Openzeppelin's contracts.

all these 3 methods are definitely not ideal.

Could there be a way to automatically provide a way to call these functions? This is mostly useful for unit tests of view and pure functions to ensure they're always doing that specific task correctly.

thanks.

Additional context

No response

onbjerg commented 2 years ago

I think following method 3 is the most ideal, to be honest. We would have the exact same issue if we were to try and figure out what private/internal function you want to test - and also, internal/private functions have very different semantics than public/external ones, so this is pretty hard to do. They may be inlined in some cases and otherwise they are just jumps.

cc @gakonst wdyt?

catmaxi commented 2 years ago

I think following method 3 is the most ideal, to be honest. We would have the exact same issue if we were to try and figure out what private/internal function you want to test - and also, internal/private functions have very different semantics than public/external ones, so this is pretty hard to do. They may be inlined in some cases and otherwise they are just jumps.

@onbjerg Though I do agree that the semantics can be different, I think there can be a simpler way to extract the logic out to unit test it. So in this sense each unit test would basically be testing only one single internal/private function which should make it a bit easier to figure out I think?

Though Method 3 isn't too bad, but perhaps a method to automate this would be ideal? Say every internal private functions are added with double underscores or something like that? In any case this isn't anything urgent.

onbjerg commented 2 years ago

I've tentatively added this to the v1 milestone and assigned myself, but it may turn out to be impractical to implement so it might be removed from the milestone

zerosnacks commented 3 months ago

Marking as not planned, though complex in some situations 3 is the most practical and makes sure you aren't changing your implementation to be testable.