Skip to content

chore: re-enable ignored Kotlin/Native tests #1379

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

Draft
wants to merge 2 commits into
base: kn-main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/
package aws.smithy.kotlin.runtime.auth.awssigning

import aws.smithy.kotlin.runtime.IgnoreNative
import aws.smithy.kotlin.runtime.auth.awscredentials.Credentials
import aws.smithy.kotlin.runtime.time.Instant
import aws.smithy.kotlin.runtime.util.PlatformProvider
Expand Down Expand Up @@ -69,7 +68,6 @@ private val TESTS = listOf(
* Tests for [SigV4aSignatureCalculator]. Currently only tests forming the string-to-sign.
*/
class SigV4aSignatureCalculatorTest {
@IgnoreNative // FIXME test resources are not loadable on iOS: https://youtrack.jetbrains.com/issue/KT-49981/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you have to do anything to fix this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worked on this test before and I didn't need to do anything to fix it. I don't know if Ian's work changed something related though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this seems to Just Work™ now.

@Test
fun testStringToSign() = TESTS.forEach { testId ->
runTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/
package aws.smithy.kotlin.runtime.http.auth

import aws.smithy.kotlin.runtime.IgnoreNative
import aws.smithy.kotlin.runtime.auth.awscredentials.Credentials
import aws.smithy.kotlin.runtime.auth.awscredentials.CredentialsProvider
import aws.smithy.kotlin.runtime.auth.awssigning.AwsSigner
Expand Down Expand Up @@ -127,7 +126,6 @@ public abstract class AwsHttpSignerTestBase(
assertEquals(expectedSig, signed.headers["Authorization"])
}

@IgnoreNative // FIXME Our JVM implementation does not sign `transfer-encoding`, but CRT does, causing a signature mismatch. Upgrade to latest version of aws-c-auth to get the fix.
@Test
public fun testSignAwsChunkedStreamNonReplayable(): TestResult = runTest {
val op = buildOperation(streaming = true, replayable = false, requestBody = "a".repeat(AWS_CHUNKED_THRESHOLD + 1))
Expand All @@ -141,7 +139,6 @@ public abstract class AwsHttpSignerTestBase(
assertEquals(expectedSig, signed.headers["Authorization"])
}

@IgnoreNative // FIXME Our JVM implementation does not sign `transfer-encoding`, but CRT does, causing a signature mismatch. Upgrade to latest version of aws-c-auth to get the fix.
@Test
public fun testSignAwsChunkedStreamReplayable(): TestResult = runTest {
val op = buildOperation(streaming = true, replayable = true, requestBody = "a".repeat(AWS_CHUNKED_THRESHOLD + 1))
Expand All @@ -155,7 +152,6 @@ public abstract class AwsHttpSignerTestBase(
assertEquals(expectedSig, signed.headers["Authorization"])
}

@IgnoreNative // FIXME Our JVM implementation does not sign `transfer-encoding`, but CRT does, causing a signature mismatch. Upgrade to latest version of aws-c-auth to get the fix.
@Test
public fun testSignOneShotStream(): TestResult = runTest {
val op = buildOperation(streaming = true, replayable = false)
Expand Down
18 changes: 15 additions & 3 deletions runtime/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/
import aws.sdk.kotlin.gradle.dsl.configurePublishing
import aws.sdk.kotlin.gradle.kmp.*
import org.gradle.kotlin.dsl.apply
import org.gradle.kotlin.dsl.withType
import aws.sdk.kotlin.gradle.kmp.configureKmpTargets
import aws.sdk.kotlin.gradle.kmp.kotlin
import aws.sdk.kotlin.gradle.kmp.needsKmpConfigured
import org.jetbrains.kotlin.gradle.dsl.JvmTarget
import org.jetbrains.kotlin.gradle.targets.native.tasks.KotlinNativeSimulatorTest

Expand Down Expand Up @@ -118,6 +118,18 @@ subprojects {
tasks.withType<KotlinNativeSimulatorTest> {
enabled = false
}

tasks.withType<AbstractTestTask> {
if (this is Test) useJUnitPlatform()

testLogging {
events("passed", "skipped", "failed")
showStandardStreams = true
showStackTraces = true
showExceptions = true
exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL
}
}
}

// configureIosSimulatorTasks()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@

package aws.smithy.kotlin.runtime.http.engine.crt

import aws.smithy.kotlin.runtime.IgnoreNative
import aws.smithy.kotlin.runtime.http.HttpMethod
import aws.smithy.kotlin.runtime.http.SdkHttpClient
import aws.smithy.kotlin.runtime.http.complete
import aws.smithy.kotlin.runtime.http.request.HttpRequestBuilder
import aws.smithy.kotlin.runtime.http.request.url
import aws.smithy.kotlin.runtime.httptest.TestWithLocalServer
import aws.smithy.kotlin.runtime.io.use
import aws.smithy.kotlin.runtime.net.Host
import aws.smithy.kotlin.runtime.net.Scheme
import io.ktor.server.cio.*
Expand All @@ -37,37 +37,38 @@ class AsyncStressTest : TestWithLocalServer() {
}
}.start()

@IgnoreNative // FIXME TlsContext needs to initialize CRT. Segmentation fault.
@Test
fun testStreamNotConsumed() = runBlocking {
// test that filling the stream window and not consuming the body stream still cleans up resources
// appropriately and allows requests to proceed (a stream that isn't consumed will be in a stuck state
// if the window is full and never incremented again, this can lead to all connections being consumed
// and the engine to no longer make further requests)
val client = SdkHttpClient(CrtHttpEngine())
val request = HttpRequestBuilder().apply {
url {
scheme = Scheme.HTTP
method = HttpMethod.GET
host = Host.Domain(testHost)
port = serverPort
path.decoded = "/largeResponse"
CrtHttpEngine().use { engine ->
val client = SdkHttpClient(engine)
val request = HttpRequestBuilder().apply {
url {
scheme = Scheme.HTTP
method = HttpMethod.GET
host = Host.Domain(testHost)
port = serverPort
path.decoded = "/largeResponse"
}
}
}

withTimeout(10.seconds) {
repeat(1_000) {
async {
try {
val call = client.call(request)
yield()
call.complete()
} catch (ex: Exception) {
println("exception on $it: $ex")
throw ex
withTimeout(10.seconds) {
repeat(1_000) {
async {
try {
val call = client.call(request)
yield()
call.complete()
} catch (ex: Exception) {
println("exception on $it: $ex")
throw ex
}
}
yield()
}
yield()
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions runtime/runtime-core/api/runtime-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ public final class aws/smithy/kotlin/runtime/ErrorMetadata$Companion {
public abstract interface annotation class aws/smithy/kotlin/runtime/ExperimentalApi : java/lang/annotation/Annotation {
}

public abstract interface annotation class aws/smithy/kotlin/runtime/IgnoreNative : java/lang/annotation/Annotation {
}

public abstract interface annotation class aws/smithy/kotlin/runtime/InternalApi : java/lang/annotation/Annotation {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,3 @@ public annotation class ExperimentalApi
*/
@DslMarker
public annotation class SdkDsl

/**
* Marks a test that should be ignored on Native platforms
*/
@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION)
public expect annotation class IgnoreNative()

This file was deleted.

This file was deleted.

Loading