ENT-933 Add spectator and participant profiles of rigorousMock (#3157)

build.gradle

@@ -52,7 +52,7 @@ buildscript {
     ext.typesafe_config_version = constants.getProperty("typesafeConfigVersion")
     ext.fileupload_version = '1.3.3'
     ext.junit_version = '4.12'
-    ext.mockito_version = '2.10.0'
+    ext.mockito_version = '2.18.3'
     ext.jopt_simple_version = '5.0.2'
     ext.jansi_version = '1.14'
     ext.hibernate_version = '5.2.6.Final'

finance/src/test/java/net/corda/finance/contracts/asset/CashTestsJava.java

@@ -16,7 +16,7 @@ import static java.util.Collections.emptyList;
 import static net.corda.finance.Currencies.DOLLARS;
 import static net.corda.finance.Currencies.issuedBy;
 import static net.corda.testing.node.NodeTestUtils.transaction;
-import static net.corda.testing.internal.InternalTestUtilsKt.rigorousMock;
+import static net.corda.testing.internal.RigorousMockKt.rigorousMock;
 import static net.corda.testing.core.TestConstants.DUMMY_NOTARY_NAME;
 import static org.mockito.Mockito.doReturn;
 

node/src/test/java/net/corda/node/services/vault/VaultQueryJavaTests.java

@@ -49,7 +49,7 @@ import static net.corda.core.node.services.vault.Builder.sum;
 import static net.corda.core.node.services.vault.QueryCriteriaUtils.*;
 import static net.corda.core.utilities.ByteArrays.toHexString;
 import static net.corda.testing.core.TestConstants.*;
-import static net.corda.testing.internal.InternalTestUtilsKt.rigorousMock;
+import static net.corda.testing.internal.RigorousMockKt.rigorousMock;
 import static net.corda.testing.node.MockServices.makeTestDatabaseAndMockServices;
 import static net.corda.testing.node.MockServicesKt.makeTestIdentityService;
 import static org.assertj.core.api.Assertions.assertThat;

node/src/test/kotlin/net/corda/node/services/events/NodeSchedulerServiceTest.kt

@@ -8,7 +8,6 @@ import net.corda.core.flows.FlowLogicRef
 import net.corda.core.flows.FlowLogicRefFactory
 import net.corda.core.internal.FlowStateMachine
 import net.corda.core.internal.concurrent.openFuture
-import net.corda.core.internal.uncheckedCast
 import net.corda.core.node.ServicesForResolution
 import net.corda.core.utilities.days
 import net.corda.node.internal.configureDatabase
@@ -20,6 +19,7 @@ import net.corda.nodeapi.internal.persistence.DatabaseConfig
 import net.corda.nodeapi.internal.persistence.DatabaseTransaction
 import net.corda.testing.internal.doLookup
 import net.corda.testing.internal.rigorousMock
+import net.corda.testing.internal.spectator
 import net.corda.testing.node.MockServices
 import net.corda.testing.node.TestClock
 import org.junit.Ignore
@@ -44,16 +44,9 @@ open class NodeSchedulerServiceTestBase {
     protected val testClock = TestClock(rigorousMock<Clock>().also {
         doReturn(mark).whenever(it).instant()
     })
-    private val database = rigorousMock<CordaPersistence>().also {
-        doAnswer {
-            val block: DatabaseTransaction.() -> Any? = uncheckedCast(it.arguments[0])
-            rigorousMock<DatabaseTransaction>().block()
-        }.whenever(it).transaction(any())
-    }
-
     protected val flowStarter = rigorousMock<FlowStarter>().also {
         doAnswer {
-            val dedupe = it.arguments[2] as DeduplicationHandler
+            val dedupe: DeduplicationHandler = it.getArgument(2)
             dedupe.insideDatabaseTransaction()
             dedupe.afterDatabaseTransaction()
             openFuture<FlowStateMachine<*>>()
@@ -74,11 +67,8 @@ open class NodeSchedulerServiceTestBase {
     protected val servicesForResolution = rigorousMock<ServicesForResolution>().also {
         doLookup(transactionStates).whenever(it).loadState(any())
     }
-    protected val log = rigorousMock<Logger>().also {
+    protected val log = spectator<Logger>().also {
         doReturn(false).whenever(it).isTraceEnabled
-        doNothing().whenever(it).trace(any(), any<Any>())
-        doNothing().whenever(it).info(any())
-        doNothing().whenever(it).error(any(), any<Throwable>())
     }
 
     protected fun assertWaitingFor(ssr: ScheduledStateRef, total: Int = 1) {
@@ -90,7 +80,7 @@ open class NodeSchedulerServiceTestBase {
 
     protected fun assertStarted(flowLogic: FlowLogic<*>) {
         // Like in assertWaitingFor, use timeout to make verify wait as we often race the call to startFlow:
-        verify(flowStarter, timeout(5000)).startFlow(same(flowLogic)!!, any(), any())
+        verify(flowStarter, timeout(5000)).startFlow(same(flowLogic), any(), any())
     }
 
     protected fun assertStarted(event: Event) = assertStarted(event.flowLogic)
@@ -124,7 +114,7 @@ class MockScheduledFlowRepository : ScheduledFlowRepository {
 class NodeSchedulerServiceTest : NodeSchedulerServiceTestBase() {
     private val database = rigorousMock<CordaPersistence>().also {
         doAnswer {
-            val block: DatabaseTransaction.() -> Any? = uncheckedCast(it.arguments[0])
+            val block: DatabaseTransaction.() -> Any? = it.getArgument(0)
             rigorousMock<DatabaseTransaction>().block()
         }.whenever(it).transaction(any())
     }
@@ -154,7 +144,7 @@ class NodeSchedulerServiceTest : NodeSchedulerServiceTestBase() {
         val logicRef = rigorousMock<FlowLogicRef>()
         transactionStates[stateRef] = rigorousMock<TransactionState<SchedulableState>>().also {
             doReturn(rigorousMock<SchedulableState>().also {
-                doReturn(ScheduledActivity(logicRef, time)).whenever(it).nextScheduledActivity(same(stateRef)!!, any())
+                doReturn(ScheduledActivity(logicRef, time)).whenever(it).nextScheduledActivity(same(stateRef), any())
             }).whenever(it).data
         }
         flows[logicRef] = flowLogic

samples/irs-demo/build.gradle

@@ -19,6 +19,7 @@ ext['hibernate.version'] = "$hibernate_version"
 ext['selenium.version'] = "$selenium_version"
 ext['jackson.version'] = "$jackson_version"
 ext['dropwizard-metrics.version'] = "$metrics_version"
+ext['mockito.version'] = "$mockito_version"
 
 apply plugin: 'java'
 apply plugin: 'kotlin'
@@ -111,4 +112,4 @@ idea {
         downloadJavadoc = true // defaults to false
         downloadSources = true
     }
-}
+}

samples/network-visualiser/build.gradle

@@ -19,7 +19,7 @@ ext['artemis.version'] = "$artemis_version"
 ext['hibernate.version'] = "$hibernate_version"
 ext['jackson.version'] = "$jackson_version"
 ext['dropwizard-metrics.version'] = "$metrics_version"
-
+ext['mockito.version'] = "$mockito_version"
 
 apply plugin: 'java'
 apply plugin: 'kotlin'

testing/test-utils/build.gradle

@@ -21,7 +21,8 @@ dependencies {
     // Unit testing helpers.
     compile "junit:junit:$junit_version"
     compile 'org.hamcrest:hamcrest-library:1.3'
-    compile "com.nhaarman:mockito-kotlin:1.1.0"
+    compile 'com.nhaarman:mockito-kotlin:1.5.0'
+    compile "org.mockito:mockito-core:$mockito_version"
     compile "org.assertj:assertj-core:$assertj_version"
 
     // Guava: Google test library (collections test suite)

testing/test-utils/src/main/kotlin/net/corda/testing/internal/InternalTestUtils.kt

@@ -16,12 +16,8 @@ import net.corda.nodeapi.internal.crypto.CertificateAndKeyPair
 import net.corda.nodeapi.internal.crypto.CertificateType
 import net.corda.nodeapi.internal.crypto.X509Utilities
 import net.corda.nodeapi.internal.serialization.amqp.AMQP_ENABLED
-import org.mockito.Mockito
-import org.mockito.internal.stubbing.answers.ThrowsException
-import java.lang.reflect.Modifier
 import java.nio.file.Files
 import java.security.KeyPair
-import java.util.*
 import javax.security.auth.x500.X500Principal
 
 @Suppress("unused")
@@ -38,28 +34,6 @@ inline fun <reified T : Any> T.amqpSpecific(reason: String, function: () -> Unit
     loggerFor<T>().info("Ignoring AMQP specific test, reason: $reason")
 }
 
-/**
- * A method on a mock was called, but no behaviour was previously specified for that method.
- * You can use [com.nhaarman.mockito_kotlin.doReturn] or similar to specify behaviour, see Mockito documentation for details.
- */
-class UndefinedMockBehaviorException(message: String) : RuntimeException(message)
-
-inline fun <reified T : Any> rigorousMock() = rigorousMock(T::class.java)
-/**
- * Create a Mockito mock that has [UndefinedMockBehaviorException] as the default behaviour of all abstract methods,
- * and [org.mockito.invocation.InvocationOnMock.callRealMethod] as the default for all concrete methods.
- * @param T the type to mock. Note if you want concrete methods of a Kotlin interface to be invoked,
- * it won't work unless you mock a (trivial) abstract implementation of that interface instead.
- */
-fun <T> rigorousMock(clazz: Class<T>): T = Mockito.mock(clazz) {
-    if (Modifier.isAbstract(it.method.modifiers)) {
-        // Use ThrowsException to hack the stack trace, and lazily so we can customise the message:
-        ThrowsException(UndefinedMockBehaviorException("Please specify what should happen when '${it.method}' is called, or don't call it. Args: ${Arrays.toString(it.arguments)}")).answer(it)
-    } else {
-        it.callRealMethod()
-    }
-}
-
 fun configureTestSSL(legalName: CordaX500Name): SSLConfiguration {
     return object : SSLConfiguration {
         override val certificatesDirectory = Files.createTempDirectory("certs")
@@ -118,9 +92,6 @@ fun createDevNodeCaCertPath(
     return Triple(rootCa, intermediateCa, nodeCa)
 }
 
-/** Application of [doAnswer] that gets a value from the given [map] using the arg at [argIndex] as key. */
-fun doLookup(map: Map<*, *>, argIndex: Int = 0) = doAnswer { map[it.arguments[argIndex]] }
-
 fun SSLConfiguration.useSslRpcOverrides(): Map<String, Any> {
     return mapOf(
             "rpcSettings.useSsl" to "true",

testing/test-utils/src/main/kotlin/net/corda/testing/internal/RigorousMock.kt

@@ -0,0 +1,119 @@
+package net.corda.testing.internal
+
+import com.nhaarman.mockito_kotlin.doAnswer
+import net.corda.core.utilities.contextLogger
+import org.mockito.Mockito
+import org.mockito.exceptions.base.MockitoException
+import org.mockito.internal.stubbing.answers.ThrowsException
+import org.mockito.invocation.InvocationOnMock
+import org.mockito.stubbing.Answer
+import java.lang.reflect.Method
+import java.lang.reflect.Modifier
+import java.lang.reflect.ParameterizedType
+import java.lang.reflect.Type
+import java.util.*
+import java.util.concurrent.ConcurrentHashMap
+
+/**
+ * A method on a mock was called, but no behaviour was previously specified for that method.
+ * You can use [com.nhaarman.mockito_kotlin.doReturn] or similar to specify behaviour, see Mockito documentation for details.
+ */
+class UndefinedMockBehaviorException(message: String) : RuntimeException(message)
+
+inline fun <reified T : Any> spectator() = spectator(T::class.java)
+inline fun <reified T : Any> rigorousMock() = rigorousMock(T::class.java)
+inline fun <reified T : Any> participant() = participant(T::class.java)
+/**
+ * Create a Mockito mock where void methods do nothing, and any method of mockable return type will return another spectator,
+ * and multiple calls to such a method with equal args will return the same spectator.
+ * This is useful for an inconsequential service such as metrics or logging.
+ * Unlike plain old Mockito, methods that return primitives and unmockable types such as [String] remain unimplemented.
+ * Use sparingly, as any invalid behaviour caused by the implicitly-created spectators is likely to be difficult to diagnose.
+ * As in the other profiles, the exception is [toString] which has a simple reliable implementation for ease of debugging.
+ */
+fun <T> spectator(clazz: Class<out T>) = Mockito.mock(clazz, SpectatorDefaultAnswer())!!
+
+/**
+ * Create a Mockito mock that inherits the implementations of all concrete methods from the given type.
+ * In particular this is convenient for mocking a Kotlin interface via a trivial abstract class.
+ * As in the other profiles, the exception is [toString] which has a simple reliable implementation for ease of debugging.
+ */
+fun <T> rigorousMock(clazz: Class<out T>) = Mockito.mock(clazz, RigorousMockDefaultAnswer)!!
+
+/**
+ * Create a Mockito mock where all methods throw [UndefinedMockBehaviorException].
+ * Such mocks are useful when testing a grey box, for complete visibility and control over what methods it calls.
+ * As in the other profiles, the exception is [toString] which has a simple reliable implementation for ease of debugging.
+ */
+fun <T> participant(clazz: Class<out T>) = Mockito.mock(clazz, ParticipantDefaultAnswer)!!
+
+private abstract class DefaultAnswer : Answer<Any?> {
+    internal abstract fun answerImpl(invocation: InvocationOnMock): Any?
+    override fun answer(invocation: InvocationOnMock): Any? {
+        val method = invocation.method
+        if (method.name == "toString" && method.parameterCount == 0) {
+            // Regular toString doesn't cache so neither do we:
+            val mock = invocation.mock
+            return "${mock.javaClass.simpleName}@${Integer.toHexString(mock.hashCode())}"
+        }
+        return answerImpl(invocation)
+    }
+}
+
+private class SpectatorDefaultAnswer : DefaultAnswer() {
+    private companion object {
+        private val log = contextLogger()
+    }
+
+    private class MethodInfo(invocation: InvocationOnMock) {
+        // FIXME LATER: The type resolution code probably doesn't cover all cases.
+        private val type = run {
+            val method = invocation.method
+            fun resolveType(context: Type, type: Type): Type {
+                context as? ParameterizedType ?: return type
+                val clazz = context.rawType as Class<*>
+                return context.actualTypeArguments[clazz.typeParameters.indexOf(resolveType(clazz.genericSuperclass, type))]
+            }
+            resolveType(invocation.mock.javaClass.genericSuperclass, method.genericReturnType) as? Class<*>
+                    ?: method.returnType!!
+        }
+
+        private fun newSpectator(invocation: InvocationOnMock) = spectator(type)!!.also { log.info("New spectator {} for: {}", it, invocation.arguments) }
+        private val spectators = try {
+            val first = newSpectator(invocation)
+            ConcurrentHashMap<InvocationOnMock, Any>().apply { put(invocation, first) }
+        } catch (e: MockitoException) {
+            null // A few types can't be mocked e.g. String.
+        }
+
+        internal fun spectator(invocation: InvocationOnMock) = spectators?.computeIfAbsent(invocation, ::newSpectator)
+    }
+
+    private val methods by lazy { ConcurrentHashMap<Method, MethodInfo>() }
+    override fun answerImpl(invocation: InvocationOnMock): Any? {
+        invocation.method.returnType.let {
+            it == Void.TYPE && return null
+            it.isPrimitive && return ParticipantDefaultAnswer.answerImpl(invocation)
+        }
+        return methods.computeIfAbsent(invocation.method) { MethodInfo(invocation) }.spectator(invocation)
+                ?: ParticipantDefaultAnswer.answerImpl(invocation)
+    }
+}
+
+private object RigorousMockDefaultAnswer : DefaultAnswer() {
+    override fun answerImpl(invocation: InvocationOnMock): Any? {
+        return if (Modifier.isAbstract(invocation.method.modifiers)) ParticipantDefaultAnswer.answerImpl(invocation) else invocation.callRealMethod()
+    }
+}
+
+private object ParticipantDefaultAnswer : DefaultAnswer() {
+    override fun answerImpl(invocation: InvocationOnMock): Any? {
+        // Use ThrowsException to hack the stack trace, and lazily so we can customise the message:
+        return ThrowsException(UndefinedMockBehaviorException(
+                "Please specify what should happen when '${invocation.method}' is called, or don't call it. Args: ${Arrays.toString(invocation.arguments)}"))
+                .answer(invocation)
+    }
+}
+
+/** Application of [doAnswer] that gets a value from the given [map] using the arg at [argIndex] as key. */
+fun doLookup(map: Map<*, *>, argIndex: Int = 0) = doAnswer { map[it.getArgument<Any?>(argIndex)] }

testing/test-utils/src/main/resources/mockito-extensions/org.mockito.plugins.MockMaker

@@ -0,0 +1 @@
+mock-maker-inline

testing/test-utils/src/test/kotlin/net/corda/testing/internal/RigorousMockTest.kt

@@ -0,0 +1,126 @@
+package net.corda.testing.internal
+
+import org.assertj.core.api.Assertions.catchThrowable
+import org.hamcrest.Matchers.isA
+import org.junit.Assert.assertThat
+import org.junit.Test
+import java.io.Closeable
+import java.io.InputStream
+import java.io.Serializable
+import java.util.stream.Stream
+import kotlin.test.*
+
+private interface MyInterface {
+    fun abstractFun(): Int
+    fun kotlinDefaultFun() = 5
+}
+
+private abstract class MyAbstract : MyInterface
+private open class MyImpl : MyInterface {
+    override fun abstractFun() = 4
+    open fun openFun() = 6
+    fun finalFun() = 7
+    override fun toString() = "8"
+}
+
+private interface MySpectator {
+    fun sideEffect()
+    fun noClearDefault(): Int
+    fun collaborator(arg: Int): MySpectator
+}
+
+class RigorousMockTest {
+    @Test
+    fun `toString has a reliable default answer in all cases`() {
+        Stream.of<(Class<out Any>) -> Any>(::spectator, ::rigorousMock, ::participant).forEach { profile ->
+            Stream.of(MyInterface::class, MyAbstract::class, MyImpl::class).forEach { type ->
+                val mock = profile(type.java)
+                assertEquals("${mock.javaClass.simpleName}@${Integer.toHexString(mock.hashCode())}", mock.toString())
+            }
+        }
+    }
+
+    @Test
+    fun `callRealMethod is preferred by rigorousMock`() {
+        rigorousMock<MyInterface>().let { m ->
+            assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.abstractFun() }.javaClass)
+            assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.kotlinDefaultFun() }.javaClass)
+        }
+        rigorousMock<MyAbstract>().let { m ->
+            assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.abstractFun() }.javaClass)
+            assertEquals(5, m.kotlinDefaultFun())
+        }
+        rigorousMock<MyImpl>().let { m ->
+            assertEquals(4, m.abstractFun())
+            assertEquals(5, m.kotlinDefaultFun())
+            assertEquals(6, m.openFun())
+            assertEquals(7, m.finalFun())
+        }
+    }
+
+    @Test
+    fun `throw exception is preferred by participant`() {
+        participant<MyInterface>().let { m ->
+            assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.abstractFun() }.javaClass)
+            assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.kotlinDefaultFun() }.javaClass)
+        }
+        participant<MyAbstract>().let { m ->
+            assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.abstractFun() }.javaClass)
+            assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.kotlinDefaultFun() }.javaClass) // Broken in older Mockito.
+        }
+        participant<MyImpl>().let { m ->
+            assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.abstractFun() }.javaClass)
+            assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.kotlinDefaultFun() }.javaClass)
+            assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.openFun() }.javaClass)
+            assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.finalFun() }.javaClass)
+        }
+    }
+
+    @Test
+    fun `doing nothing is preferred by spectator`() {
+        val mock: MySpectator = spectator()
+        mock.sideEffect()
+        assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { mock.noClearDefault() }.javaClass)
+        val collaborator = mock.collaborator(1)
+        assertNotSame(mock, collaborator)
+        assertSame(collaborator, mock.collaborator(1))
+        assertNotSame(collaborator, mock.collaborator(2))
+        collaborator.sideEffect()
+        assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { collaborator.noClearDefault() }.javaClass)
+    }
+
+    private open class AB<out A, out B> {
+        val a: A get() = throw UnsupportedOperationException()
+        val b: B get() = throw UnsupportedOperationException()
+    }
+
+    private open class CD<out C, out D> : AB<D, C>()
+    private class CDImpl : CD<Runnable, String>()
+
+    @Test
+    fun `method return type resolution works`() {
+        val m = spectator<CDImpl>()
+        assertThat(m.b, isA(Runnable::class.java))
+        assertSame<Any>(UndefinedMockBehaviorException::class.java, catchThrowable { m.a }.javaClass) // Can't mock String.
+    }
+
+    private interface RS : Runnable, Serializable
+    private class TU<out T> where T : Runnable, T : Serializable {
+        fun t(): T = throw UnsupportedOperationException()
+        fun <U : Closeable> u(): U = throw UnsupportedOperationException()
+    }
+
+    @Test
+    fun `method return type erasure cases`() {
+        val m = spectator<TU<RS>>()
+        m.t().let { t: Any ->
+            assertFalse(t is RS)
+            assertTrue(t is Runnable)
+            assertFalse(t is Serializable) // Erasure picks the first bound.
+        }
+        m.u<InputStream>().let { u: Any ->
+            assertFalse(u is InputStream)
+            assertTrue(u is Closeable)
+        }
+    }
+}

tools/demobench/build.gradle

@@ -74,7 +74,6 @@ dependencies {
     testCompile "junit:junit:$junit_version"
     testCompile "org.jetbrains.kotlin:kotlin-test:$kotlin_version"
     testCompile "org.assertj:assertj-core:${assertj_version}"
-    testCompile "org.mockito:mockito-core:$mockito_version"
 }
 
 jar {