-
Notifications
You must be signed in to change notification settings - Fork 3
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
Conversation
} | ||
|
||
val commonTest by getting { | ||
dependencies { | ||
implementation(libs.kotlin.test) |
There was a problem hiding this comment.
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 :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
There was a problem hiding this comment.
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
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:(
|
||
|
||
@SharedImmutable |
There was a problem hiding this comment.
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
* @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 -> |
There was a problem hiding this comment.
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
@@ -151,6 +147,16 @@ internal fun <T : Mock> T.rawVerify( | |||
} | |||
} | |||
|
|||
private fun assertEquals( |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
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>>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
recorder
andresponses
maps access where not threadsafe