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

Assertions on parameters during verification #2285

Closed
Saberos opened this issue May 6, 2021 · 1 comment · Fixed by #2949
Closed

Assertions on parameters during verification #2285

Saberos opened this issue May 6, 2021 · 1 comment · Fixed by #2949

Comments

@Saberos
Copy link

Saberos commented May 6, 2021

Suggestion

I would like to suggest adding an API allowing for assertions during parameter verification without argument captors in simple cases.

Motivation

In my projects I generally use Mockito and AssertJ for testing. For non trivial method parameters, I often encounter the following ArgumentCaptor pattern:

ClassUnderTest classUnderTest = new ClassUnderTest(otherServiceMock);

classUnderTest.method();

ArgumentCaptor<Param> paramArgumentCaptor = ArgumentCaptor.forClass(Param.class);
verify(otherServiceMock).doStuff(paramArgumentCaptor.capture());
Param paramFromCaptor = paramArgumentCaptor.getValue();
assertThat(paramFromCaptor.getField1()).isEqualTo("value1");
assertThat(paramFromCaptor.getField2()).isEqualTo("value2");

This is fine, but a little bit verbose.

An alternative pattern I find myself using lately is using the argThat argument matcher to eliminate the captor, resulting in the verify statement looking something like this:

verify(otherServiceMock).doStuff(argThat(param -> {
    assertThat(param.getField1()).isEqualTo("value1");
    assertThat(param.getField2()).isEqualTo("value2");
    return true;
}));

In my opinion, at least in simple cases where only assertions on a single parameter are desired, this looks a little cleaner. Just the return statement is a bit pointless.

My suggestion is adding an API that would allow eliminating the return statement in this pattern, for example:

verify(otherServiceMock).doStuff(assertArg(param -> {
    assertThat(param.getField1()).isEqualTo("value1");
    assertThat(param.getField2()).isEqualTo("value2");
}));

Or is there already an API that would allow a similar pattern that I've missed?

@sbernard31
Copy link

I also like the idea about removing this return true statement when I wrote something which is more "assertion" than "matching".

But it seems there is some limitation about this, see : argThat plus asserts.
Not sure to understand details, maybe a Mockito committer could explain this ?

Ideally, I think it would be great if Mockito provide an elegant way to use AssertJ to match arguments and/or more generally provide a way to assert arguments instead of matching it.

Waiting, you could implement your own :
The general way (not dependent of AssertJ - source : assertj/assertj#972 (comment))

public static <T> T assertArg(Consumer<T> assertion) {
    return argThat(argument -> {
        assertion.accept(argument);
        return true;
    });
}

The AssertJ oriented (depend of AssertJ and unfortunately Hamcrest too... - source : https://gist.github.com/superbob/091489c61ce9bd79d14a4d3d731c9c49?permalink_comment_id=3863242#gistcomment-3863242)

import org.assertj.core.matcher.AssertionMatcher;
import org.mockito.hamcrest.MockitoHamcrest;

import java.util.function.Consumer;

public class TestUtil {

    public static <T> T assertArg(Consumer<T> assertions) {
        return MockitoHamcrest.argThat(new AssertionMatcher<T>() {
            @Override
            public void assertion(T actual) throws AssertionError {
                assertions.accept(actual);
            }
        });
    }
}

I didn't dig to understand what is the benefits of AssertJ oriented solution compare to general way ... maybe better miss match report ? 🤷
if the AssertJ oriented solution is better, I guess it should be great to have one without Hamcrest dependency.

maciejwalkowiak added a commit to maciejwalkowiak/mockito that referenced this issue Mar 25, 2023
maciejwalkowiak added a commit to maciejwalkowiak/mockito that referenced this issue Mar 25, 2023
maciejwalkowiak added a commit to maciejwalkowiak/mockito that referenced this issue Apr 3, 2023
maciejwalkowiak added a commit to maciejwalkowiak/mockito that referenced this issue Apr 5, 2023
maciejwalkowiak added a commit to maciejwalkowiak/mockito that referenced this issue Apr 11, 2023
TimvdLippe pushed a commit that referenced this issue Apr 11, 2023
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 a pull request may close this issue.

2 participants