5

list.stream().forEach(e -> { dbCall.delete(e.Id());});

Each item from the list is deleted from the database.

Assuming there were 3 items in the list, how to unit test:

  1. Delete was called 3 times.
  2. Delete was called 'in order/sequence', i.e. sequence of elements in the list?
Mikhail Kholodkov
  • 23,642
  • 17
  • 61
  • 78
Tanvi Jaywant
  • 375
  • 1
  • 6
  • 18
  • 1
    Since you are using `forEach`, which is explicitly unordered, there is no point in testing whether the calls were done in order. But anyway, what you are asking for, is not *unit testing*. You are trying to verify implementation details. If the statement does the right thing, and the right thing is to delete the specified three entries, *and that’s what you should verify*, who cares which method has been invoked in what order? The way to test whether the code deletes the three specified items, does not depend on whether you use a loop, `list.forEach`, or `list.stream().forEach(…)`. – Holger Jun 13 '18 at 16:15
  • @Holger `forEach` is not 'explicitly unordered' ; for sequential streams, it's guaranteed to be ordered, for parallel ones is implicitly unordered. Also, the order of deletions might well matter, for example if the list is ordered by a reference relation between the items that is enforced by the database. Far fetched, maybe, not the best design, probably, but possible. – daniu Jun 14 '18 at 19:11
  • @daniu how do you come to that conclusion? The [documentation of `forEach`](https://docs.oracle.com/javase/9/docs/api/java/util/stream/Stream.html#forEach-java.util.function.Consumer-) clearly says: “*The behavior of this operation is explicitly nondeterministic*”. [As said by one of the authors](https://stackoverflow.com/a/34253279/2711488) the statement about parallel stream execution does not allow to draw an opposite conclusion about sequential streams. But for a *unit test*, the order is still irrelevant. What matters, is, whether the three items are correctly deleted. – Holger Jun 15 '18 at 06:33
  • @Holger I come to that conclusion because in this case, the stream is coming from a `List`, and the behavior is the same as calling `List#forEach`, for which " actions are performed in the order of iteration (if an iteration order is specified)" (https://docs.oracle.com/javase/8/docs/api/java/lang/Iterable.html#forEach-java.util.function.Consumer-). Brian's statement is about the general case of `Stream#forEach` calls, eg also for streams that are created from an unordered `Collection`. You're right, "is guaranteed to" is not true - but the JDK has never ever changed existing behavior. – daniu Jun 15 '18 at 06:42
  • @Holger As for the order being irrelevant in a unit test, that's just not true. "The three items are correctly deleted" is an operational requirement, unit tests (in the strict sense) verify technical behavior. Also, in general, call order is well in the scope of a unit test: if these were three different service calls instead of db deletes, I'm sure you'd agree the order could be relevant and we wouldn't have this discussion. – daniu Jun 15 '18 at 06:46
  • @daniu well, it happens to do the same, but there is that semantic difference between `list.forEach(…)` and `list.stream.forEach(…)` and unit testing is about verifying your expectations. If you expect it to be ordered but use `stream.forEach(…)`, you get the worst possible outcome. The code is sematically wrong, using an operation that does not guaranty the order, but the unit test is not capable of detecting this, because the current implementation does not exhibit a wrong behavior. – Holger Jun 15 '18 at 06:49
  • @Holger Well yes the operation does not guarantee the order, but the unit test is well capable of detecting that, because it runs on the current implementation (if you use the same implementation in test and prod). – daniu Jun 15 '18 at 06:52
  • @daniu well, yes, if you are fine with the idea that your software could just happen to do the right thing by accident and some subtle change of the environment could break it, then, that’s sufficient. After all, *you* understand that stream().forEach(…)` is semantically wrong but is implemented in a way that happen to do the right thing here and *you* understand that it will continue to do so as long as you don’t change the JRE. But that’s not the answer of the unit test. A unit test does not tell you which circumstances (JRE version, JVM options, Random seed) were crucial to the success. – Holger Jun 15 '18 at 06:59
  • @Holger not sure what you mean. The unit test (if using `InOrder` as in my answer) tells you that if you execute that statement in the JRE in which it's run with the options it's run with, the call will behave the way I haphazardly assume. Of course the unit test doesn't tell you those, but if your build management can't tell you, you have a different problem. I will disregard the notion that the random seed has any influence on this, as there is such a thing as the real world. – daniu Jun 15 '18 at 07:06
  • @daniu yes there is such thing as the real world and in the real world, `Set.of(…)` produces sets with changing iteration order, to make it easier to spot errors in code expecting a particular order. But of course, a single run may happen to produce the order you’re relying on. The other conclusion of this feature is, that code relying on `HashSet`’s iteration order, which is not randomized, may run multiple times without spotting such an error. By the way, a lot of developers produce code supposed to run in more than one JRE and still use unit testing, but use it *the right way*. – Holger Jun 15 '18 at 07:14
  • @Holger why are you talking about `Set`? I specifically said I was talking about streams coming from a `List`. Yes, if they refactor the code into `list = Set.of(d1, d2, d3)`, they might run into the problem of the unit test not failing when it should. – daniu Jun 15 '18 at 07:43
  • @daniu I’m talking about *unit tests*. If a unit test succeeds, it doesn’t tell you whether it contained such dependencies or not. That’s the point. In this specific case, *you* are the one who knows that there’s no such dependency to a random seed (as no such `Set` is involved), but to the JRE version (as the code is relying on implementation details of `Stream.forEach`). The test only succeeds in the current environment (as always), but *you* are the one knowing that “environment” implies “JRE version”, but not “random seed” here. So the test produces less information than you already know. – Holger Jun 15 '18 at 07:57
  • @Holger A succeeded unit test never tells you anything anyway. But what this unit test _can_ do is fail when the stream source is changed, which will tell you the code behaves differently than before. That's the point of the unit test, and it's worthwhile for that reason (independently of JRE version and JVM options). – daniu Jun 15 '18 at 08:08
  • That is going in circles, so we should stop here. Normally, developers try to develop code that is not crucially dependent on the JRE version and all I said at the beginning, is, that this code *is* dependent on the JRE version and unit tests are not able to spot that problem. In the end, you agreed upon that, but dodged the problem by saying that this would be fine for code intended to run only on a particular JRE version. Unfortunately, there is no indicator that the OP wants exactly that nor that the OP knew about the JRE dependency (and given your first comment, you didn’t know either)… – Holger Jun 15 '18 at 08:18

2 Answers2

4

You can use JUnit's InOrder.

DbCall dbCall = mock(DbCall.class);
List<Element> list = Arrays.asList(newElement(1), newElement(2), newElement(3));

runDeleteMethod(list);

InOrder inorder = inOrder(dbCall);
inorder.verify(dbCall).delete(1);
inorder.verify(dbCall).delete(2);
inorder.verify(dbCall).delete(3);
daniu
  • 14,137
  • 4
  • 32
  • 53
1

Just verify the expected time dbCall.delete() should be called. This can look like this:

Mockito.verify(dbCall, times(3L)).delete(any(String.class));

Streams can work parallel and so you can´t verify the the sequence. You can do it with verify element on index but the sequence will be ignored. Mockito will just verify the call+value was used. That´s enought for a unit-test.

LenglBoy
  • 1,451
  • 1
  • 10
  • 24