Add `ArgumentMatchers#assertArg` method. #2949

maciejwalkowiak · 2023-03-25T19:56:49Z

Enables convenient alternative to ArgumentCaptor to assert arguments during verification:

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

Verification succeeds as long as no exception is thrown from the lambda passed to assertArg, which means it works with any assertions (JUnit, AssertJ, ...).

Since this is my first PR to Mockito, I may have missed something, or perhaps the documentation should be improved. Feedback is very welcome!

Checklist

Read the contributing guide
PR should be motivated, i.e. what does it fix, why, and if relevant how
If possible / relevant include an example in the description, that could help all readers
including project members to get a better picture of the change
Avoid other runtime dependencies
Meaningful commit history ; intention is important please rebase your commit history so that each
commit is meaningful and help the people that will explore a change in 2 years
The pull request follows coding style
Mention Fixes #<issue number> in the description if relevant
At least one commit should mention Fixes #<issue number> if relevant

TimvdLippe

Simple and elegant, nice! I wonder if we can even deprecate ArgumentCaptor at this point in favor of this, but let's cross that bridge when we get to it.

TimvdLippe

Ah in fact, it is good to call out this new API in our main documentation. To do so, add section 55 to

mockito/src/main/java/org/mockito/Mockito.java

Line 1663 in 0b8685c

* </p>

and add it to the index at

mockito/src/main/java/org/mockito/Mockito.java

Line 114 in 0b8685c

    
            *      <a href="#54">54. Mocking/spying without specifying class (Since 4.9.0)</a><br/>

It's best to include an example usage (such as the one you included in your PR description).

codecov-commenter · 2023-03-30T18:45:47Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.04 🎉

Comparison is base (5486617) 85.70% compared to head (f9aeefe) 85.75%.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2949      +/-   ##
============================================
+ Coverage     85.70%   85.75%   +0.04%     
- Complexity     2865     2872       +7     
============================================
  Files           325      325              
  Lines          8696     8716      +20     
  Branches       1074     1076       +2     
============================================
+ Hits           7453     7474      +21     
  Misses          966      966              
+ Partials        277      276       -1

Impacted Files	Coverage Δ
src/main/java/org/mockito/Mockito.java	`96.55% <ø> (ø)`
src/main/java/org/mockito/ArgumentMatchers.java	`99.04% <100.00%> (+0.02%)`	⬆️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

maciejwalkowiak · 2023-04-03T06:13:16Z

@TimvdLippe Javadocs added (feel free to rephrase if you find better wording). I added "(Since 5.3.0)" but I am not sure in which version exactly it would be released.

crohit · 2023-04-03T07:08:24Z

Oops I didn't mean to approve! I was just really looking forward for this change and clicked the wrong button!.

maciejwalkowiak · 2023-04-05T05:12:24Z

@TimvdLippe checkstyle issues are fixed now. Please take another look when you have a minute

src/test/java/org/mockitousage/matchers/MatchersTest.java

…ertion

TimvdLippe

Awesome thank you!

sbernard31 · 2023-04-12T07:58:06Z

@TimvdLippe,

Simple and elegant, nice! I wonder if we can even deprecate ArgumentCaptor at this point in favor of this, but let's cross that bridge when we get to it.

Even if this new feature is great and more elegant than ArgumentCaptor in some cases.
This is less powerful than ArgumentCaptor and I don't think this is a good idea to deprecate or remove ArgumentCaptor.

We have some use cases where this will be hard to replace :

like capturing a arguument and then use it in another verify.
create some utility doing general validation on some argument but returning a captured one for more specific validation.

maciejwalkowiak · 2023-04-12T10:22:01Z

@sbernard31 I agree. An example when assertArg is not going to replace argument captors: https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/BannerTests.java#L105

TimvdLippe · 2023-04-13T18:23:15Z

@sbernard31 @maciejwalkowiak I understand that use case and I think you can achieve the same with assertArg:

AtomicReference<String> reference = new AtomicReference<>();
verify(mock).invocation(assertArg(arg -> reference.set(arg)));
verify(mock).secondInvocation(eq(reference.get()));

Hence I am not sure if we need a dedicate ArgumentCaptor class if a lambda suffices. But that was mostly speculation. No need to deprecate it already. Let's see how assertArg is used first, before we make any further decisions.

sbernard31 · 2023-04-14T08:38:57Z

@TimvdLippe you're right about it seems we can do the same with assertArg combining with AtomicReference and it seems I was wrong when I said "This is less powerful than ArgumentCaptor".

But If we compare ArgumentCaptors to assertArg+AtomicReference, I feel ArgumentCaptor is less verbose and more easy to read and this could be a good reason to keep it. But clearly a less strong reason that what I thought at the beginning.

// assertArg + AtomicReference
AtomicReference<String> reference = new AtomicReference<>();
verify(mock).invocation(assertArg(arg -> reference.set(arg)));
verify(mock).secondInvocation(eq(reference.get()));

// ArgumentCaptor
ArgumentCaptor<String> arg = new  ArgumentCaptor<>();
verify(mock).invocation(arg.capture());
verify(mock).secondInvocation(eq(arg.getValue()));

TimvdLippe · 2023-04-14T18:01:07Z

