PR feedback

Author: lauzadis

runtime/runtime-core/build.gradle.kts

@@ -1,10 +1,10 @@
-import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinNativeTarget
-
 /*
  * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinNativeTarget
+
 plugins {
     alias(libs.plugins.kotlinx.serialization)
 }

runtime/runtime-core/common/test/aws/smithy/kotlin/runtime/util/SystemPlatformProviderTest.kt

@@ -18,7 +18,7 @@ class SystemPlatformProviderTest {
         val ps = PlatformProvider.System
 
         val tempDir = if (ps.osInfo().family == OsFamily.Windows) {
-            ps.getenv("TEMP") ?: "C:\\Windows\\Temp"
+            requireNotNull(ps.getenv("TEMP")) { "%TEMP% unexpectedly null" }
         } else {
             "/tmp"
         }
@@ -36,8 +36,9 @@ class SystemPlatformProviderTest {
     @Test
     fun testGetEnv() = runTest {
         val envVarKeys = listOf("PATH", "USERPROFILE") // PATH is not set on Windows CI
-        val result: String? = envVarKeys.fold(null) { acc, curr -> acc ?: PlatformProvider.System.getenv(curr) }
-        assertNotNull(result)
+        assertNotNull(
+            envVarKeys.firstNotNullOfOrNull { PlatformProvider.System.getenv(it) }
+        )
 
         assertNull(PlatformProvider.System.getenv("THIS_ENV_VAR_IS_NOT_SET"))
     }
@@ -48,12 +49,9 @@ class SystemPlatformProviderTest {
         assertTrue(allEnv.isNotEmpty())
 
         val envVarKeys = listOf("PATH", "USERPROFILE") // PATH is not set on Windows CI
-
-        var envContainsKey = false
-        envVarKeys.forEach { key ->
-            envContainsKey = envContainsKey || allEnv.contains(key)
-        }
-        assertTrue(envContainsKey)
+        assertTrue(
+            envVarKeys.any { allEnv.contains(it) }
+        )
     }
 
     @Test

runtime/runtime-core/linux/test/aws/smithy/kotlin/runtime/util/SystemPlatformProviderTestLinux.kt

@@ -0,0 +1,17 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package aws.smithy.kotlin.runtime.util
+
+import kotlinx.coroutines.test.runTest
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class SystemPlatformProviderTestLinux {
+    @Test
+    fun testOsInfo() = runTest {
+        val osInfo = PlatformProvider.System.osInfo()
+        assertEquals(OsFamily.Linux, osInfo.family)
+    }
+}

runtime/runtime-core/native/src/aws/smithy/kotlin/runtime/util/PlatformNative.kt

@@ -5,8 +5,10 @@
 package aws.smithy.kotlin.runtime.util
 
 import aws.smithy.kotlin.runtime.io.IOException
+import aws.smithy.kotlin.runtime.io.internal.SdkDispatchers
 import aws.smithy.platform.posix.get_environ_ptr
 import kotlinx.cinterop.*
+import kotlinx.coroutines.withContext
 import platform.posix.*
 
 internal actual object SystemDefaultProvider : PlatformProvider {
@@ -17,7 +19,7 @@ internal actual object SystemDefaultProvider : PlatformProvider {
             .takeWhile { it != null }
             .associate { env ->
                 val parts = env?.split("=", limit = 2)
-                check(parts?.size == 2) { "Environment entry $env is malformed" }
+                check(parts?.size == 2) { "Environment entry \"$env\" is malformed" }
                 parts[0] to parts[1]
             }
     }
@@ -30,9 +32,9 @@ internal actual object SystemDefaultProvider : PlatformProvider {
             else -> "/"
         }
 
-    actual override suspend fun readFileOrNull(path: String): ByteArray? {
-        return try {
-            val file = fopen(path, "rb") ?: return null
+    actual override suspend fun readFileOrNull(path: String): ByteArray? = withContext(SdkDispatchers.IO) {
+        try {
+            val file = fopen(path, "rb") ?: return@withContext null
 
             try {
                 // Get file size
@@ -42,7 +44,7 @@ internal actual object SystemDefaultProvider : PlatformProvider {
 
                 // Read file content
                 val buffer = ByteArray(size.toInt()).pin()
-                val rc = fread(buffer.addressOf(0), 1.toULong(), size.toULong(), file)
+                val rc = fread(buffer.addressOf(0), 1uL, size.toULong(), file)
                 if (rc == size.toULong()) buffer.get() else null
             } finally {
                 fclose(file)
@@ -52,10 +54,10 @@ internal actual object SystemDefaultProvider : PlatformProvider {
         }
     }
 
-    actual override suspend fun writeFile(path: String, data: ByteArray) {
+    actual override suspend fun writeFile(path: String, data: ByteArray) = withContext(SdkDispatchers.IO) {
         val file = fopen(path, "wb") ?: throw IOException("Cannot open file for writing: $path")
         try {
-            val wc = fwrite(data.refTo(0), 1.toULong(), data.size.toULong(), file)
+            val wc = fwrite(data.refTo(0), 1uL, data.size.toULong(), file)
             if (wc != data.size.toULong()) {
                 throw IOException("Failed to write all bytes to file $path, expected ${data.size.toLong()}, wrote $wc")
             }