diff --git a/build.gradle b/build.gradle index d823ed5246..b2136ccf87 100644 --- a/build.gradle +++ b/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' diff --git a/finance/src/test/java/net/corda/finance/contracts/asset/CashTestsJava.java b/finance/src/test/java/net/corda/finance/contracts/asset/CashTestsJava.java index 3e28b1aee1..f7f29b445d 100644 --- a/finance/src/test/java/net/corda/finance/contracts/asset/CashTestsJava.java +++ b/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; diff --git a/node/src/main/kotlin/net/corda/node/services/messaging/P2PMessagingClient.kt b/node/src/main/kotlin/net/corda/node/services/messaging/P2PMessagingClient.kt index 3acae7898b..190cefbe60 100644 --- a/node/src/main/kotlin/net/corda/node/services/messaging/P2PMessagingClient.kt +++ b/node/src/main/kotlin/net/corda/node/services/messaging/P2PMessagingClient.kt @@ -420,7 +420,7 @@ class P2PMessagingClient(val config: NodeConfiguration, deliver(cordaMessage, artemisMessage) } else { log.trace { "Discard duplicate message ${cordaMessage.uniqueMessageId} for ${cordaMessage.topic}" } - artemisMessage.individualAcknowledge() + messagingExecutor!!.acknowledge(artemisMessage) } } } diff --git a/node/src/test/java/net/corda/node/services/vault/VaultQueryJavaTests.java b/node/src/test/java/net/corda/node/services/vault/VaultQueryJavaTests.java index 7500afbddb..3c9b727313 100644 --- a/node/src/test/java/net/corda/node/services/vault/VaultQueryJavaTests.java +++ b/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; diff --git a/node/src/test/kotlin/net/corda/node/services/events/NodeSchedulerServiceTest.kt b/node/src/test/kotlin/net/corda/node/services/events/NodeSchedulerServiceTest.kt index d5787acec9..ec6eb06604 100644 --- a/node/src/test/kotlin/net/corda/node/services/events/NodeSchedulerServiceTest.kt +++ b/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().also { doReturn(mark).whenever(it).instant() }) - private val database = rigorousMock().also { - doAnswer { - val block: DatabaseTransaction.() -> Any? = uncheckedCast(it.arguments[0]) - rigorousMock().block() - }.whenever(it).transaction(any()) - } - protected val flowStarter = rigorousMock().also { doAnswer { - val dedupe = it.arguments[2] as DeduplicationHandler + val dedupe: DeduplicationHandler = it.getArgument(2) dedupe.insideDatabaseTransaction() dedupe.afterDatabaseTransaction() openFuture>() @@ -74,11 +67,8 @@ open class NodeSchedulerServiceTestBase { protected val servicesForResolution = rigorousMock().also { doLookup(transactionStates).whenever(it).loadState(any()) } - protected val log = rigorousMock().also { + protected val log = spectator().also { doReturn(false).whenever(it).isTraceEnabled - doNothing().whenever(it).trace(any(), any()) - doNothing().whenever(it).info(any()) - doNothing().whenever(it).error(any(), any()) } 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().also { doAnswer { - val block: DatabaseTransaction.() -> Any? = uncheckedCast(it.arguments[0]) + val block: DatabaseTransaction.() -> Any? = it.getArgument(0) rigorousMock().block() }.whenever(it).transaction(any()) } @@ -154,7 +144,7 @@ class NodeSchedulerServiceTest : NodeSchedulerServiceTestBase() { val logicRef = rigorousMock() transactionStates[stateRef] = rigorousMock>().also { doReturn(rigorousMock().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 diff --git a/samples/irs-demo/build.gradle b/samples/irs-demo/build.gradle index 938dd005af..b116b7d8e9 100644 --- a/samples/irs-demo/build.gradle +++ b/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 } -} \ No newline at end of file +} diff --git a/samples/network-visualiser/build.gradle b/samples/network-visualiser/build.gradle index 25b8c89ae7..b45f06246f 100644 --- a/samples/network-visualiser/build.gradle +++ b/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' diff --git a/testing/test-utils/build.gradle b/testing/test-utils/build.gradle index e11cea5ebc..98beb3372a 100644 --- a/testing/test-utils/build.gradle +++ b/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) diff --git a/testing/test-utils/src/main/kotlin/net/corda/testing/internal/InternalTestUtils.kt b/testing/test-utils/src/main/kotlin/net/corda/testing/internal/InternalTestUtils.kt index c620ee6731..4f80f6992d 100644 --- a/testing/test-utils/src/main/kotlin/net/corda/testing/internal/InternalTestUtils.kt +++ b/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 T.amqpSpecific(reason: String, function: () -> Unit loggerFor().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 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 rigorousMock(clazz: Class): 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 { return mapOf( "rpcSettings.useSsl" to "true", diff --git a/testing/test-utils/src/main/kotlin/net/corda/testing/internal/RigorousMock.kt b/testing/test-utils/src/main/kotlin/net/corda/testing/internal/RigorousMock.kt new file mode 100644 index 0000000000..72f4942635 --- /dev/null +++ b/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 spectator() = spectator(T::class.java) +inline fun rigorousMock() = rigorousMock(T::class.java) +inline fun 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 spectator(clazz: Class) = 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 rigorousMock(clazz: Class) = 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 participant(clazz: Class) = Mockito.mock(clazz, ParticipantDefaultAnswer)!! + +private abstract class DefaultAnswer : Answer { + 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().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() } + 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(argIndex)] } diff --git a/testing/test-utils/src/main/resources/mockito-extensions/org.mockito.plugins.MockMaker b/testing/test-utils/src/main/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 0000000000..1f0955d450 --- /dev/null +++ b/testing/test-utils/src/main/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1 @@ +mock-maker-inline diff --git a/testing/test-utils/src/test/kotlin/net/corda/testing/internal/RigorousMockTest.kt b/testing/test-utils/src/test/kotlin/net/corda/testing/internal/RigorousMockTest.kt new file mode 100644 index 0000000000..7a03b5b359 --- /dev/null +++ b/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) -> 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().let { m -> + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.abstractFun() }.javaClass) + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.kotlinDefaultFun() }.javaClass) + } + rigorousMock().let { m -> + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.abstractFun() }.javaClass) + assertEquals(5, m.kotlinDefaultFun()) + } + rigorousMock().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().let { m -> + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.abstractFun() }.javaClass) + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.kotlinDefaultFun() }.javaClass) + } + participant().let { m -> + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.abstractFun() }.javaClass) + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.kotlinDefaultFun() }.javaClass) // Broken in older Mockito. + } + participant().let { m -> + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.abstractFun() }.javaClass) + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.kotlinDefaultFun() }.javaClass) + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.openFun() }.javaClass) + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.finalFun() }.javaClass) + } + } + + @Test + fun `doing nothing is preferred by spectator`() { + val mock: MySpectator = spectator() + mock.sideEffect() + assertSame(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(UndefinedMockBehaviorException::class.java, catchThrowable { collaborator.noClearDefault() }.javaClass) + } + + private open class AB { + val a: A get() = throw UnsupportedOperationException() + val b: B get() = throw UnsupportedOperationException() + } + + private open class CD : AB() + private class CDImpl : CD() + + @Test + fun `method return type resolution works`() { + val m = spectator() + assertThat(m.b, isA(Runnable::class.java)) + assertSame(UndefinedMockBehaviorException::class.java, catchThrowable { m.a }.javaClass) // Can't mock String. + } + + private interface RS : Runnable, Serializable + private class TU where T : Runnable, T : Serializable { + fun t(): T = throw UnsupportedOperationException() + fun u(): U = throw UnsupportedOperationException() + } + + @Test + fun `method return type erasure cases`() { + val m = spectator>() + m.t().let { t: Any -> + assertFalse(t is RS) + assertTrue(t is Runnable) + assertFalse(t is Serializable) // Erasure picks the first bound. + } + m.u().let { u: Any -> + assertFalse(u is InputStream) + assertTrue(u is Closeable) + } + } +} diff --git a/tools/demobench/build.gradle b/tools/demobench/build.gradle index 35d5c82e2b..a064dcef6e 100644 --- a/tools/demobench/build.gradle +++ b/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 {