Skip to content

Commit dc3e3b0

Browse files
robobariorobobario
committed
Ensure only one invocation inside assertion can throw runtime exception
Signed-off-by: Robert Young <robertyoungnz@gmail.com>
1 parent f073a6b commit dc3e3b0

5 files changed

Lines changed: 32 additions & 19 deletions

File tree

‎kroxylicious-integration-test-support/src/test/java/io/kroxylicious/test/tester/KroxyliciousTestersTest.java‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.kafka.common.protocol.ApiKeys;
2929
import org.apache.kafka.common.protocol.ApiMessage;
3030
import org.apache.kafka.common.protocol.Errors;
31+
import org.apache.kafka.common.serialization.Serde;
3132
import org.apache.kafka.common.serialization.Serdes;
3233
import org.apache.kafka.common.serialization.StringDeserializer;
3334
import org.apache.kafka.common.serialization.StringSerializer;
@@ -288,15 +289,18 @@ void testSimpleTestClientReportsConnectionState() {
288289
@Test
289290
void testIllegalToAskForNonExistentVirtualCluster() {
290291
try (var tester = kroxyliciousTester(proxy(kafkaCluster))) {
292+
// these are variables to prevent multiple method invocations within assertThrows
293+
Map<String, Object> emptyMap = Map.of();
294+
Serde<String> stringSerde = Serdes.String();
291295
assertThrows(IllegalArgumentException.class, () -> tester.simpleTestClient("NON_EXIST"));
292296
assertThrows(IllegalArgumentException.class, () -> tester.consumer("NON_EXIST"));
293-
assertThrows(IllegalArgumentException.class, () -> tester.consumer("NON_EXIST", Map.of()));
294-
assertThrows(IllegalArgumentException.class, () -> tester.consumer("NON_EXIST", Serdes.String(), Serdes.String(), Map.of()));
297+
assertThrows(IllegalArgumentException.class, () -> tester.consumer("NON_EXIST", emptyMap));
298+
assertThrows(IllegalArgumentException.class, () -> tester.consumer("NON_EXIST", stringSerde, stringSerde, emptyMap));
295299
assertThrows(IllegalArgumentException.class, () -> tester.producer("NON_EXIST"));
296-
assertThrows(IllegalArgumentException.class, () -> tester.producer("NON_EXIST", Map.of()));
297-
assertThrows(IllegalArgumentException.class, () -> tester.producer("NON_EXIST", Serdes.String(), Serdes.String(), Map.of()));
300+
assertThrows(IllegalArgumentException.class, () -> tester.producer("NON_EXIST", emptyMap));
301+
assertThrows(IllegalArgumentException.class, () -> tester.producer("NON_EXIST", stringSerde, stringSerde, emptyMap));
298302
assertThrows(IllegalArgumentException.class, () -> tester.admin("NON_EXIST"));
299-
assertThrows(IllegalArgumentException.class, () -> tester.admin("NON_EXIST", Map.of()));
303+
assertThrows(IllegalArgumentException.class, () -> tester.admin("NON_EXIST", emptyMap));
300304
}
301305
}
302306

@@ -309,13 +313,16 @@ void testIllegalToAskForDefaultClientsWhenVirtualClustersAmbiguous() {
309313
try (var tester = kroxyliciousTester(proxy)) {
310314
assertThrows(AmbiguousVirtualClusterException.class, tester::simpleTestClient);
311315
assertThrows(AmbiguousVirtualClusterException.class, tester::consumer);
312-
assertThrows(AmbiguousVirtualClusterException.class, () -> tester.consumer(Map.of()));
313-
assertThrows(AmbiguousVirtualClusterException.class, () -> tester.consumer(Serdes.String(), Serdes.String(), Map.of()));
316+
// these are variables to prevent multiple method invocations within assertThrows
317+
Map<String, Object> emptyMap = Map.of();
318+
Serde<String> stringSerde = Serdes.String();
319+
assertThrows(AmbiguousVirtualClusterException.class, () -> tester.consumer(emptyMap));
320+
assertThrows(AmbiguousVirtualClusterException.class, () -> tester.consumer(stringSerde, stringSerde, emptyMap));
314321
assertThrows(AmbiguousVirtualClusterException.class, tester::producer);
315-
assertThrows(AmbiguousVirtualClusterException.class, () -> tester.producer(Map.of()));
316-
assertThrows(AmbiguousVirtualClusterException.class, () -> tester.producer(Serdes.String(), Serdes.String(), Map.of()));
322+
assertThrows(AmbiguousVirtualClusterException.class, () -> tester.producer(emptyMap));
323+
assertThrows(AmbiguousVirtualClusterException.class, () -> tester.producer(stringSerde, stringSerde, emptyMap));
317324
assertThrows(AmbiguousVirtualClusterException.class, tester::admin);
318-
assertThrows(AmbiguousVirtualClusterException.class, () -> tester.admin(Map.of()));
325+
assertThrows(AmbiguousVirtualClusterException.class, () -> tester.admin(emptyMap));
319326
}
320327
}
321328

‎kroxylicious-operator-test-support/src/test/java/io/kroxylicious/kubernetes/operator/assertj/MetadataAssertTest.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.assertj.core.api.Assertions;
1212
import org.junit.jupiter.api.Test;
1313

14+
import io.kroxylicious.kubernetes.api.v1alpha1.KafkaProxyIngress;
1415
import io.kroxylicious.kubernetes.api.v1alpha1.KafkaProxyIngressBuilder;
1516

1617
class MetadataAssertTest {
@@ -45,7 +46,8 @@ void shouldFailWithEmptyAnnotations() {
4546

4647
// When
4748
// Then
48-
Assertions.assertThatThrownBy(() -> MetadataAssert.assertThat(thingWithMetadata).hasAnnotations()).isInstanceOf(AssertionError.class);
49+
MetadataAssert<KafkaProxyIngress> metadataAssert = MetadataAssert.assertThat(thingWithMetadata);
50+
Assertions.assertThatThrownBy(metadataAssert::hasAnnotations).isInstanceOf(AssertionError.class);
4951
}
5052

5153
@Test
@@ -55,7 +57,8 @@ void shouldFailWithNoMetadata() {
5557

5658
// When
5759
// Then
58-
Assertions.assertThatThrownBy(() -> MetadataAssert.assertThat(thingWithoutMetadata).assertHasObjectMeta()).isInstanceOf(AssertionError.class);
60+
MetadataAssert<KafkaProxyIngress> metadataAssert = MetadataAssert.assertThat(thingWithoutMetadata);
61+
Assertions.assertThatThrownBy(metadataAssert::assertHasObjectMeta).isInstanceOf(AssertionError.class);
5962
}
6063

6164
@Test
@@ -66,9 +69,9 @@ void shouldFailWhenAnnotationIsMissing() {
6669

6770
// When
6871
// Then
72+
MetadataAssert<KafkaProxyIngress> metadataAssert = MetadataAssert.assertThat(thingWithoutMetadata);
6973
Assertions.assertThatThrownBy(
70-
() -> MetadataAssert.assertThat(thingWithoutMetadata)
71-
.hasAnnotationSatisfying("MissingAnnotation", value -> Assertions.assertThat(value).isNotNull()))
74+
() -> metadataAssert.hasAnnotationSatisfying("MissingAnnotation", value -> Assertions.assertThat(value).isNotNull()))
7275
.isInstanceOf(AssertionError.class);
7376
}
7477

@@ -80,9 +83,9 @@ void shouldFailWhenAnnotationHasWrongValue() {
8083

8184
// When
8285
// Then
86+
MetadataAssert<KafkaProxyIngress> metadataAssert = MetadataAssert.assertThat(thingWithoutMetadata);
8387
Assertions.assertThatThrownBy(
84-
() -> MetadataAssert.assertThat(thingWithoutMetadata)
85-
.hasAnnotationSatisfying(ANNOTATION_A, value -> Assertions.assertThat(value).isBlank()))
88+
() -> metadataAssert.hasAnnotationSatisfying(ANNOTATION_A, value -> Assertions.assertThat(value).isBlank()))
8689
.isInstanceOf(AssertionError.class);
8790
}
8891

