Skip to content

misc: enable ignored kn tests #1292

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.

Sign up for GitHub

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 1 commit into
base: kn-main
Choose a base branch
from
Draft

misc: enable ignored kn tests #1292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion ...lt/common/test/aws/smithy/kotlin/runtime/auth/awssigning/SigV4aSignatureCalculatorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,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/
@Test
fun testStringToSign() = TESTS.forEach { testId ->
runTest {
Expand Down
4 changes: 0 additions & 4 deletions ...th/http-auth-aws/common/test/aws/smithy/kotlin/runtime/http/auth/AwsHttpSignerTestBase.kt
View file
Open in desktop
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 @@ -147,7 +146,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 @@ -161,7 +159,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 @@ -175,7 +172,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
5 changes: 2 additions & 3 deletions ...engine-crt/jvmAndNative/test/aws/smithy/kotlin/runtime/http/engine/crt/AsyncStressTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

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
Expand Down Expand Up @@ -37,7 +36,6 @@ 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
Expand All @@ -55,13 +53,14 @@ class AsyncStressTest : TestWithLocalServer() {
}
}

withTimeout(10.seconds) {
withTimeout(1000.seconds) {
Comment on lines -58 to +56
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: 16 minutes and 40 seconds is a very long timeout. Shouldn't we be able to handle 1000 calls in 10 seconds? How long is this test actually taking?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1000 is just for testing, I'll bring it back to 10 when this is ready for review. I'm currently just testing runs of the KN tests in CI, I forgot to say so in the description. Thanks for taking a preemptive look at it though.

repeat(1_000) {
async {
try {
val call = client.call(request)
yield()
call.complete()
println("Call complete for #$it")
} catch (ex: Exception) {
println("exception on $it: $ex")
throw ex
Expand Down
Loading