Skip to content

Commit 69ec88c

Browse files
gaborgsomogyigaborgsomogyi
committed
Test simplification and restructuring
1 parent 19aff30 commit 69ec88c

2 files changed

Lines changed: 125 additions & 81 deletions

File tree

‎flink-filesystems/flink-s3-fs-native/src/test/java/org/apache/flink/fs/s3native/S3EncryptionConfigTest.java‎

Lines changed: 98 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.io.ByteArrayOutputStream;
2929
import java.io.ObjectInputStream;
3030
import java.io.ObjectOutputStream;
31+
import java.util.Base64;
3132
import java.util.Collections;
3233
import java.util.HashMap;
3334
import java.util.Map;
@@ -42,35 +43,6 @@
4243
/** Tests for {@link S3EncryptionConfig}. */
4344
class S3EncryptionConfigTest {
4445

45-
@ParameterizedTest
46-
@MethodSource
47-
void noArgFactories_encryptionTypeCorrect(
48-
S3EncryptionConfig config,
49-
S3EncryptionConfig.EncryptionType expectedType,
50-
boolean expectedEnabled,
51-
ServerSideEncryption expectedSse) {
52-
assertThat(config.getEncryptionType()).isEqualTo(expectedType);
53-
assertThat(config.isEnabled()).isEqualTo(expectedEnabled);
54-
assertThat(config.getKmsKeyId()).isNull();
55-
assertThat(config.getServerSideEncryption()).isEqualTo(expectedSse);
56-
}
57-
58-
static Stream<Arguments> noArgFactories_encryptionTypeCorrect() {
59-
return Stream.of(
60-
Arguments.of(S3EncryptionConfig.none(), NONE, false, null),
61-
Arguments.of(S3EncryptionConfig.sseS3(), SSE_S3, true, ServerSideEncryption.AES256),
62-
Arguments.of(
63-
S3EncryptionConfig.sseKms(), SSE_KMS, true, ServerSideEncryption.AWS_KMS));
64-
}
65-
66-
@Test
67-
void sseKms_withKeyId_keyIdStoredAndEnabled() {
68-
S3EncryptionConfig c = S3EncryptionConfig.sseKms("arn:aws:kms:us-east-1:123:key/abc");
69-
70-
assertThat(c.getKmsKeyId()).isEqualTo("arn:aws:kms:us-east-1:123:key/abc");
71-
assertThat(c.isEnabled()).isTrue();
72-
}
73-
7446
@Test
7547
void sseKms_withContext_contextStoredDefensively() {
7648
Map<String, String> ctx = new HashMap<>(Map.of("dept", "finance"));
@@ -89,6 +61,48 @@ void sseKms_nullContext_contextIsEmpty() {
8961
assertThat(c.hasEncryptionContext()).isFalse();
9062
}
9163

64+
@ParameterizedTest
65+
@MethodSource
66+
void sseKms_contextOnlyFactory_absentContext_hasEncryptionContextFalse(
67+
Map<String, String> context) {
68+
S3EncryptionConfig c = S3EncryptionConfig.sseKms(context);
69+
70+
assertThat(c.getEncryptionContext()).isEmpty();
71+
assertThat(c.hasEncryptionContext()).isFalse();
72+
}
73+
74+
static Stream<Arguments> sseKms_contextOnlyFactory_absentContext_hasEncryptionContextFalse() {
75+
return Stream.of(
76+
Arguments.of(Collections.emptyMap()),
77+
Arguments.of((Object) null));
78+
}
79+
80+
@Test
81+
void sseKms_contextOnlyFactory_contextMutatedAfterCreation_contextUnchanged() {
82+
Map<String, String> ctx = new HashMap<>(Map.of("dept", "finance"));
83+
S3EncryptionConfig c = S3EncryptionConfig.sseKms(ctx);
84+
ctx.put("extra", "value");
85+
86+
assertThat(c.getEncryptionContext()).isEqualTo(Map.of("dept", "finance"));
87+
}
88+
89+
@ParameterizedTest
90+
@MethodSource
91+
void getServerSideEncryption_allTypes_returnsCorrectSseValue(
92+
String configType, ServerSideEncryption expected) {
93+
S3EncryptionConfig c =
94+
S3EncryptionConfig.fromConfig(configType, null, Collections.emptyMap());
95+
96+
assertThat(c.getServerSideEncryption()).isEqualTo(expected);
97+
}
98+
99+
static Stream<Arguments> getServerSideEncryption_allTypes_returnsCorrectSseValue() {
100+
return Stream.of(
101+
Arguments.of(null, null),
102+
Arguments.of("sse-s3", ServerSideEncryption.AES256),
103+
Arguments.of("sse-kms", ServerSideEncryption.AWS_KMS));
104+
}
105+
92106
@ParameterizedTest
93107
@MethodSource
94108
void fromConfig_typeVariants_returnExpectedType(
@@ -105,6 +119,7 @@ static Stream<Arguments> fromConfig_typeVariants_returnExpectedType() {
105119
Arguments.of("", NONE),
106120
Arguments.of("none", NONE),
107121
Arguments.of("NONE", NONE),
122+
Arguments.of(" ", NONE),
108123
Arguments.of("sse-s3", SSE_S3),
109124
Arguments.of("AES256", SSE_S3),
110125
Arguments.of("sse-kms", SSE_KMS),
@@ -141,6 +156,15 @@ void fromConfig_sseKmsDefaultKeyWithContext_contextPreserved() {
141156
.isTrue();
142157
}
143158

159+
@Test
160+
void fromConfig_sseS3WithContext_contextIgnored() {
161+
S3EncryptionConfig c =
162+
S3EncryptionConfig.fromConfig("sse-s3", null, Map.of("dept", "finance"));
163+
164+
assertThat(c.getEncryptionType()).isEqualTo(SSE_S3);
165+
assertThat(c.getEncryptionContext()).isEmpty();
166+
}
167+
144168
@Test
145169
void fromConfig_unknownType_throwsIllegalArgument() {
146170
assertThatThrownBy(
@@ -151,42 +175,67 @@ void fromConfig_unknownType_throwsIllegalArgument() {
151175
.hasMessageContaining("invalid-type");
152176
}
153177

154-
@Test
155-
void toString_noKeyOrContext_containsTypeOnly() {
156-
S3EncryptionConfig c = S3EncryptionConfig.none();
178+
@ParameterizedTest
179+
@MethodSource
180+
void serializeEncryptionContext_exactOutput_correctBase64Json(
181+
Map<String, String> context, String expectedDecoded) {
182+
S3EncryptionConfig c = S3EncryptionConfig.sseKms(context);
183+
String decoded = new String(Base64.getDecoder().decode(c.serializeEncryptionContext()));
157184

158-
assertThat(c.toString()).contains("NONE");
159-
assertThat(c.toString()).doesNotContain("kmsKeyId");
160-
assertThat(c.toString()).doesNotContain("encryptionContext");
185+
assertThat(decoded).isEqualTo(expectedDecoded);
186+
}
187+
188+
static Stream<Arguments> serializeEncryptionContext_exactOutput_correctBase64Json() {
189+
return Stream.of(
190+
Arguments.of(Collections.emptyMap(), "{}"),
191+
Arguments.of(Map.of("k", "v"), "{\"k\":\"v\"}"));
161192
}
162193

163194
@Test
164-
void toString_withKeyId_includesKeyId() {
165-
S3EncryptionConfig c = S3EncryptionConfig.sseKms("my-key");
195+
void serializeEncryptionContext_multipleEntries_allEntriesPresent() {
196+
S3EncryptionConfig c = S3EncryptionConfig.sseKms(Map.of("k1", "v1", "k2", "v2"));
197+
String decoded = new String(Base64.getDecoder().decode(c.serializeEncryptionContext()));
166198

167-
assertThat(c.toString()).contains("my-key");
199+
assertThat(decoded).contains("\"k1\":\"v1\"", "\"k2\":\"v2\"");
168200
}
169201

170-
@Test
171-
void toString_withContext_includesContextKeys() {
172-
S3EncryptionConfig c = S3EncryptionConfig.sseKms("k", Map.of("dept", "finance"));
202+
@ParameterizedTest
203+
@MethodSource
204+
void serializeEncryptionContext_jsonSpecialChars_escapedCorrectly(
205+
String key, String value, String expectedFragment) {
206+
S3EncryptionConfig c = S3EncryptionConfig.sseKms(Map.of(key, value));
207+
String decoded = new String(Base64.getDecoder().decode(c.serializeEncryptionContext()));
173208

174-
assertThat(c.toString()).contains("dept");
209+
assertThat(decoded).contains(expectedFragment);
175210
}
176211

177-
@Test
178-
void serialization_roundTrip_preservesAllFields() throws Exception {
179-
S3EncryptionConfig original = S3EncryptionConfig.sseKms("key-id", Map.of("k", "v"));
212+
static Stream<Arguments> serializeEncryptionContext_jsonSpecialChars_escapedCorrectly() {
213+
return Stream.of(
214+
Arguments.of("k", "val\"ue", "\"k\":\"val\\\"ue\""),
215+
Arguments.of("k", "val\\ue", "\"k\":\"val\\\\ue\""),
216+
Arguments.of("k\"ey", "v", "\"k\\\"ey\":\"v\""));
217+
}
180218

219+
@ParameterizedTest
220+
@MethodSource
221+
void serialization_roundTrip_preservesAllFields(S3EncryptionConfig config) throws Exception {
181222
ByteArrayOutputStream bos = new ByteArrayOutputStream();
182-
new ObjectOutputStream(bos).writeObject(original);
223+
new ObjectOutputStream(bos).writeObject(config);
183224
S3EncryptionConfig copy =
184225
(S3EncryptionConfig)
185226
new ObjectInputStream(new ByteArrayInputStream(bos.toByteArray()))
186227
.readObject();
187228

188-
assertThat(copy.getEncryptionType()).isEqualTo(original.getEncryptionType());
189-
assertThat(copy.getKmsKeyId()).isEqualTo(original.getKmsKeyId());
190-
assertThat(copy.getEncryptionContext()).isEqualTo(original.getEncryptionContext());
229+
assertThat(copy.getEncryptionType()).isEqualTo(config.getEncryptionType());
230+
assertThat(copy.getKmsKeyId()).isEqualTo(config.getKmsKeyId());
231+
assertThat(copy.getEncryptionContext()).isEqualTo(config.getEncryptionContext());
232+
}
233+
234+
static Stream<Arguments> serialization_roundTrip_preservesAllFields() {
235+
return Stream.of(
236+
Arguments.of(S3EncryptionConfig.sseKms("key-id", Map.of("k", "v"))),
237+
Arguments.of(S3EncryptionConfig.none()),
238+
Arguments.of(S3EncryptionConfig.sseS3()),
239+
Arguments.of(S3EncryptionConfig.sseKms()));
191240
}
192241
}

‎flink-filesystems/flink-s3-fs-native/src/test/java/org/apache/flink/fs/s3native/S3ExceptionUtilsTest.java‎

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
package org.apache.flink.fs.s3native;
2020

21-
import org.junit.jupiter.api.Test;
2221
import org.junit.jupiter.params.ParameterizedTest;
2322
import org.junit.jupiter.params.provider.Arguments;
2423
import org.junit.jupiter.params.provider.MethodSource;
@@ -44,7 +43,7 @@ static Stream<Arguments> toIoExceptionCases() {
4443
Arguments.of(
4544
s3Exception(500, "internal error"),
4645
"put object",
47-
"put object (HTTP 500: internal error"));
46+
"put object (HTTP 500: internal error)"));
4847
}
4948

5049
@ParameterizedTest
@@ -57,16 +56,22 @@ void toIOException_contextAndStatusCode_formatsMessageAndPreservesCause(
5756
assertThat(result.getCause()).isSameAs(cause);
5857
}
5958

60-
@Test
61-
void errorMessage_detailsHasMessage_returnsDetailsMessage() {
62-
S3Exception e =
63-
s3Exception(
64-
404,
65-
AwsErrorDetails.builder()
66-
.errorMessage("The specified key does not exist.")
67-
.build());
59+
static Stream<Arguments> errorMessageExactCases() {
60+
return Stream.of(
61+
Arguments.of(
62+
s3Exception(
63+
404,
64+
AwsErrorDetails.builder()
65+
.errorMessage("The specified key does not exist.")
66+
.build()),
67+
"The specified key does not exist."),
68+
Arguments.of(s3ExceptionStatusOnly(500), "Unknown S3 error"));
69+
}
6870

69-
assertThat(S3ExceptionUtils.errorMessage(e)).isEqualTo("The specified key does not exist.");
71+
@ParameterizedTest
72+
@MethodSource("errorMessageExactCases")
73+
void errorMessage_exactReturn_matchesExpected(S3Exception e, String expected) {
74+
assertThat(S3ExceptionUtils.errorMessage(e)).isEqualTo(expected);
7075
}
7176

7277
static Stream<Arguments> errorMessageFallbackCases() {
@@ -82,40 +87,30 @@ static Stream<Arguments> errorMessageFallbackCases() {
8287

8388
@ParameterizedTest
8489
@MethodSource("errorMessageFallbackCases")
85-
void errorMessage_noDetailsMessage_fallsBackToExceptionMessage(
90+
void errorMessage_fallbackToExceptionMessage_containsExpected(
8691
S3Exception e, String expectedMessage) {
8792
assertThat(S3ExceptionUtils.errorMessage(e)).contains(expectedMessage);
8893
}
8994

90-
@Test
91-
void errorMessage_noMessageAvailable_returnsUnknownS3Error() {
92-
S3Exception e = s3ExceptionStatusOnly(500);
93-
94-
assertThat(S3ExceptionUtils.errorMessage(e)).isEqualTo("Unknown S3 error");
95-
}
96-
97-
@Test
98-
void errorCode_detailsHasCode_returnsErrorCode() {
99-
S3Exception e = s3Exception(404, AwsErrorDetails.builder().errorCode("NoSuchKey").build());
100-
101-
assertThat(S3ExceptionUtils.errorCode(e)).isEqualTo("NoSuchKey");
102-
}
103-
104-
static Stream<Arguments> errorCodeUnknownCases() {
95+
static Stream<Arguments> errorCodeAllCases() {
10596
return Stream.of(
106-
Arguments.of(s3Exception(500, "some error")),
97+
Arguments.of(
98+
s3Exception(404, AwsErrorDetails.builder().errorCode("NoSuchKey").build()),
99+
"NoSuchKey"),
100+
Arguments.of(s3Exception(500, "some error"), "Unknown"),
107101
Arguments.of(
108102
s3Exception(
109103
500,
110104
AwsErrorDetails.builder()
111105
.errorMessage("Something went wrong")
112-
.build())));
106+
.build()),
107+
"Unknown"));
113108
}
114109

115110
@ParameterizedTest
116-
@MethodSource("errorCodeUnknownCases")
117-
void errorCode_noCodeAvailable_returnsUnknown(S3Exception e) {
118-
assertThat(S3ExceptionUtils.errorCode(e)).isEqualTo("Unknown");
111+
@MethodSource("errorCodeAllCases")
112+
void errorCode_allCases_returnsExpected(S3Exception e, String expected) {
113+
assertThat(S3ExceptionUtils.errorCode(e)).isEqualTo(expected);
119114
}
120115

121116
private static S3Exception s3Exception(int statusCode, AwsErrorDetails details) {

0 commit comments

Comments
 (0)