9

Issue with unit test is that the same collection is processed differently in a stream and in a for loop. The collection in both cases is empty (data.size() = 0), but in the Case 1 that collection is somehow processed, in other words it will step in the for-loop. In Case 2, that collection is just skipped (which is expected since it's empty).

Tests are using Mockito, and Result<Record> is comming for JOOQ.

The tests are old and unchanged, the only change is going from for-loop to stream.

Case 1

private SearchResult iterateData(
      Result<Record> data, ...) {

      for (Record record : data) {
           doSomething(record);
    }

Case 2

private SearchResult iterateData(
      Result<Record> data, ...) {
      data.stream().forEach(record -> doSomething(record)); 

Screenshot of Case 1 for loop example

Mocked Result object

private DefaultSearchRequestModel rowSpecificValuesTestSetup(
      parameters...) {
    

    DefaultSearchRequestModel searchRequest = new DefaultSearchRequestModel(
        Arrays.asList(....),
        Collections.singletonList(
            new SearchFilter(
                "test",
                Collections.singletonList(...)));

    List<Column> columns =
        this.searchService.filterUserAllowedColumns(...);

    Condition searchCondition =
        this.searchRepositoryMock.getSearchConditions(...);

    List<TableJoinMapping> joinMappings = ColumnHelper.getColumnTranslateDeviceJoinMappings(
        columns,
        searchRequest.getFilters());

    Result<Record> deviceDataResultMock = Mockito.mock(Result.class);
    Iterator<Record> resultIterator = Mockito.mock(Iterator.class);
    final Table fromTableMock = Mockito.mock(Table.class);
    when(resultIterator.hasNext()).thenReturn(true, false);
    Record recordMock = Mockito.mock(Record.class);
    when(resultIterator.next()).thenReturn(recordMock);
    when(deviceDataResultMock.iterator()).thenReturn(resultIterator);
    when(recordMock.get(CONTRACTID)).thenReturn(contractId);
   ...
when(this.userPermissions.getAccessPermissions()).thenReturn(searchRequest.getColumns().stream().map
        (name -> Column.findByName(name).getId()).collect(
        Collectors.toList()));
    when(this.searchRepositoryMock.getCurrentTable(companyId))
        .thenReturn(fromTableMock);
    when(recordMock.get(TYPEID)).thenReturn(financialTypeId);
    when(this.searchRepositoryMock.getDeviceData(
        ArgumentMatchers.anyList(),
        ArgumentMatchers.anyList(),
        any(),
        any(),
        eq(searchRequest.getPageSize()),
        eq(searchRequest.getPage()),
        eq(searchRequest.getSortCriterias()),
        eq(fromTableMock),
        ArgumentMatchers.anyList(),
        eq(Optional.empty()),
        eq(this.dslContextMock)))
        .thenReturn(deviceDataResultMock);

    return searchRequest;
  }```
Nautilus
  • 113
  • 9
  • 1
    I can't believe that in Case 1 the for loop is entered. – Simon Martinelli Aug 23 '22 at 09:23
  • It doesn't make sense, I agree, but it does happen. Added a screenshot. – Nautilus Aug 23 '22 at 09:32
  • You need to provide information how it is mocked. I would assume you mock the method used for the foreach-loop, but not `data#stream`. Please provide a [mcve]. – daniu Aug 23 '22 at 09:35
  • @daniu yeah, like that. So the class is mocked Mockito.mock(SearcService.class), and the method is called where this foreach-loop is called. – Nautilus Aug 23 '22 at 09:42
  • The SearchService as such is irrelevant, `record` is what is being looped/streamd. In your screenshot it says that `record` is a mock, and its behavior depends on how you mock it - that's the point of mocking. – daniu Aug 23 '22 at 09:47
  • @daniu Oh that one, got it. Thank you. Record is built the same way for both stream/for-loop. The tests are old ones and they're not changed, the actual code is changed from for-loop to stream and this issue got up. – Nautilus Aug 23 '22 at 09:54
  • It doesn't matter how "Record is built" if you mock it in the test. Again, please provide a [mcve]. The issue you have is not in the code you show. – daniu Aug 23 '22 at 10:29
  • Sorry, it's not `Record` that's the issue, the problem is that `Result` gives a record even though it is empty. Looks to me like the loop behavior is wrong, but again, one can only guess. – daniu Aug 23 '22 at 10:42
  • @daniu Exactly, loop behaves like there is something in the list, but there's nothing. I'll update the description once I get deeper into the whole mocking logic. I was sure it's something with the loop, since that's the only change in the code, but seems like I have to dig more. – Nautilus Aug 23 '22 at 10:57
  • 1
    As others have requested, can you please show how you mock `Result`. – Lukas Eder Aug 23 '22 at 12:29
  • Added how the Result is mocked. It's a private method in the test class which is called with different parameters based on the test case. Not sure, but seems like that private method where this for loop is located in the method that's being tested is't processing the object correctly cuse it's not a real object. – Nautilus Aug 24 '22 at 05:21
  • 1
    You’re clearly mocking the Iterator on data which is used in the for loop, but not data.stream…so there is your problem. It would be easier if data was a Collection, then you would not have to mock that. – slindenau Aug 24 '22 at 05:29
  • @slindenau: `data` is a jOOQ `Result`, which extends `List`, but I think the problem is that mockito doesn't call actual `default` methods, by default? – Lukas Eder Aug 24 '22 at 09:20
  • @LukasEder i do remember having problems myself with `default` methods on an interface in combination with Mockito. So that might be worth looking into indeed. – slindenau Aug 24 '22 at 09:23

1 Answers1

19

Why it didn't work

You're mocking Result.iterator():

when(deviceDataResultMock.iterator()).thenReturn(resultIterator);

But you didn't mock Result.spliterator(), or at least I didn't see it, which is what's being called by Result.stream(), which is just Collection.stream():

default Stream<E> stream() {
    return StreamSupport.stream(spliterator(), false);
}

So, you'll have to mock the spliterator() method as well, and probably a few others, too! Alternatively, tell Mockito to call default methods:

Can you make mockito (1.10.17) work with default methods in interfaces?

A comment on mocking in general

I'm not convinced that mocking the jOOQ API is a very good idea. The jOOQ API is vast, and you'll likely forget to mock this or that method as this question here has aptly shown. With your current setup, you're planning on updating your mock every time you project a new column? E.g. you're doing this:

when(recordMock.get(DEVICEID.getName()).thenReturn(deviceId);

What if the column is renamed? Or another column is projected? You'll have to update this test. That feels like quite the chore, and it's very error prone.

While jOOQ itself has JDBC mocking capabilities, please consider the bold disclaimer on that manual page:

Disclaimer: The general idea of mocking a JDBC connection with this jOOQ API is to provide quick workarounds, injection points, etc. using a very simple JDBC abstraction. It is NOT RECOMMENDED to emulate an entire database (including complex state transitions, transactions, locking, etc.) using this mock API. Once you have this requirement, please consider using an actual database product instead for integration testing, rather than implementing your test database inside of a MockDataProvider

When working with databases, it's usually best to resort to running integration tests, see the following resources for some details:

Of course, you can write a few smoke tests to ensure jOOQ works correctly if you don't trust jOOQ (jOOQ being an external dependency). But the jOOQ unit and integration tests are vast, so in general, you should be able to trust core types like Result or Record to do the right thing for you. What you really want to test is your query correctness, and that, you can only integration test against an actual database instance.

Lukas Eder
  • 211,314
  • 129
  • 689
  • 1,509
  • Oh, wow. I didn't know any of these, thanks for eye-opening answer! One question, how can I keep the same behaviour by using Spliterator instead? This part: `Result deviceDataResultMock = Mockito.mock(Result.class);` `Iterator resultIterator = Mockito.mock(Iterator.class);` `when(resultIterator.hasNext()).thenReturn(true, false);` `Record recordMock = Mockito.mock(Record.class);` `when(resultIterator.next()).thenReturn(recordMock);` `when(deviceDataResultMock.iterator()).thenReturn(resultIterator);` – Nautilus Aug 24 '22 at 08:39
  • @Nautilus: I've researched this a bit more, the problem is probably related to the `spliterator()` method being a `default` method, which you have to instruct mockito to call explicitly (?). For the sake of helping others who will find this question in the future, if you find out how to correctly mock the `Result` type, you can provide an answer to your own question showing how. – Lukas Eder Aug 24 '22 at 09:19