@sbernard31 Right, that's exactly what we should be monitoring in the years to come, which pattern is used more frequently than the other. For now, there is no need to deprecate it, I only saw the similarities between the two API's and hence my comment, but nothing more than that.

edwnmrtnz · 2023-05-13T02:05:34Z

Will this work in kotlin too? Tried it just now and the arg is null.

 @Test
    fun `should only fetch active movies`() = runBlocking {
        val repo = Mockito.mock(MoviesRepository::class.java)
        val sut = FetchMoviesUseCase(repo)

        sut.execute(page = 1)

        Mockito.verify(repo).fetch(assertArg { params ->
            Truth.assertThat(params["status"]).isEqualTo("active")
        })
        Unit
    }

Above code results to

assertArg {
            println(it)
        } must not be null
java.lang.NullPointerException: assertArg {
            println(it)
        } must not be null

In a couple of places, `argThat` was being called with a sequence of boolean expressions. That pattern of use is incorrect, and while all the expressions are evaluated only the last line of the block is returned and used by `argThat`. Any of the preceding expressions could evaluate to false without failing the test. The most direct fix to this would be to take the conjunction of all the expressions by interspersing some `&&` between each. While that would work, it would also lead to terribly obscure error message on a failure, as it would be impossible to figure out which expression evaluated to false. The chosen fix is to instead use a series of assertions, and then return true at the end of the block. Given that any failed assertion raises an exception, this doesn't have the same issue as the old code. It would have been nice to avoid the confusing `true` at the end. In fact, Mockito has added a [new `assertArg` method][assertArg] in recent versions. I tried to update mockito but ran into too many issues to deal with. [assertArg]: mockito/mockito#2949

Fixes mockito#2285 : Add ArgumentMatchers#assertArg method.

f7b55d1

TimvdLippe approved these changes Mar 30, 2023

View reviewed changes

TimvdLippe requested changes Mar 30, 2023

View reviewed changes

Fixes mockito#2285 : Add Javadocs

cbf266b

crohit approved these changes Apr 3, 2023

View reviewed changes

Fixes mockito#2285 : Fix Javadocs

0ecdb54

TimvdLippe reviewed Apr 9, 2023

View reviewed changes

src/test/java/org/mockitousage/matchers/MatchersTest.java Show resolved Hide resolved

TimvdLippe mentioned this pull request Apr 9, 2023

Assertion matcher #2961

Closed

Fixes mockito#2285 : Add tests that verify mockito behavior after ass…

f9aeefe

…ertion

TimvdLippe approved these changes Apr 11, 2023

View reviewed changes

TimvdLippe merged commit 6ffd23e into mockito:main Apr 11, 2023

philwebb mentioned this pull request Apr 12, 2023

Replace Mockito argument captors with assertArg spring-projects/spring-boot#34943

Closed

wilkinsona mentioned this pull request Apr 19, 2023

Provide a variant of assertArg that works well with checked exceptions #2982

Closed

plietar mentioned this pull request Sep 5, 2024

Fix incorrect uses of argThat. mrc-ide/packit#118

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add `ArgumentMatchers#assertArg` method. #2949

Add `ArgumentMatchers#assertArg` method. #2949

Uh oh!

maciejwalkowiak commented Mar 25, 2023

Uh oh!

TimvdLippe left a comment

Uh oh!

TimvdLippe left a comment

Uh oh!

codecov-commenter commented Mar 30, 2023 •

edited

Loading

Uh oh!

maciejwalkowiak commented Apr 3, 2023

Uh oh!

crohit commented Apr 3, 2023

Uh oh!

maciejwalkowiak commented Apr 5, 2023

Uh oh!

Uh oh!

TimvdLippe left a comment

Uh oh!

sbernard31 commented Apr 12, 2023

Uh oh!

maciejwalkowiak commented Apr 12, 2023

Uh oh!

TimvdLippe commented Apr 13, 2023

Uh oh!

sbernard31 commented Apr 14, 2023

Uh oh!

TimvdLippe commented Apr 14, 2023

Uh oh!

edwnmrtnz commented May 13, 2023

Uh oh!

Uh oh!

Uh oh!

Add ArgumentMatchers#assertArg method. #2949

Add ArgumentMatchers#assertArg method. #2949

Uh oh!

Conversation

maciejwalkowiak commented Mar 25, 2023

Checklist

Uh oh!

TimvdLippe left a comment

Choose a reason for hiding this comment

Uh oh!

TimvdLippe left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Mar 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

maciejwalkowiak commented Apr 3, 2023

Uh oh!

crohit commented Apr 3, 2023

Uh oh!

maciejwalkowiak commented Apr 5, 2023

Uh oh!

Uh oh!

TimvdLippe left a comment

Choose a reason for hiding this comment

Uh oh!

sbernard31 commented Apr 12, 2023

Uh oh!

maciejwalkowiak commented Apr 12, 2023

Uh oh!

TimvdLippe commented Apr 13, 2023

Uh oh!

sbernard31 commented Apr 14, 2023

Uh oh!

TimvdLippe commented Apr 14, 2023

Uh oh!

edwnmrtnz commented May 13, 2023

Uh oh!

Uh oh!

Add `ArgumentMatchers#assertArg` method. #2949

Add `ArgumentMatchers#assertArg` method. #2949

codecov-commenter commented Mar 30, 2023 •

edited

Loading