0

I am trying to verify a mocked service was called with an arguments in which the last is an iterator. This is the assertion in the test:

verify(myService).myMethod(
    ...,
    argThat(dataIterator -> iteratorEquals(dataIterator, dataToSave.iterator())));

and I have this TestHelper method:

public class TestHelpers {

    public static <T> boolean iteratorEquals(Iterator<T> x, Iterator<T> y) {
        while (x.hasNext() && y.hasNext()) {
            if (x.next() != y.next()) {
                return false;
            }
        }
        return x.hasNext() == y.hasNext();
    }
}

I was debugging the static method and it seems like the value returned from it is indeed true. Also, when debugging I can see the arguments passed to the service are as expected, but the assertion for some reason will fail in this case. When I am changing the assertion to this:

verify(myService).myMethod(
    ...,
    any());

the test will pass, meaning the problem is, indeed, with the iterator argument. This is the error I receive when the test fails:

myService.myMethod( ..., <custom argument matcher> ); Wanted 1 time: -> at ...(StorageClientTest.java:91) But was 0 times.

org.mockito.exceptions.verification.TooFewActualInvocations: myService.myMethod( ..., <custom argument matcher> ); Wanted 1 time: -> at ...(StorageClientTest.java:91) But was 0 times.

Am I doing something wrong here? why does the test fail?

Tom Carmi
  • 385
  • 1
  • 5
  • 18
  • Sorry I think the title is misleading I'll change it - the method I am verifying is not static, the method I am using as a test helper to check the arg passed (iterator) is a static method – Tom Carmi Oct 21 '22 at 08:45
  • Try to clarify a bit the question description please. When exactly does the test fail and when does it pass? What parameters do you pass in both cases and what are the actual code invocations performed? – mrkachariker Oct 21 '22 at 10:31
  • I was trying to clarify the 2 cases. But I am not sure why do we care for the actual parameters or code invocations. The other parameters are ok, otherwise, it won't pass when changing only the last argument to any(). – Tom Carmi Oct 21 '22 at 18:29

2 Answers2

0

I've never used Mockito.argThat, partially because I never had the need in defining custom argument matcher.

When I understand your use case correctly, then you want to test some class which used a stub of type MyService. And then you want to verify that the stub was called with an Iterable and that the elements in the Iterable have the same elements in the same order as another iterable.

In your case, I would use an ArgumentCaptor<> instead and then verify the captured argument against some expected values like that.

@ExtendWith(MockitoExtension.class)
class MyControllerTest {

    @Captor
    ArgumentCaptor<List<Data>> dataArgumentCaptor;

    @Test
    void save() {
        // Arrange
        final var service = mock(MyService.class);
        final var controller = new MyController(service);
        final var myExpectedData = List.of(new Data("Foo"), new Data("Bar"));
        
        // Act
        controller.save("Foo", "Bar");

        // Assert
        verify(service).save(dataArgumentCaptor.capture());
        assertThat(dataArgumentCaptor.getValue()).containsExactlyElementsOf(myExpectedData);
        // when you dont use assertj you could assert them also with plain junit like that:
        assertArrayEquals(dataToSave.toArray(), myExpectedData.toArray());
    }

    class MyService {
        public void save(List<Data> data) {
        }
    }

    record Data(String n) {
    }

    class MyController {
        private final MyService service;

        public MyController(MyService service) {
            this.service = service;
        }

        public void save(String... data) {
            final var list = Arrays.stream(data).map(Data::new).toList();
            service.save(list);
        }
    }
}

And for why your test passes, when you use any() instead, is that you only verify that your stub was called with something, but it is certainly not equal to what you expect.

And by using your TestHelpers method, which only returns true or false, you gain a better overview through assertj which elements you called it with and where it differs from the expected ones.

Valerij Dobler
  • 1,848
  • 15
  • 25
  • Thx for the response. I am aware of how to use `ArgumentCaptor`, but prefer using `argThat ` since it is less verbose and more readable in my opinion. I am not looking for alternatives, but want to understand why what I did does not work. About the `any()` - I understand why it works when using it, just tried to give it as proof that this argument is the one that causing the mismatch in the assertion. – Tom Carmi Oct 21 '22 at 18:10
  • Your custom argument captor will only display to you, that it didn't match, instead of showing you what was not matching. I think it is not the intended use of argThat. And by misusing it, you write more complex tests. I, for example, was looking very long at your code to understand what it is, that you try to accomplish. – Valerij Dobler Oct 21 '22 at 18:53
  • When you try to minimize verbosity, the test becomes more complex and harder to debug. Consider that code is read 10x more often than written. – Valerij Dobler Oct 21 '22 at 18:54
  • And by misusing the Mockito API you are obfuscating the true problem. For one, it might be that you check your `dataToSave` via reference equal `x.next() != y.next()`. junit and assertj would use the equals method. – Valerij Dobler Oct 21 '22 at 19:04
  • 1. I agree, the advantage of `ArgumentCaptor` is in the visibility of the actual argument, but since I am looking for a standard reusable tool to check the equality of iterators I am less worried about it. I only need to make it work once, then I can just reuse it. – Tom Carmi Oct 23 '22 at 05:23
  • 2. I am not sure why you say I am misusing it all the time? – Tom Carmi Oct 23 '22 at 05:25
  • 3. The problem is not in `.iteratorEquals` method, since as I've said I was debugging it and the returned value is ok. – Tom Carmi Oct 23 '22 at 05:27
  • 4. I think that my code is much more `clean code` than the one you've supplied (and that is before the fact I'll need to extract the desired argument from the dataArgumentCaptor). But here is the main issue here, it becomes opinion based, so I think we should drop it :). I want to find a way to use argThat, if you can help with that I'd appreciate it. – Tom Carmi Oct 23 '22 at 05:32
  • I can't help you with argThat, since the problem described in your question is not [reproducible](https://stackoverflow.com/help/minimal-reproducible-example). – Valerij Dobler Oct 23 '22 at 19:06
  • Your DIY iteratorEquals has the same information density as `assertTrue(expected == actuall)` instead of the better solutions I proposed to you earlier, which not only show you, that in case of a failing test that they failed but also with what passed and expected results. And by quoting Uncle Bob, `Bad code tries to do too much, it has muddled intent and ambiguity of purpose. Clean code is focused.` – Valerij Dobler Oct 23 '22 at 19:24
0

The solution to this issue is to change the signature of the generic help method to return ArgumentMatcher and to move the lambda expression inside it:

public <T> ArgumentMatcher<Iterator<T>> equalsIterator(Iterator<T> compareTo) {
    return x -> {
        while (x.hasNext() && compareTo.hasNext()) {
            if (x.next() != compareTo.next()) {
                return false;
            }
        }
        return x.hasNext() == compareTo.hasNext();
    };
}

and I am calling it this way:

verify(myService).myMethod(
    ...,
    argThat(equalsIterator(dataToSave.iterator())));
Tom Carmi
  • 385
  • 1
  • 5
  • 18