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

feat(cheatcodes): add vm.expectMockCall to enforce that a mocked function is called with certain arguments #5480

Open
JasoonS opened this issue May 4, 2022 · 7 comments
Labels
A-cheatcodes Area: cheatcodes P-low Priority: low T-feature Type: feature T-to-discuss Type: requires discussion

Comments

@JasoonS
Copy link

JasoonS commented May 4, 2022

Component

Forge

Describe the feature you would like

Currently tests don't break if a mocked function is called with different arguments to what was expected.

Currently for example:

    cheats.mockCall(
      address(mocker),
      abi.encodeWithSelector(
        SomeContract.someFunction.selector,
        5
      ),
      abi.encode(6)
    );

With the above code it will return 6 if the function is called with an argument of 5 or anything else, additionally it won't fail if it isn't called with 5 (which is something you sometimes want to check.

I propose adding a function called something like mockCallStrict to the cheats so that tests can be more strict about this.

Additional context

This is a feature of Smock: https://smock.readthedocs.io/en/latest/fakes.html#asserting-call-arguments

@JasoonS JasoonS added the T-feature Type: feature label May 4, 2022
@onbjerg
Copy link
Member

onbjerg commented May 4, 2022

You might be interested in expectCall instead. I don't think mockCall should be a superset of expectCall. Sometimes you definitely do want to only mock for a certain parameter, but still retain the ability to get real data for all other cases. If you want a stricter version perhaps this is a good addition to Forge Std instead as a wrapper around expectCall + mockCall

@gakonst
Copy link
Member

gakonst commented May 4, 2022

adding a mock+expect combination in forge-std makes sense to me

@JasoonS
Copy link
Author

JasoonS commented May 5, 2022

Thanks, that makes sense to me too.

I won't make any changes to the std lib right now, but I'm working on some code-gen stuff that might be useful for others once it is done to assist with mocking.
Maybe that can be upstreamed at some point if it is useful.

@JasoonS
Copy link
Author

JasoonS commented May 5, 2022

Also FYI - my codegen work will allow mocking of internal functions without needing to move the the REVM as mentioned in this PR - #432 .

Like I say, I'll definitely share my work on that in the coming weeks/months 👍

@onbjerg
Copy link
Member

onbjerg commented May 5, 2022

@brockelmore Perhaps we should transfer this to Forge Std as well? Or maybe just close it and open up a new issue? Wdyt?

@brockelmore brockelmore transferred this issue from foundry-rs/foundry May 5, 2022
@mds1
Copy link
Collaborator

mds1 commented Jul 26, 2023

Per #3782, I'm going to move this back to the foundry repo

I'm also not too clear on the exact proposal here, I'd be curious to see sample code of that the cheat/UX would look like

@mds1 mds1 transferred this issue from foundry-rs/forge-std Jul 26, 2023
@zerosnacks zerosnacks added this to the v1.0.0 milestone Jul 26, 2024
@zerosnacks zerosnacks added the A-cheatcodes Area: cheatcodes label Aug 1, 2024
@zerosnacks zerosnacks changed the title Add a way to enforce that a mocked function is called with certain arguments feat(cheatcodes): add vm.expectMockCall to enforce that a mocked function is called with certain arguments Aug 1, 2024
@zerosnacks zerosnacks added the P-low Priority: low label Sep 11, 2024
@grandizzy grandizzy removed this from the v1.0.0 milestone Nov 5, 2024
@grandizzy grandizzy added the T-to-discuss Type: requires discussion label Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cheatcodes Area: cheatcodes P-low Priority: low T-feature Type: feature T-to-discuss Type: requires discussion
Projects
Status: Todo
Development

No branches or pull requests

7 participants
@JasoonS @onbjerg @mds1 @gakonst @grandizzy @zerosnacks and others