Skip to content

Commit 3ea91d6

Browse files
ianbotsfianbotsf
authored
fix: correctly close S3 clients in tests (#1657)
1 parent 9737746 commit 3ea91d6

File tree

7 files changed

+151
-132
lines changed

7 files changed

+151
-132
lines changed

‎services/build.gradle.kts‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55
import aws.sdk.kotlin.gradle.dsl.configurePublishing
6-
import aws.sdk.kotlin.gradle.kmp.*
6+
import aws.sdk.kotlin.gradle.kmp.kotlin
77
import aws.sdk.kotlin.gradle.util.typedProp
88
import org.jetbrains.kotlin.gradle.dsl.JvmTarget
99
import java.time.LocalDateTime
@@ -109,13 +109,6 @@ subprojects {
109109
testClassesDirs = output.classesDirs
110110

111111
useJUnitPlatform()
112-
testLogging {
113-
events("passed", "skipped", "failed")
114-
showStandardStreams = true
115-
showStackTraces = true
116-
showExceptions = true
117-
exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL
118-
}
119112

120113
// model a random input to enable re-running e2e tests back to back without
121114
// up-to-date checks or cache getting in the way
@@ -135,6 +128,16 @@ subprojects {
135128
}
136129
}
137130

131+
tasks.withType<org.gradle.api.tasks.testing.AbstractTestTask>().configureEach {
132+
testLogging {
133+
events("passed", "skipped", "failed")
134+
showStandardStreams = true
135+
showStackTraces = true
136+
showExceptions = true
137+
exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL
138+
}
139+
}
140+
138141
configurePublishing("aws-sdk-kotlin")
139142
publishing {
140143
publications.all {

‎services/s3/common/test/aws/sdk/kotlin/services/s3/CreateClientTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class CreateClientTest {
1818
@Test
1919
fun testMissingRegion() {
2020
// Should _not_ throw an exception since region is optional
21-
S3Client { }
21+
S3Client { }.use { }
2222
}
2323

2424
@Test

‎services/s3/common/test/aws/sdk/kotlin/services/s3/express/DefaultS3ExpressCredentialsProviderTest.kt‎

Lines changed: 87 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ import aws.smithy.kotlin.runtime.operation.ExecutionContext
1616
import aws.smithy.kotlin.runtime.time.ManualClock
1717
import kotlinx.coroutines.*
1818
import kotlinx.coroutines.test.runTest
19-
import kotlin.test.*
19+
import kotlin.test.Test
20+
import kotlin.test.assertEquals
21+
import kotlin.test.assertFalse
2022
import kotlin.time.ComparableTimeMark
2123
import kotlin.time.Duration.Companion.milliseconds
2224
import kotlin.time.Duration.Companion.minutes
@@ -38,15 +40,15 @@ class DefaultS3ExpressCredentialsProviderTest {
3840
expiration = clock.now() + 5.minutes
3941
}
4042

41-
val client = TestS3Client(expectedCredentials)
42-
43-
DefaultS3ExpressCredentialsProvider(timeSource, clock).use { provider ->
44-
val credentials = provider.createSessionCredentials(
45-
S3ExpressCredentialsCacheKey("bucket", DEFAULT_BASE_CREDENTIALS),
46-
client,
47-
)
48-
assertFalse(credentials.isExpired)
49-
assertEquals(timeSource.markNow() + 5.minutes, credentials.expiresAt)
43+
TestS3Client(expectedCredentials).use { client ->
44+
DefaultS3ExpressCredentialsProvider(timeSource, clock).use { provider ->
45+
val credentials = provider.createSessionCredentials(
46+
S3ExpressCredentialsCacheKey("bucket", DEFAULT_BASE_CREDENTIALS),
47+
client,
48+
)
49+
assertFalse(credentials.isExpired)
50+
assertEquals(timeSource.markNow() + 5.minutes, credentials.expiresAt)
51+
}
5052
}
5153
}
5254

@@ -67,16 +69,17 @@ class DefaultS3ExpressCredentialsProviderTest {
6769
expiration = clock.now() + 5.minutes
6870
}
6971

70-
val testClient = TestS3Client(expectedCredentials)
71-
DefaultS3ExpressCredentialsProvider(timeSource, clock, cache, refreshBuffer = 1.minutes).use { provider ->
72-
val attributes = ExecutionContext.build {
73-
this.attributes[S3Attributes.ExpressClient] = testClient
74-
this.attributes[S3Attributes.Bucket] = "bucket"
75-
}
72+
TestS3Client(expectedCredentials).use { testClient ->
73+
DefaultS3ExpressCredentialsProvider(timeSource, clock, cache, refreshBuffer = 1.minutes).use { provider ->
74+
val attributes = ExecutionContext.build {
75+
this.attributes[S3Attributes.ExpressClient] = testClient
76+
this.attributes[S3Attributes.Bucket] = "bucket"
77+
}
7678

77-
provider.resolve(attributes)
79+
provider.resolve(attributes)
80+
}
81+
assertEquals(1, testClient.numCreateSession)
7882
}
79-
assertEquals(1, testClient.numCreateSession)
8083
}
8184

8285
@Test
@@ -96,23 +99,23 @@ class DefaultS3ExpressCredentialsProviderTest {
9699
expiration = clock.now() + 5.minutes
97100
}
98101

99-
val testClient = TestS3Client(expectedCredentials)
100-
101-
val provider = DefaultS3ExpressCredentialsProvider(timeSource, clock, cache, refreshBuffer = 1.minutes)
102+
TestS3Client(expectedCredentials).use { testClient ->
103+
val provider = DefaultS3ExpressCredentialsProvider(timeSource, clock, cache, refreshBuffer = 1.minutes)
102104

103-
val attributes = ExecutionContext.build {
104-
this.attributes[S3Attributes.ExpressClient] = testClient
105-
this.attributes[S3Attributes.Bucket] = "bucket"
106-
}
107-
provider.resolve(attributes)
105+
val attributes = ExecutionContext.build {
106+
this.attributes[S3Attributes.ExpressClient] = testClient
107+
this.attributes[S3Attributes.Bucket] = "bucket"
108+
}
109+
provider.resolve(attributes)
108110

109-
// allow the async refresh to initiate before closing the provider
110-
runBlocking { delay(500.milliseconds) }
111+
// allow the async refresh to initiate before closing the provider
112+
runBlocking { delay(500.milliseconds) }
111113

112-
provider.close()
113-
provider.coroutineContext.job.join()
114+
provider.close()
115+
provider.coroutineContext.job.join()
114116

115-
assertEquals(1, testClient.numCreateSession)
117+
assertEquals(1, testClient.numCreateSession)
118+
}
116119
}
117120

118121
@Test
@@ -132,26 +135,26 @@ class DefaultS3ExpressCredentialsProviderTest {
132135
expiration = clock.now() + 5.minutes
133136
}
134137

135-
val testClient = TestS3Client(expectedCredentials)
136-
137-
val provider = DefaultS3ExpressCredentialsProvider(timeSource, clock, cache, refreshBuffer = 1.minutes)
138+
TestS3Client(expectedCredentials).use { testClient ->
139+
val provider = DefaultS3ExpressCredentialsProvider(timeSource, clock, cache, refreshBuffer = 1.minutes)
138140

139-
val attributes = ExecutionContext.build {
140-
this.attributes[S3Attributes.ExpressClient] = testClient
141-
this.attributes[S3Attributes.Bucket] = "bucket"
142-
}
143-
val calls = (1..5).map {
144-
async { provider.resolve(attributes) }
145-
}
146-
calls.awaitAll()
141+
val attributes = ExecutionContext.build {
142+
this.attributes[S3Attributes.ExpressClient] = testClient
143+
this.attributes[S3Attributes.Bucket] = "bucket"
144+
}
145+
val calls = (1..5).map {
146+
async { provider.resolve(attributes) }
147+
}
148+
calls.awaitAll()
147149

148-
// allow the async refresh to initiate before closing the provider
149-
runBlocking { delay(500.milliseconds) }
150+
// allow the async refresh to initiate before closing the provider
151+
runBlocking { delay(500.milliseconds) }
150152

151-
provider.close()
152-
provider.coroutineContext.job.join()
153+
provider.close()
154+
provider.coroutineContext.job.join()
153155

154-
assertEquals(1, testClient.numCreateSession)
156+
assertEquals(1, testClient.numCreateSession)
157+
}
155158
}
156159

157160
@Test
@@ -174,35 +177,35 @@ class DefaultS3ExpressCredentialsProviderTest {
174177
}
175178

176179
// client should throw an exception when `ExceptionBucket` credentials are fetched, but it should be caught
177-
val testClient = TestS3Client(expectedCredentials, throwExceptionOnBucketNamed = "ExceptionBucket")
178-
179-
val provider = DefaultS3ExpressCredentialsProvider(timeSource, clock, cache, refreshBuffer = 1.minutes)
180-
val attributes = ExecutionContext.build {
181-
this.attributes[S3Attributes.ExpressClient] = testClient
182-
this.attributes[S3Attributes.Bucket] = "ExceptionBucket"
183-
}
184-
provider.resolve(attributes)
180+
TestS3Client(expectedCredentials, throwExceptionOnBucketNamed = "ExceptionBucket").use { testClient ->
181+
val provider = DefaultS3ExpressCredentialsProvider(timeSource, clock, cache, refreshBuffer = 1.minutes)
182+
val attributes = ExecutionContext.build {
183+
this.attributes[S3Attributes.ExpressClient] = testClient
184+
this.attributes[S3Attributes.Bucket] = "ExceptionBucket"
185+
}
186+
provider.resolve(attributes)
185187

186-
withTimeout(5.seconds) {
187-
while (testClient.numCreateSession != 1) {
188-
yield()
188+
withTimeout(5.seconds) {
189+
while (testClient.numCreateSession != 1) {
190+
yield()
191+
}
189192
}
190-
}
191-
assertEquals(1, testClient.numCreateSession)
193+
assertEquals(1, testClient.numCreateSession)
192194

193-
attributes[S3Attributes.Bucket] = "SuccessfulBucket"
194-
provider.resolve(attributes)
195+
attributes[S3Attributes.Bucket] = "SuccessfulBucket"
196+
provider.resolve(attributes)
195197

196-
withTimeout(5.seconds) {
197-
while (testClient.numCreateSession != 2) {
198-
yield()
198+
withTimeout(5.seconds) {
199+
while (testClient.numCreateSession != 2) {
200+
yield()
201+
}
199202
}
200-
}
201203

202-
provider.close()
203-
provider.coroutineContext.job.join()
204+
provider.close()
205+
provider.coroutineContext.job.join()
204206

205-
assertEquals(2, testClient.numCreateSession)
207+
assertEquals(2, testClient.numCreateSession)
208+
}
206209
}
207210

208211
@Test
@@ -224,24 +227,26 @@ class DefaultS3ExpressCredentialsProviderTest {
224227

225228
val provider = DefaultS3ExpressCredentialsProvider(timeSource, clock, cache, refreshBuffer = 1.minutes)
226229

227-
val blockingTestS3Client = object : TestS3Client(expectedCredentials) {
230+
class BlockingTestS3Client : TestS3Client(expectedCredentials) {
228231
override suspend fun createSession(input: CreateSessionRequest): CreateSessionResponse {
229232
delay(10.seconds)
230233
numCreateSession += 1
231234
return CreateSessionResponse { credentials = expectedCredentials }
232235
}
233236
}
234237

235-
val attributes = ExecutionContext.build {
236-
this.attributes[S3Attributes.ExpressClient] = blockingTestS3Client
237-
this.attributes[S3Attributes.Bucket] = "bucket"
238-
}
238+
BlockingTestS3Client().use { blockingTestS3Client ->
239+
val attributes = ExecutionContext.build {
240+
this.attributes[S3Attributes.ExpressClient] = blockingTestS3Client
241+
this.attributes[S3Attributes.Bucket] = "bucket"
242+
}
239243

240-
withTimeout(5.seconds) {
241-
provider.resolve(attributes)
242-
provider.close()
244+
withTimeout(5.seconds) {
245+
provider.resolve(attributes)
246+
provider.close()
247+
}
248+
assertEquals(0, blockingTestS3Client.numCreateSession)
243249
}
244-
assertEquals(0, blockingTestS3Client.numCreateSession)
245250
}
246251

247252
/**
@@ -281,5 +286,9 @@ class DefaultS3ExpressCredentialsProviderTest {
281286
}
282287
return CreateSessionResponse { credentials = expectedCredentials }
283288
}
289+
290+
override fun close() {
291+
client.close()
292+
}
284293
}
285294
}

‎services/s3/common/test/aws/sdk/kotlin/services/s3/internal/ExpiresFieldInterceptorTest.kt‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import aws.smithy.kotlin.runtime.http.HttpBody
1313
import aws.smithy.kotlin.runtime.http.HttpStatusCode
1414
import aws.smithy.kotlin.runtime.http.response.HttpResponse
1515
import aws.smithy.kotlin.runtime.httptest.buildTestConnection
16+
import aws.smithy.kotlin.runtime.io.use
1617
import aws.smithy.kotlin.runtime.time.Instant
1718
import kotlinx.coroutines.test.runTest
1819
import kotlin.test.Test
@@ -41,15 +42,16 @@ class ExpiresFieldInterceptorTest {
4142
append("Expires", "Mon, 1 Apr 2024 00:00:00 +0000")
4243
}.build()
4344

44-
val s3 = newTestClient(headers = expectedHeaders)
45-
s3.getObject(
46-
GetObjectRequest {
47-
bucket = "test"
48-
key = "key"
49-
},
50-
) {
51-
assertEquals(Instant.fromEpochSeconds(1711929600), it.expires)
52-
assertEquals("Mon, 1 Apr 2024 00:00:00 +0000", it.expiresString)
45+
newTestClient(headers = expectedHeaders).use { s3 ->
46+
s3.getObject(
47+
GetObjectRequest {
48+
bucket = "test"
49+
key = "key"
50+
},
51+
) {
52+
assertEquals(Instant.fromEpochSeconds(1711929600), it.expires)
53+
assertEquals("Mon, 1 Apr 2024 00:00:00 +0000", it.expiresString)
54+
}
5355
}
5456
}
5557

@@ -61,15 +63,16 @@ class ExpiresFieldInterceptorTest {
6163
append("Expires", invalidExpiresField)
6264
}.build()
6365

64-
val s3 = newTestClient(headers = expectedHeaders)
65-
s3.getObject(
66-
GetObjectRequest {
67-
bucket = "test"
68-
key = "key"
69-
},
70-
) {
71-
assertNull(it.expires)
72-
assertEquals(invalidExpiresField, it.expiresString)
66+
newTestClient(headers = expectedHeaders).use { s3 ->
67+
s3.getObject(
68+
GetObjectRequest {
69+
bucket = "test"
70+
key = "key"
71+
},
72+
) {
73+
assertNull(it.expires)
74+
assertEquals(invalidExpiresField, it.expiresString)
75+
}
7376
}
7477
}
7578
}

0 commit comments

Comments
 (0)