‎kroxylicious-runtime/src/test/java/io/kroxylicious/proxy/internal/MeterRegistriesTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ void testPreventingRegistrationOfMetersWithSameNameButDifferentTags() {
2222
MeterRegistries.preventDifferentTagNameRegistration(registry);
2323
registry.counter("abc", List.of(Tag.of("a", "b")));
2424
registry.counter("abc", List.of(Tag.of("a", "c")));
25+
List<Tag> tags = List.of(Tag.of("c", "d"));
2526
Assertions.assertThrows(IllegalArgumentException.class, () -> {
26-
registry.counter("abc", List.of(Tag.of("c", "d")));
27+
registry.counter("abc", tags);
2728
});
2829
}
2930

‎kroxylicious-runtime/src/test/java/io/kroxylicious/proxy/micrometer/CommonTagsHookTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ class CommonTagsHookTest {
2222

2323
@Test
2424
void testNullConfig() {
25+
CommonTagsHook commonTagsHook = new CommonTagsHook();
2526
assertThatThrownBy(() -> {
26-
new CommonTagsHook().build(null);
27+
commonTagsHook.build(null);
2728
}).isInstanceOf(IllegalArgumentException.class);
2829
}
2930

‎kroxylicious-runtime/src/test/java/io/kroxylicious/proxy/micrometer/StandardBindersHookTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ class StandardBindersHookTest {
3232

3333
@Test
3434
void testNullHookConfigThrows() {
35-
assertThatThrownBy(() -> new StandardBindersHook().build(null)).isInstanceOf(IllegalArgumentException.class);
35+
StandardBindersHook standardBindersHook = new StandardBindersHook();
36+
assertThatThrownBy(() -> standardBindersHook.build(null)).isInstanceOf(IllegalArgumentException.class);
3637
}
3738

3839
@Test

0 commit comments

Comments
 (0)