[ovirt-devel] Line of death for tests

[Allon Mureinik]

The root cause seems to be MigrateVmCommandTest and its relation to
MigrateVmCommand.

A common misconception is that if an object is annotated with @InjectMocks,
Mockito will inject its @Mocks and @Spys to fields annotated with @Inject.
This is, as noted, not correct - Mockito will attempt to injcet its mocks
to *ALL* the fields, regardless of annotations.

And here's where things start getting funky.
Since the new field you introduced is an Object, any mock would be a
possible match for it. If more than one mock fits the field (as you have in
the test - dbFacade and vmValidator), Mockito silently skips injecting it,
and leaves it null (not sure if this is a bug or an intentional ugly
behavior) - need to look into this further.
Even worse, this failure breaks the mocking sequence, so the mocked object
isn't spied. When you attempt to stub it's behavior (e.g., with doReturn,
as the first line
of testValidationFailsWhenVmHasDisksPluggedWithScsiReservation does), it
will fail with a NotAMockException). From there on, it's all down hill.

The good news is that this test doesn't really need to inject mocks, so
just removing that annotation solves the issue. I've posted a patch [1],
please review it.

I'll also organize an MCVE reproducer and report a bug against mockito, and
we'll see if there's any chance to get it solved any time soon.

-Allon

[1] https://gerrit.ovirt.org/#/q/topic:MigrateVmCommandTest

On Thu, Mar 16, 2017 at 11:55 AM, Shmuel Melamud <smelamud at redhat.com>
wrote:

> Hi!
>
> I've found yesterday a very strange bug, most likely in Mockito. Hard
> to believe it really happens, but I've reproduced it in different
> environmentы and several revisions of oVirt master.
>
> I've added the following line into MigrateVmCommand:
>
>      private Integer actualDowntime;
> +    private Object actualDowntimeLock;
>
> This line causes ALL tests to fail with NullPointerException. Some
> tests print such a stack trace:
>
> java.lang.NullPointerException
>     at org.mockito.internal.junit.util.TestName.getTestName(
> TestName.java:11)
>     at org.mockito.internal.junit.MismatchReportingTestListener.
> testFinished(MismatchReportingTestListener.java:29)
>     at org.mockito.internal.runners.DefaultInternalRunner$1$1.
> testFinished(DefaultInternalRunner.java:56)
>     at org.junit.runner.notification.SynchronizedRunListener.testFinished(
> SynchronizedRunListener.java:56)
>     at org.junit.runner.notification.RunNotifier$7.notifyListener(
> RunNotifier.java:190)
> ...
>
> Name of the field doesn't matter, but type matters - changing the type
> to Integer solved the problem.
>
> Does anybody has an idea why this happens and how to fix it?
>
> Shmuel
> _______________________________________________
> Devel mailing list
> Devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/devel/attachments/20170316/6e5310c7/attachment.html>