Skip to content
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

Jump to bottom

Dokka and coroutine version bump #514

Merged
merged 21 commits into from
Feb 15, 2022
Merged

Dokka and coroutine version bump #514

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
9b4888a
WIP
kggilmer Jan 27, 2022
58af350
Merge branch 'main' into refactor-dokka-1405
kggilmer Jan 27, 2022
139bbc3
WIP
kggilmer Jan 28, 2022
7782147
Updates for coroutine test 1.6
kggilmer Jan 31, 2022
f015f11
Updates for coroutine test 1.6
kggilmer Feb 1, 2022
17eda19
experimental fix
kggilmer Feb 1, 2022
9642911
Fix tests where simple to do so, ignore others with presumably bigger…
kggilmer Feb 1, 2022
fc7d734
Refactor to use Duration type over Int to model time
kggilmer Feb 1, 2022
39d9fe0
Misc fixes and disable tests broken by coroutines-test 1.6
kggilmer Feb 1, 2022
68b269d
lint
kggilmer Feb 1, 2022
4287180
Re-enable tests after switching to runBlocking as advised in PR
kggilmer Feb 3, 2022
6f35e85
lint
kggilmer Feb 3, 2022
f49680d
Merge branch 'main' into chore-coroutine-version-bump
kggilmer Feb 3, 2022
6d107e5
Fix warnings found by CI
kggilmer Feb 3, 2022
97f13e9
Fix warnings
kggilmer Feb 3, 2022
981e85e
Disable failing test on windows
kggilmer Feb 3, 2022
875696a
Fix test failures
kggilmer Feb 3, 2022
f58e627
lint fix
kggilmer Feb 3, 2022
9ce45d6
Add note regarding test regression
kggilmer Feb 4, 2022
b13cfaf
remove ignore windows from async stress test
aajtodd Feb 15, 2022
dd1d144
Merge remote-tracking branch 'origin/main' into chore-coroutine-versi…
aajtodd Feb 15, 2022
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: 1 addition & 0 deletions aws-runtime/aws-config/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ kotlin {
dependencies {
api(project(":aws-runtime:aws-core"))
api(project(":aws-runtime:aws-types"))

implementation("aws.smithy.kotlin:logging:$smithyKotlinVersion")
implementation("aws.smithy.kotlin:http:$smithyKotlinVersion")
implementation("aws.smithy.kotlin:utils:$smithyKotlinVersion")
Expand Down
2 changes: 2 additions & 0 deletions ...nfig/common/test/aws/sdk/kotlin/runtime/auth/credentials/StaticCredentialsProviderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@

package aws.sdk.kotlin.runtime.auth.credentials

import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import kotlin.test.Test
import kotlin.test.assertEquals

@OptIn(ExperimentalCoroutinesApi::class)
class StaticCredentialsProviderTest {
@Test
fun testStaticProvider() = runTest {
Expand Down
2 changes: 2 additions & 0 deletions ...runtime/aws-config/common/test/aws/sdk/kotlin/runtime/config/AwsClientConfigLoaderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ package aws.sdk.kotlin.runtime.config

import aws.sdk.kotlin.runtime.auth.credentials.StaticCredentialsProvider
import aws.sdk.kotlin.runtime.testing.TestPlatformProvider
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import kotlin.test.Test
import kotlin.test.assertEquals

@OptIn(ExperimentalCoroutinesApi::class)
class AwsClientConfigLoaderTest {
@Test
fun testExplicit() = runTest {
Expand Down
4 changes: 2 additions & 2 deletions aws-runtime/aws-config/common/test/aws/sdk/kotlin/runtime/config/imds/ImdsClientTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import aws.smithy.kotlin.runtime.time.ManualClock
import aws.smithy.kotlin.runtime.time.epochMilliseconds
import io.kotest.matchers.string.shouldContain
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.withTimeout
import kotlinx.serialization.json.*
Expand Down Expand Up @@ -212,9 +213,8 @@ class ImdsClientTest {
}

@IgnoreWindows("DNS fails faster on windows and results in a different error")
@Ignore // FIXME: Test elapsed assertions fail after kotlinx-coroutines-test 1.6.0 upgrade
@Test
fun testHttpConnectTimeouts() = runTest() {
fun testHttpConnectTimeouts() = runBlocking {
// end-to-end real client times out after 1-second
val client = ImdsClient {
// will never resolve
Expand Down
2 changes: 2 additions & 0 deletions ...ntime/aws-config/common/test/aws/sdk/kotlin/runtime/config/profile/AwsConfigParserTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ import io.mockk.coEvery
import io.mockk.every
import io.mockk.mockk
import io.mockk.slot
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import kotlinx.serialization.json.*
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFails

@OptIn(ExperimentalCoroutinesApi::class)
class AwsProfileParserTest {

@Test
Expand Down
2 changes: 2 additions & 0 deletions ...ime/aws-config/common/test/aws/sdk/kotlin/runtime/region/EnvironmentRegionProviderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@

package aws.sdk.kotlin.runtime.region

import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNull

@OptIn(ExperimentalCoroutinesApi::class)
class EnvironmentRegionProviderTest {

@Test
Expand Down
2 changes: 2 additions & 0 deletions ...time/aws-config/jvm/test/aws/sdk/kotlin/runtime/region/JvmSystemPropRegionProviderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
package aws.sdk.kotlin.runtime.region

import aws.sdk.kotlin.runtime.testing.TestPlatformProvider
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNull

@OptIn(ExperimentalCoroutinesApi::class)
class JvmSystemPropRegionProviderTest {

@Test
Expand Down
11 changes: 5 additions & 6 deletions ...http-client-engine-crt/jvm/test/aws/sdk/kotlin/runtime/http/engine/crt/AsyncStressTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ import aws.smithy.kotlin.runtime.http.request.url
import aws.smithy.kotlin.runtime.http.response.complete
import aws.smithy.kotlin.runtime.http.sdkHttpClient
import aws.smithy.kotlin.runtime.httptest.TestWithLocalServer
import aws.smithy.kotlin.runtime.testing.IgnoreWindows
import io.ktor.application.*
import io.ktor.response.*
import io.ktor.routing.*
import io.ktor.server.cio.*
import io.ktor.server.engine.*
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeout
import kotlinx.coroutines.yield
import kotlin.test.Ignore
import kotlin.test.Test
import kotlin.time.Duration.Companion.seconds

Expand All @@ -42,8 +42,7 @@ class AsyncStressTest : TestWithLocalServer() {
}

@Test
@Ignore // FIXME: Test fails after kotlinx-coroutines-test 1.6.0 upgrade
fun testConcurrentRequests() = runTest {
fun testConcurrentRequests() = runBlocking {
// https://github.com/awslabs/aws-sdk-kotlin/issues/170
val client = sdkHttpClient(CrtHttpEngine())
val request = HttpRequestBuilder().apply {
Expand Down Expand Up @@ -73,9 +72,9 @@ class AsyncStressTest : TestWithLocalServer() {
}
}

@IgnoreWindows
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Can we please add a reason to this annotation?

Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the reason for this now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the test run in which the regression occurs on Windows: https://github.com/awslabs/aws-sdk-kotlin/runs/5057351434?check_suite_focus=true

Here is the error snippet from the log:

AsyncStressTest[jvm] > testStreamNotConsumed()[jvm] STANDARD_ERROR
    [DefaultDispatcher-worker-1 @coroutine#2078] INFO ktor.application - Autoreload is disabled because the development mode is off.
    [DefaultDispatcher-worker-1 @coroutine#2078] INFO ktor.application - Responding at http://0.0.0.0:51019/
    [DefaultDispatcher-worker-1 @coroutine#2078] INFO ktor.application - Application started in 0.013 seconds.
    [Test worker @coroutine#2079] INFO aws.smithy.kotlin.runtime.httptest.TestWithLocalServer - test server listening on: localhost:51019

AsyncStressTest[jvm] > testStreamNotConsumed()[jvm] STANDARD_OUT
    exception on 973: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 974: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 975: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 976: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 977: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 982: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 981: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 985: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 987: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 978: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 979: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 983: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 986: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 980: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 984: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 988: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 989: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 990: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 991: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 992: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 993: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 994: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 995: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 996: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 997: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 998: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms
    exception on 999: java.util.concurrent.CancellationException: Timed out waiting for 5000 ms

AsyncStressTest[jvm] > testStreamNotConsumed()[jvm] STANDARD_ERROR
    [Test worker] INFO aws.smithy.kotlin.runtime.httptest.TestWithLocalServer - test server stopped

AsyncStressTest[jvm] > testStreamNotConsumed()[jvm] FAILED
    kotlinx.coroutines.TimeoutCancellationException at �:-1
        Caused by: kotlinx.coroutines.TimeoutCancellationException at Timeout.kt:184

Copy link
Collaborator

@aajtodd aajtodd Feb 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

OK IDK if we can merge this without looking into this further to ensure we aren't introducing a regression on windows (i.e. is this an issue with the test surfaced by the upgrade OR a latent issue with the implementation discovered in this test by the upgrade)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly related:

@Test
@Ignore // FIXME: Test fails after kotlinx-coroutines-test 1.6.0 upgrade
fun testStreamNotConsumed() = runTest {
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
Expand Down
2 changes: 2 additions & 0 deletions ...rotocols/common/test/aws/sdk/kotlin/runtime/protocol/xml/Ec2QueryErrorDeserializerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
package aws.sdk.kotlin.runtime.protocol.xml

import aws.smithy.kotlin.runtime.serde.DeserializationException
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertNull

@OptIn(ExperimentalCoroutinesApi::class)
class Ec2QueryErrorDeserializerTest {
@Test
fun `it deserializes ec2Query errors`() = runTest {
Expand Down
10 changes: 9 additions & 1 deletion services/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ val sdkVersion: String by project

val optinAnnotations = listOf(
"aws.smithy.kotlin.runtime.util.InternalApi",
"aws.sdk.kotlin.runtime.InternalSdkApi"
"aws.sdk.kotlin.runtime.InternalSdkApi",
)

subprojects {
Expand Down Expand Up @@ -67,6 +67,9 @@ subprojects {
kotlinOptions {
jvmTarget = "1.8" // this is the default but it's better to be explicit (e.g. it may change in Kotlin 1.5)
allWarningsAsErrors = false // FIXME Tons of errors occur in generated code
// Enable coroutine runTests in 1.6.10
// NOTE: may be removed after coroutines-test runTests becomes stable
freeCompilerArgs = freeCompilerArgs + "-opt-in=kotlin.RequiresOptIn"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: freeCompilerArgs += "-opt-in=kotlin.RequiresOptIn"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had it that way originally but the IDE warned me of the creation of a list under the hood and recommended this form instead.

}
}

Expand Down Expand Up @@ -109,6 +112,11 @@ subprojects {
implementation(project(":aws-runtime:testing"))
}
}
kotlinOptions {
// Enable coroutine runTests in 1.6.10
// NOTE: may be removed after coroutines-test runTests becomes stable
freeCompilerArgs = freeCompilerArgs + "-opt-in=kotlin.RequiresOptIn"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: freeCompilerArgs += "-opt-in=kotlin.RequiresOptIn"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had it that way originally but the IDE warned me of the creation of a list under the hood and recommended this form instead.

}

tasks.register<Test>("e2eTest") {
description = "Run e2e service tests"
Expand Down
4 changes: 3 additions & 1 deletion services/glacier/common/test/aws/sdk/kotlin/services/glacier/internal/TreeHasherTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,18 @@ import aws.smithy.kotlin.runtime.io.SdkByteReadChannel
import aws.smithy.kotlin.runtime.util.HashFunction
import aws.smithy.kotlin.runtime.util.Sha256
import aws.smithy.kotlin.runtime.util.encodeToHex
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.withTimeout
import kotlin.test.Test
import kotlin.test.assertContentEquals
import kotlin.test.assertEquals
import kotlin.test.fail
import kotlin.test.runTest

private const val megabyte = 1024 * 1024

@OptIn(ExperimentalCoroutinesApi::class)
class TreeHasherTest {
@Test
fun testCalculateHashes() = runTest {
Expand Down
7 changes: 2 additions & 5 deletions services/polly/e2eTest/PollyPresignerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ import aws.sdk.kotlin.services.polly.model.VoiceId
import aws.sdk.kotlin.services.polly.presigners.presign
import aws.smithy.kotlin.runtime.http.response.complete
import aws.smithy.kotlin.runtime.http.sdkHttpClient
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.runBlocking
import org.junit.jupiter.api.TestInstance
import kotlin.test.Ignore
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.time.Duration.Companion.seconds
Expand All @@ -22,8 +20,7 @@ import kotlin.time.Duration.Companion.seconds
class PollyPresignerTest {

@Test
@Ignore // FIXME: CRT HTTP client fails to get connection after kotlinx-coroutines-test 1.6.0 upgrade
fun clientBasedPresign() = runTest(context = StandardTestDispatcher()) {
fun clientBasedPresign() = runBlocking {
val request = SynthesizeSpeechRequest {
voiceId = VoiceId.Salli
outputFormat = OutputFormat.Pcm
Expand Down
11 changes: 3 additions & 8 deletions services/s3/e2eTest/S3IntegrationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,10 @@ import aws.smithy.kotlin.runtime.content.fromFile
import aws.smithy.kotlin.runtime.testing.RandomTempFile
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.withTimeout
import org.junit.jupiter.api.AfterAll
import org.junit.jupiter.api.BeforeAll
import org.junit.jupiter.api.TestInstance
import kotlin.test.Ignore
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.time.Duration.Companion.seconds
Expand Down Expand Up @@ -50,8 +48,7 @@ class S3BucketOpsIntegrationTest {
}

@Test
@Ignore // FIXME: CRT HTTP client fails to get connection after kotlinx-coroutines-test 1.6.0 upgrade
fun testPutObjectFromMemory() = runTest {
fun testPutObjectFromMemory(): Unit = runBlocking {
val contents = """
A lep is a ball.
A tay is a hammer.
Expand All @@ -78,8 +75,7 @@ class S3BucketOpsIntegrationTest {
}

@Test
@Ignore // FIXME: CRT HTTP client fails to get connection after kotlinx-coroutines-test 1.6.0 upgrade
fun testPutObjectFromFile() = runTest {
fun testPutObjectFromFile(): Unit = runBlocking {
val tempFile = RandomTempFile(1024)
val keyName = "put-obj-from-file.txt"

Expand All @@ -104,8 +100,7 @@ class S3BucketOpsIntegrationTest {
}

@Test
@Ignore // FIXME: CRT HTTP client fails to get connection after kotlinx-coroutines-test 1.6.0 upgrade
fun testListObjectsWithDelimiter() = runTest {
fun testListObjectsWithDelimiter(): Unit = runBlocking {
// see: https://github.com/awslabs/aws-sdk-kotlin/issues/448

client.listObjects {
Expand Down
6 changes: 2 additions & 4 deletions services/sts/e2eTest/STSPresignerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ import aws.sdk.kotlin.services.sts.presigners.presign
import aws.smithy.kotlin.runtime.http.response.complete
import aws.smithy.kotlin.runtime.http.sdkHttpClient
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.runBlocking
import org.junit.jupiter.api.TestInstance
import kotlin.test.Ignore
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.time.Duration.Companion.seconds
Expand All @@ -21,8 +20,7 @@ import kotlin.time.Duration.Companion.seconds
class StsPresignerTest {

@Test
@Ignore // FIXME: CRT HTTP client fails to get connection after kotlinx-coroutines-test 1.6.0 upgrade
fun testGetCallerIdentityPresigner() = runTest {
fun testGetCallerIdentityPresigner() = runBlocking {
val c = StsClient { region = "us-east-2" }
val req = GetCallerIdentityRequest { }
val presignedRequest = req.presign(c.config, 60.seconds)
Expand Down