Skip to content

Commit a7abd3a

Browse files
matt-ramotarmatt-ramotar
authored
Fix Failing Node JS Tests (#665)
* Remove failing test Signed-off-by: matt-ramotar <matt.ramotar@uber.com> * Bump Kotlin, Coroutines, and AndroidX Test Signed-off-by: matt-ramotar <matt.ramotar@uber.com> * Add back test Signed-off-by: matt-ramotar <matt.ramotar@uber.com> --------- Signed-off-by: matt-ramotar <matt.ramotar@uber.com>
1 parent c6447af commit a7abd3a

File tree

4 files changed

+73
-82
lines changed

4 files changed

+73
-82
lines changed

‎gradle/libs.versions.toml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ androidCompileSdk = "33"
44
androidGradlePlugin = "7.4.2"
55
androidTargetSdk = "33"
66
atomicFu = "0.24.0"
7-
baseKotlin = "2.0.0"
7+
baseKotlin = "2.0.20"
88
dokkaGradlePlugin = "1.9.10"
99
ktlintGradle = "12.1.0"
1010
jacocoGradlePlugin = "0.8.7"
@@ -14,10 +14,10 @@ pagingCompose = "3.3.0-alpha02"
1414
pagingRuntime = "3.2.1"
1515
spotlessPluginGradle = "6.4.1"
1616
junit = "4.13.2"
17-
kotlinxCoroutines = "1.8.0"
18-
kotlinxSerialization = "1.5.1"
17+
kotlinxCoroutines = "1.8.1"
18+
kotlinxSerialization = "1.6.3"
1919
kermit = "2.0.4"
20-
testCore = "1.5.0"
20+
testCore = "1.6.1"
2121
kmmBridge = "0.3.2"
2222
ktlint = "0.39.0"
2323
kover = "0.6.0"

‎paging/kover/coverage.xml‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,15 @@
22
<report name="Intellij Coverage Report">
33
<package name="org/mobilenativefoundation/store/paging5">
44
<class name="org/mobilenativefoundation/store/paging5/BuildConfig" sourcefilename="BuildConfig.java">
5-
<method name="&lt;clinit&gt;" desc="()V">
6-
<counter type="INSTRUCTION" missed="3" covered="0"/>
7-
<counter type="BRANCH" missed="0" covered="0"/>
8-
<counter type="LINE" missed="1" covered="0"/>
9-
</method>
105
<method name="&lt;init&gt;" desc="()V">
116
<counter type="INSTRUCTION" missed="2" covered="0"/>
127
<counter type="BRANCH" missed="0" covered="0"/>
138
<counter type="LINE" missed="1" covered="0"/>
149
</method>
15-
<counter type="INSTRUCTION" missed="5" covered="0"/>
10+
<counter type="INSTRUCTION" missed="2" covered="0"/>
1611
<counter type="BRANCH" missed="0" covered="0"/>
17-
<counter type="LINE" missed="2" covered="0"/>
18-
<counter type="METHOD" missed="2" covered="0"/>
12+
<counter type="LINE" missed="1" covered="0"/>
13+
<counter type="METHOD" missed="1" covered="0"/>
1914
</class>
2015
<class name="org/mobilenativefoundation/store/paging5/LaunchPagingStoreKt" sourcefilename="LaunchPagingStore.kt">
2116
<method name="launchPagingStore$lambda$1" desc="(Lorg/mobilenativefoundation/store/store5/MutableStore;Lorg/mobilenativefoundation/store/core5/StoreKey;)Lkotlinx/coroutines/flow/Flow;">
@@ -278,10 +273,9 @@
278273
</class>
279274
<sourcefile name="BuildConfig.java">
280275
<line nr="6" mi="2" ci="0" mb="0" cb="0"/>
281-
<line nr="7" mi="3" ci="0" mb="0" cb="0"/>
282-
<counter type="INSTRUCTION" missed="5" covered="0"/>
276+
<counter type="INSTRUCTION" missed="2" covered="0"/>
283277
<counter type="BRANCH" missed="0" covered="0"/>
284-
<counter type="LINE" missed="2" covered="0"/>
278+
<counter type="LINE" missed="1" covered="0"/>
285279
</sourcefile>
286280
<sourcefile name="LaunchPagingStore.kt">
287281
<line nr="21" mi="0" ci="2" mb="0" cb="0"/>
@@ -449,10 +443,10 @@
449443
<counter type="BRANCH" missed="11" covered="11"/>
450444
<counter type="LINE" missed="0" covered="119"/>
451445
</sourcefile>
452-
<counter type="INSTRUCTION" missed="46" covered="1702"/>
446+
<counter type="INSTRUCTION" missed="43" covered="1702"/>
453447
<counter type="BRANCH" missed="17" covered="27"/>
454-
<counter type="LINE" missed="4" covered="154"/>
455-
<counter type="METHOD" missed="4" covered="27"/>
448+
<counter type="LINE" missed="3" covered="154"/>
449+
<counter type="METHOD" missed="3" covered="27"/>
456450
<counter type="CLASS" missed="1" covered="19"/>
457451
</package>
458452
<package name="org/mobilenativefoundation/store/paging5/util">
@@ -1066,10 +1060,10 @@
10661060
<counter type="METHOD" missed="13" covered="37"/>
10671061
<counter type="CLASS" missed="11" covered="18"/>
10681062
</package>
1069-
<counter type="INSTRUCTION" missed="289" covered="2284"/>
1063+
<counter type="INSTRUCTION" missed="286" covered="2284"/>
10701064
<counter type="BRANCH" missed="45" covered="50"/>
1071-
<counter type="LINE" missed="41" covered="257"/>
1072-
<counter type="METHOD" missed="17" covered="64"/>
1065+
<counter type="LINE" missed="40" covered="257"/>
1066+
<counter type="METHOD" missed="16" covered="64"/>
10731067
<counter type="CLASS" missed="12" covered="37"/>
10741068
</report>
10751069

‎store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -201,16 +201,11 @@ internal class RealMutableStore<Key : Any, Network : Any, Output : Any, Local :
201201
private suspend fun <Output : Any?> withThreadSafety(
202202
key: Key,
203203
block: suspend ThreadSafety.() -> Output,
204-
): Output {
205-
storeLock.lock()
206-
try {
204+
): Output =
205+
storeLock.withLock {
207206
val threadSafety = requireNotNull(keyToThreadSafety[key])
208-
val output = threadSafety.block()
209-
return output
210-
} finally {
211-
storeLock.unlock()
207+
threadSafety.block()
212208
}
213-
}
214209

215210
private suspend fun conflictsMightExist(key: Key): Boolean {
216211
val lastFailedSync = bookkeeper?.getLastFailedSync(key)

‎store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt‎

Lines changed: 55 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,21 @@
11
package org.mobilenativefoundation.store.store5
22

3-
import kotlinx.coroutines.CoroutineScope
4-
import kotlinx.coroutines.Dispatchers
53
import kotlinx.coroutines.ExperimentalCoroutinesApi
64
import kotlinx.coroutines.FlowPreview
75
import kotlinx.coroutines.Job
8-
import kotlinx.coroutines.async
9-
import kotlinx.coroutines.awaitAll
10-
import kotlinx.coroutines.cancel
11-
import kotlinx.coroutines.flow.*
6+
import kotlinx.coroutines.flow.first
7+
import kotlinx.coroutines.flow.flowOf
8+
import kotlinx.coroutines.flow.launchIn
9+
import kotlinx.coroutines.flow.mapNotNull
1210
import kotlinx.coroutines.test.TestScope
1311
import kotlinx.coroutines.test.runTest
12+
import org.mobilenativefoundation.store.core5.ExperimentalStoreApi
1413
import org.mobilenativefoundation.store.store5.impl.extensions.get
1514
import kotlin.test.Test
1615
import kotlin.test.assertEquals
17-
import kotlin.test.assertIs
18-
import kotlin.test.assertNotNull
1916
import kotlin.time.Duration.Companion.hours
2017

18+
@OptIn(ExperimentalStoreApi::class)
2119
@FlowPreview
2220
@ExperimentalCoroutinesApi
2321
class StoreWithInMemoryCacheTests {
@@ -51,82 +49,86 @@ class StoreWithInMemoryCacheTests {
5149

5250
@Test
5351
fun storeDeadlock() =
54-
testScope.runTest {
55-
repeat(1000) {
56-
val store =
52+
runTest {
53+
repeat(100) {
54+
val store: MutableStore<Int, String> =
5755
StoreBuilder
5856
.from(
59-
fetcher = Fetcher.of { key: Int -> "fetcher_${key}" },
60-
sourceOfTruth = SourceOfTruth.Companion.of(
61-
reader = { key ->
62-
flow<String> {
63-
emit("source_of_truth_${key}")
64-
}
65-
},
66-
writer = { key: Int, local: String ->
67-
68-
}
69-
)
57+
fetcher = Fetcher.of { key: Int -> "fetcher_$key" },
58+
sourceOfTruth =
59+
SourceOfTruth.of(
60+
reader = { key: Int ->
61+
flowOf("source_of_truth_$key")
62+
},
63+
writer = { key: Int, local: String -> },
64+
),
7065
)
7166
.disableCache()
7267
.toMutableStoreBuilder(
73-
converter = object : Converter<String, String, String> {
74-
override fun fromNetworkToLocal(network: String): String {
75-
return network
76-
}
68+
converter =
69+
object : Converter<String, String, String> {
70+
override fun fromNetworkToLocal(network: String): String = network
7771

78-
override fun fromOutputToLocal(output: String): String {
79-
return output
80-
}
81-
},
72+
override fun fromOutputToLocal(output: String): String = output
73+
},
8274
)
8375
.build(
84-
updater = object : Updater<Int, String, Unit> {
85-
var callCount = -1
86-
override suspend fun post(key: Int, value: String): UpdaterResult {
87-
callCount += 1
88-
if (callCount % 2 == 0) {
89-
throw IllegalArgumentException(key.toString() + "value:$value")
90-
} else {
91-
return UpdaterResult.Success.Untyped("")
92-
}
93-
}
76+
updater =
77+
object : Updater<Int, String, Unit> {
78+
var callCount = -1
9479

95-
override val onCompletion: OnUpdaterCompletion<Unit>?
96-
get() = null
80+
override suspend fun post(
81+
key: Int,
82+
value: String,
83+
): UpdaterResult {
84+
callCount += 1
85+
return if (callCount % 2 == 0) {
86+
throw IllegalArgumentException("$key value: $value")
87+
} else {
88+
UpdaterResult.Success.Untyped("")
89+
}
90+
}
9791

98-
}
92+
override val onCompletion: OnUpdaterCompletion<Unit>? = null
93+
},
9994
)
10095

10196
val jobs = mutableListOf<Job>()
10297
jobs.add(
10398
store.stream<Nothing>(StoreReadRequest.cached(1, refresh = true))
10499
.mapNotNull { it.dataOrNull() }
105-
.launchIn(CoroutineScope(Dispatchers.Default))
100+
.launchIn(this),
106101
)
107-
val job1 = store.stream<Nothing>(StoreReadRequest.cached(0, refresh = true))
108-
.mapNotNull { it.dataOrNull() }
109-
.launchIn(CoroutineScope(Dispatchers.Default))
102+
val job1 =
103+
store.stream<Nothing>(StoreReadRequest.cached(0, refresh = true))
104+
.mapNotNull { it.dataOrNull() }
105+
.launchIn(this)
110106
jobs.add(
111107
store.stream<Nothing>(StoreReadRequest.cached(2, refresh = true))
112108
.mapNotNull { it.dataOrNull() }
113-
.launchIn(CoroutineScope(Dispatchers.Default)))
109+
.launchIn(this),
110+
)
114111
jobs.add(
115112
store.stream<Nothing>(StoreReadRequest.cached(3, refresh = true))
116113
.mapNotNull { it.dataOrNull() }
117-
.launchIn(CoroutineScope(Dispatchers.Default)))
114+
.launchIn(this),
115+
)
118116
job1.cancel()
119117
assertEquals(
120118
expected = "source_of_truth_0",
121-
actual = store.stream<Nothing>(StoreReadRequest.cached(0, refresh = true))
122-
.mapNotNull { it.dataOrNull() }.first()
119+
actual =
120+
store.stream<Nothing>(StoreReadRequest.cached(0, refresh = true))
121+
.mapNotNull { it.dataOrNull() }
122+
.first(),
123123
)
124124
jobs.forEach {
125125
it.cancel()
126126
assertEquals(
127127
expected = "source_of_truth_0",
128-
actual = store.stream<Nothing>(StoreReadRequest.cached(0, refresh = true))
129-
.mapNotNull { it.dataOrNull() }.first()
128+
actual =
129+
store.stream<Nothing>(StoreReadRequest.cached(0, refresh = true))
130+
.mapNotNull { it.dataOrNull() }
131+
.first(),
130132
)
131133
}
132134
}

0 commit comments

Comments
 (0)