Skip to content

Commit

Permalink
Fix corner cases in tests discovered after running on Github Actions (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
cmelchior authored Nov 28, 2023
1 parent 6460014 commit 75b35dc
Show file tree
Hide file tree
Showing 20 changed files with 80 additions and 27 deletions.
2 changes: 1 addition & 1 deletion benchmarks/settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
// For CI buils, the packages are expected to have
// been built and deployed to a local filesystem
// maven repo.
if (System.getenv("JENKINS_HOME") == null) {
if (System.getenv("JENKINS_HOME") == null && System.getenv("CI") == null) {
includeBuild("../packages")
}

Expand Down
2 changes: 1 addition & 1 deletion buildSrc/src/main/kotlin/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private fun findHostOs(): OperatingSystem {
val HOST_OS: OperatingSystem = findHostOs()

object Realm {
val ciBuild = (System.getenv("JENKINS_HOME") != null)
val ciBuild = (System.getenv("JENKINS_HOME") != null || System.getenv("CI") != null)
const val version = "1.13.0-SNAPSHOT"
const val group = "io.realm.kotlin"
const val projectUrl = "https://realm.io"
Expand Down
2 changes: 1 addition & 1 deletion examples/kmm-sample/settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// been built and deployed to a local filesystem
// maven repo. We cannot reference `Realm.ciBuild`
// from buildSrc here.
if (System.getenv("JENKINS_HOME") == null) {
if (System.getenv("JENKINS_HOME") == null && System.getenv("CI") == null) {
includeBuild("../../packages")
}

Expand Down
2 changes: 1 addition & 1 deletion examples/realm-java-compatibility/settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
// been built and deployed to a local filesystem
// maven repo. We cannot reference `Realm.ciBuild`
// from buildSrc here.
if (System.getenv("JENKINS_HOME") == null) {
if (System.getenv("JENKINS_HOME") == null && System.getenv("CI") == null) {
includeBuild("../../packages")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,32 +300,30 @@ public interface Configuration {
} as S

/**
* Dispatcher used to run background writes to the Realm.
* Dispatcher on which Realm notifications are run. It is possible to listen for changes to
* Realm objects from any thread, but the underlying logic will run on this dispatcher
* before any changes are returned to the receiving context.
*
* Defaults to a single threaded dispatcher started when the configuration is built.
*
* NOTE On Android the dispatcher's thread must have an initialized
* [Looper](https://developer.android.com/reference/android/os/Looper#prepare()).
*
* @param dispatcher dispatcher on which writes are run. It is required to be backed by a
* single thread only.
* @param dispatcher dispatcher on which notifications are run. It is required to be backed
* by a single thread only.
*/
internal fun notificationDispatcher(dispatcher: CoroutineDispatcher) = apply {
this.notificationDispatcher = dispatcher
} as S

/**
* Dispatcher on which Realm notifications are run. It is possible to listen for changes to
* Realm objects from any thread, but the underlying logic will run on this dispatcher
* before any changes are returned to the caller thread.
*
* Defaults to a single threaded dispatcher started when the configuration is built.
* Dispatcher used to run background writes to the Realm. *
*
* NOTE On Android the dispatcher's thread must have an initialized
* [Looper](https://developer.android.com/reference/android/os/Looper#prepare()).
*
* @param dispatcher Dispatcher on which notifications are run. It is required to be backed
* by a single thread only.
* @param dispatcher dispatcher on which writes are run. It is required to be backed by a
* single thread only.
*/
internal fun writeDispatcher(dispatcher: CoroutineDispatcher) = apply {
this.writeDispatcher = dispatcher
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import io.realm.kotlin.internal.interop.RealmInterop
import io.realm.kotlin.internal.interop.RealmSchemaPointer
import io.realm.kotlin.internal.interop.SchemaMode
import io.realm.kotlin.internal.interop.use
import io.realm.kotlin.internal.platform.PATH_SEPARATOR
import io.realm.kotlin.internal.platform.appFilesDirectory
import io.realm.kotlin.internal.platform.prepareRealmFilePath
import io.realm.kotlin.internal.platform.realmObjectCompanionOrThrow
Expand Down Expand Up @@ -255,8 +256,8 @@ public open class ConfigurationImpl(
private fun normalizePath(directoryPath: String, fileName: String): String {
var dir = directoryPath.ifEmpty { appFilesDirectory() }
// If dir is a relative path, replace with full path for easier debugging
if (dir.startsWith("./")) {
dir = dir.replaceFirst("./", "${appFilesDirectory()}/")
if (dir.startsWith(".$PATH_SEPARATOR")) {
dir = dir.replaceFirst(".$PATH_SEPARATOR", "${appFilesDirectory()}$PATH_SEPARATOR")
}
return prepareRealmFilePath(dir, fileName)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ class Decimal128Tests {

@AfterTest
fun tearDown() {
if (this::realm.isInitialized) {
realm.close()
}
PlatformUtils.deleteTempDir(tmpDir)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ class EmbeddedRealmObjectTests {

@AfterTest
fun tearDown() {
if (this::realm.isInitialized) {
realm.close()
}
PlatformUtils.deleteTempDir(tmpDir)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ class FqNameImportTests {

@AfterTest
fun tearDown() {
if (this::realm.isInitialized) {
realm.close()
}
PlatformUtils.deleteTempDir(tmpDir)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ class ImportTests {

@AfterTest
fun tearDown() {
if (this::realm.isInitialized) {
realm.close()
}
PlatformUtils.deleteTempDir(tmpDir)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ class LinkTests {

@AfterTest
fun tearDown() {
if (this::realm.isInitialized) {
realm.close()
}
PlatformUtils.deleteTempDir(tmpDir)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ class NullabilityTests {

@AfterTest
fun tearDown() {
if (this::realm.isInitialized) {
realm.close()
}
PlatformUtils.deleteTempDir(tmpDir)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import io.realm.kotlin.notifications.UpdatedResults
import io.realm.kotlin.test.platform.PlatformUtils
import io.realm.kotlin.types.RealmList
import io.realm.kotlin.types.RealmObject
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CloseableCoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.async
import kotlinx.coroutines.cancel
Expand All @@ -57,7 +57,7 @@ import kotlin.test.Test
*/
class ReadMeTests {
private lateinit var scope: CoroutineScope
private lateinit var context: CoroutineDispatcher
private lateinit var context: CloseableCoroutineDispatcher
lateinit var tmpDir: String
lateinit var realm: Realm

Expand All @@ -79,6 +79,7 @@ class ReadMeTests {
scope.cancel()
context.cancel()
realm.close()
context.close()
PlatformUtils.deleteTempDir(tmpDir)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class RealmConfigurationTests {

val configFromBuilderWithCurrentDir: RealmConfiguration =
RealmConfiguration.Builder(schema = setOf(Sample::class))
.directory("./my_dir")
.directory(pathOf(".", "my_dir"))
.name("foo.realm")
.build()
assertEquals(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ class RealmInMemoryTests {

@AfterTest
fun tearDown() {
PlatformUtils.deleteTempDir(tmpDir)
if (this::realm.isInitialized && !realm.isClosed()) {
realm.close()
}
PlatformUtils.deleteTempDir(tmpDir)
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class RealmMigrationTests {
assertNull(newSchema["SchemaVariations"])
}
}
)
).close()
}

// TODO Test all schema modifications (theoretically test core behavior, so postponed for now)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ class RealmNotificationsTests : FlowableTests {

runBlocking {
val listener = async {
withTimeout(10.seconds) {
withTimeout(30.seconds) {
assertFailsWith<CancellationException> {
flow.collect {
delay(1000.milliseconds)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,9 @@ class SystemNotificationTests {
writer1.write { copyToRealm(Sample()) }
writer2.write { copyToRealm(Sample()) }
}
writer1.close()
writer2.close()
liveRealmContext.close()
baseRealm.close()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@

package io.realm.kotlin.test.platform

import java.io.File
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths
import java.nio.file.attribute.PosixFilePermission
import java.util.stream.Collectors
import kotlin.io.path.absolutePathString
import kotlin.time.Duration

Expand All @@ -40,7 +41,22 @@ actual object PlatformUtils {
}

actual fun deleteTempDir(path: String) {
File(path).deleteRecursively()
val rootPath: Path = Paths.get(path)
val pathsToDelete: List<Path> =
Files.walk(rootPath).sorted(Comparator.reverseOrder()).collect(Collectors.toList())
for (p in pathsToDelete) {
try {
Files.deleteIfExists(p)
} catch (e: java.nio.file.FileSystemException) {
// Sometimes (on Windows) we need the give a GC a chance to run and close all native pointers
// before we can delete the Realm, otherwise delete will fail with " The process cannot access the
// file because it is being used by another process".
//
// We try to trigger the GC once then retry the delete.
triggerGC()
Files.deleteIfExists(p)
}
}
}

actual fun sleep(duration: Duration) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,13 @@ class RealmTests {
// Closing a Realm should also cleanup our default (internal) dispatchers.
// The core notifier and the finalizer thread will never be closed.
val expectedThreadCount = initialThreads.size + 1 /* core-notifier */ + if (finalizerRunning) 0 else 1
var counter = 5 // Wait 5 seconds for threads to settle
while (totalThreadCount() != expectedThreadCount && counter > 0) {
var counter = 10 // Wait 10 seconds for threads to settle
while (newThreads().any { !it.isDaemon } && counter > 0) {
delay(1000)
counter--
}
assertEquals(expectedThreadCount, totalThreadCount(), "Unexpected thread count after closing realm: ${newThreads()}")
val totalThreadCount = totalThreadCount()
assertTrue(totalThreadCount <= expectedThreadCount, "Unexpected thread count after closing realm: $expectedThreadCount <= $totalThreadCount. New threads: ${newThreads()}. Threads: ${threadTrace()}")

// Verify that all remaining threads are daemon threads, so that we don't keep the JVM alive
newThreads().filter { !it.isDaemon }.let {
Expand Down Expand Up @@ -122,4 +123,18 @@ class RealmTests {
Unit
}
}

private fun threadTrace(): String {
val sb = StringBuilder()
sb.appendLine("--------------------------------")
val stack = Thread.getAllStackTraces()
stack.keys
.sortedBy { it.name }
.forEach { t: Thread ->
sb.appendLine("${t.name} - Is Daemon ${t.isDaemon} - Is Alive ${t.isAlive}")
}
sb.appendLine("All threads: ${stack.keys.size}")
sb.appendLine("Active threads: ${Thread.activeCount()}")
return sb.toString()
}
}

0 comments on commit 75b35dc

Please sign in to comment.