Conversation
45c4fa3 to
79040b3
Compare
| sqldelightAndroid = { module = "com.squareup.sqldelight:android-driver", version.ref = "sqldelight" } | ||
| sqldelightCoroutinesExt = { module = "com.squareup.sqldelight:coroutines-extensions", version.ref = "sqldelight" } | ||
| sqliteJdbc = { module = "org.xerial:sqlite-jdbc", version = "3.36.0" } | ||
| statelyIsoCollections = { module = "co.touchlab:stately-iso-collections", version = "1.2.0" } |
There was a problem hiding this comment.
From the comments here, it seems like this library is completely deprecated and seems to say that the ArrayDeque that is now part of the Kotlin Standard Library should be enough. Do all the tests still pass if we swap to the stdlib version?
There was a problem hiding this comment.
[…] it seems like this library is completely deprecated […]
The library isn't deprecated, but the module is. Unfortunately, the proposed replacement (stately-concurrent-collections) for this module isn't stable yet.
[…] and seems to say that the
ArrayDequethat is now part of the Kotlin Standard Library should be enough.
A KMP version of java.util.ArrayDeque was added in Kotlin 1.3.70. Like java.util.ArrayDeque, kotlin.collections.ArrayDeque isn't thread-safe.
Do all the tests still pass if we swap to the stdlib version?
From the previous point, ArrayQueue isn't thread-safe. Swapping out IsoArrayDeque with ArrayDeque could make all the tests pass, but those tests would presumably be a bit flaky.
|
Hi @veyndan thank you for your patience. After we merged your other PRs removing most usages of TestDispatcher, the remaining tests still using it appears to be single threaded access to the queue. I think its safe to at this point to switch to |
daedd4a to
c46c51a
Compare
c46c51a to
a261291
Compare
|
@claraf3 Sounds good to me! I've just updated the PR to use |
|
Awesome, thank you Veyndan! Yeah it looks to be our issue. Don't worry about it :) |
There was a problem hiding this comment.
We're getting a lint failure on this file:
$SUPPORT/testutils/testutils-ktx/src/jvmMain/kotlin/androidx/testutils/VerifyWithPolling.kt:29: Error: Uses Thread.sleep() [BanThreadSleep from androidx.build]
Thread.sleep(periodMs)
~~~~~
Explanation for issues of type "BanThreadSleep":
Use of Thread.sleep() is not allowed, please use a callback or another way
to make more reliable code. See more details at
https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:docs/testability.md#calling-threadsleep-as-a-synchronization-barrier
Do you mind seeing if you can suppress the BanThreadSleep check here? Ideally you should be able to reproduce this with by running :internal-testutils-ktx:lint
There was a problem hiding this comment.
I tried recreating this issue locally, but it always passes. I'm running the following command within the paging directory:
./gradlew :internal-testutils-ktx:lint --rerun-tasks --no-configuration-cache
There was a problem hiding this comment.
I was able to repro the error on my end. I added the suppression and its running through checks. Should be good now, thanks for trying!
Upstreaming Multiplatform Paging's kmp-ified testutils-ktx. See #505 (comment) for more context.
Test: ./gradlew test connectedCheck
Bug: 270612487