Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix concurrent invocations of InvocationRecorder #144

Merged
merged 2 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .idea/codeStyles/Project.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ nexusPublishing {
sonatype {
nexusUrl.set(uri("https://s01.oss.sonatype.org/service/local/"))
snapshotRepositoryUrl.set(uri("https://s01.oss.sonatype.org/content/repositories/snapshots/"))
username.set((prop["ossrhUsername"] ?: System.getenv("OSSRH_USERNAME")) as String)
password.set((prop["ossrhPassword"] ?: System.getenv("OSSRH_PASSWORD")) as String)
username.set((prop["ossrhUsername"] ?: System.getenv("OSSRH_USERNAME") ?: "not-set") as String)
password.set((prop["ossrhPassword"] ?: System.getenv("OSSRH_PASSWORD") ?: "not-set") as String)
stagingProfileId.set((prop["sonatypeStagingProfileId"] ?: System.getenv("SONATYPE_STAGING_PROFILE_ID")) as String?)
}
}
Expand Down Expand Up @@ -122,9 +122,9 @@ subprojects {
pluginManager.withPlugin("signing") {
extensions.configure<SigningExtension> {
useInMemoryPgpKeys(
(prop["signing.keyId"] ?: System.getenv("SIGNING_KEY_ID")).toString(),
(prop["signing.key"] ?: System.getenv("SIGNING_KEY")).toString(),
(prop["signing.password"] ?: System.getenv("SIGNING_PASSWORD")).toString()
(prop["signing.keyId"] ?: System.getenv("SIGNING_KEY_ID") ?: "not-set").toString(),
(prop["signing.key"] ?: System.getenv("SIGNING_KEY") ?: "not-set").toString(),
(prop["signing.password"] ?: System.getenv("SIGNING_PASSWORD") ?: "not-set").toString()
)
sign(publishing.publications)
}
Expand Down
7 changes: 7 additions & 0 deletions mockingbird/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,16 @@ kotlin {
implementation(libs.touchlab.stately.concurrency)
implementation(libs.touchlab.stately.concurrent.collections)
implementation(libs.kotlinx.coroutines)
}
}

val commonTest by getting {
dependencies {
implementation(libs.kotlin.test)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kotlin test was a production library :(

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep also in mind that mockingbird is a testing library as well, clients should use mockingbird as a test library

implementation(libs.kotlinx.coroutines.test)
}
}

val iosMain by getting
val iosSimulatorArm64Main by getting {
dependsOn(iosMain)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,10 @@ package com.careem.mockingbird.test

import kotlinx.atomicfu.AtomicInt
import kotlinx.atomicfu.atomic
import kotlin.native.concurrent.SharedImmutable
import kotlin.test.assertEquals
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:(



@SharedImmutable
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SharedImmutable is ignored with the new memory model

internal const val AWAIT_POOLING_TIME = 10L

@SharedImmutable
private val uuidGenerator: AtomicInt = atomic(0)

public interface Mock {
Expand Down Expand Up @@ -151,6 +147,16 @@ internal fun <T : Mock> T.rawVerify(
}
}

private fun assertEquals(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find usage of this function. Where can it be used if it is private?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 138. was using the function from kotlin.test

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I would change this, why not keeping the kotlin test? Mockingbird is a test library it shouldn't be used in production code anyway

expected: Int,
actual: Int,
message: String
) {
if (expected != actual) {
throw AssertionError(message)
}
}

/**
* Function to mock a function
* @param methodName name of the method that you want to mock
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,23 @@
*/
package com.careem.mockingbird.test

import co.touchlab.stately.collections.ConcurrentMutableMap

internal class InvocationRecorder {

private val recorder = mutableMapOf<String, MutableList<Invocation>>()
private val responses = mutableMapOf<String, LinkedHashMap<Invocation, (Invocation) -> Any?>>()
private val recorder = ConcurrentMutableMap<String, MutableList<Invocation>>()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

private val responses = ConcurrentMutableMap<String, LinkedHashMap<Invocation, (Invocation) -> Any?>>()

/**
* This function must be called by the mock when a function call is exceuted on it
* This function must be called by the mock when a function call is executed on it
* @param uuid the uuid of the mock
* @param invocation the Invocation object @see [Invocation]
*/
fun storeInvocation(uuid: String, invocation: Invocation) {
if (!recorder.containsKey(uuid)) {
recorder[uuid] = mutableListOf()
recorder.block { map ->
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block will sync the call to the map

val list = map.getOrPut(uuid) { mutableListOf() }
list.add(invocation)
}
recorder[uuid]!!.add(invocation)
}

/**
Expand Down Expand Up @@ -60,10 +62,10 @@ internal class InvocationRecorder {
* @param answer the lambda that must be invoked when the invocation happen
*/
fun <T> storeAnswer(uuid: String, invocation: Invocation, answer: (Invocation) -> T) {
if (!responses.containsKey(uuid)) {
responses[uuid] = LinkedHashMap()
responses.block { map ->
val invocationsMap = map.getOrPut(uuid) { LinkedHashMap() }
invocationsMap[invocation] = answer as (Invocation) -> Any?
}
responses[uuid]!![invocation] = answer as (Invocation) -> Any?
}

/**
Expand All @@ -75,8 +77,8 @@ internal class InvocationRecorder {
* @return the mocked response, or null if relaxed (throws if not relaxed)
*/
fun getResponse(uuid: String, invocation: Invocation, relaxed: Boolean = false): Any? {
return if (uuid in responses.keys) {
responses[uuid]!!.let {
return if (uuid in responses) {
responses.getValue(uuid).let {
val lambda = findResponseByInvocation(it, invocation, relaxed)
return@let lambda(invocation)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,24 @@
*/
package com.careem.mockingbird.test

import kotlin.test.*
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.joinAll
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.runTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue

class InvocationRecorderTest {

private val invocationRecorder = InvocationRecorder()

@Test
fun testEmptyListWhenNoInvocationsRegisteredForAnInstance(){
fun testEmptyListWhenNoInvocationsRegisteredForAnInstance() {
val mock = newMock()

val mockInvocations = invocationRecorder.getInvocations(mock.uuid)
Expand Down Expand Up @@ -85,7 +95,7 @@ class InvocationRecorderTest {
val invocation1 = Invocation(METHOD_1, ARGS_1)
val uuid = mutableSetOf<String>()

(0 until iterations).forEach {
(0 until iterations).forEach { _ ->
val mock = Mocks.MyDependencyMock()
uuid.add(mock.uuid)
invocationRecorder.storeInvocation(mock.uuid, invocation1)
Expand Down Expand Up @@ -178,7 +188,10 @@ class InvocationRecorderTest {
invocationRecorder.getResponse(mock.uuid, invocation1)
} catch (ise: IllegalStateException) {
e = ise
assertEquals("Not mocked response for current object and instance, instance:${mock.uuid}, invocation: $invocation1", e.message)
assertEquals(
"Not mocked response for current object and instance, instance:${mock.uuid}, invocation: $invocation1",
e.message
)
}
assertNotNull(e)
}
Expand Down Expand Up @@ -247,7 +260,48 @@ class InvocationRecorderTest {
assertEquals(responseInv2, response2)
}

fun newMock(): Mock{
@Test
fun concurrentRecordInvocation() = runTest {
val jobs = mutableListOf<Job>()

repeat(10_000) {
launch(Dispatchers.Default) {
val invocation = Invocation("abc $it", mapOf(ARG_NAME_1 to "value1"))
invocationRecorder.storeInvocation("uuid", invocation)
}.also { jobs.add(it) }
}

jobs.joinAll()

assertEquals(10_000, invocationRecorder.getInvocations("uuid").size)
}

@Test
fun concurrentRecordAnswers() = runTest {
val jobs = mutableListOf<Job>()

repeat(1000) { counter ->
launch(Dispatchers.Default) {
val invocation = Invocation("abc $counter", mapOf(ARG_NAME_1 to "value1"))
invocationRecorder.storeAnswer("uuid", invocation) { "Yo $counter" }
}.also { jobs.add(it) }
}

jobs.joinAll()

repeat(1000) { counter ->
assertEquals(
"Yo $counter", invocationRecorder.getResponse(
"uuid",
Invocation("abc $counter", mapOf(ARG_NAME_1 to "value1")),
relaxed = false
)
)
}

}

private fun newMock(): Mock {
return object : Mock {
override val uuid: String by uuid()
}
Expand Down
3 changes: 2 additions & 1 deletion versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ kotlin = "1.9.10"
kspVersion = "1.9.10-1.0.13"
junit = "4.13.1"
jacoco = "0.8.10"
stately = "2.0.0-rc3"
stately = "2.0.5"
atomicFu = "0.22.0"
kotlinPoet = "1.14.2"
kotlinxMetadata = "0.7.0"
Expand All @@ -20,6 +20,7 @@ touchlab-stately-concurrency = { module = "co.touchlab:stately-concurrency", ver
touchlab-stately-concurrent-collections = { module = "co.touchlab:stately-concurrent-collections", version.ref = "stately" }
kotlinx-atomicfu = { module = "org.jetbrains.kotlinx:atomicfu", version.ref = "atomicFu" }
kotlinx-coroutines = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "coroutines" }
kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "coroutines" }
kotlin-test = { module = "org.jetbrains.kotlin:kotlin-test", version.ref = "kotlin" }
kotlin-test-common = { module = "org.jetbrains.kotlin:kotlin-test-common", version.ref = "kotlin" }
kotlin-test-annotations = { module = "org.jetbrains.kotlin:kotlin-test-annotations-common", version.ref = "kotlin" }
Expand Down