diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/IntegrationTestsHelper.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/IntegrationTestsHelper.java index 7d3e657c03..a2becabe34 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/IntegrationTestsHelper.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/IntegrationTestsHelper.java @@ -21,7 +21,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import java.lang.annotation.Annotation; -import java.lang.reflect.AnnotatedElement; import java.net.URI; import java.nio.file.Path; import java.util.Arrays; @@ -76,9 +75,8 @@ public static T extractFromAnnotatedElements( TestInfo testInfo, Class annotationClass, Function extractor, T defaultValue) { return testInfo .getTestMethod() - .map(AnnotatedElement.class::cast) - .or(testInfo::getTestClass) - .map(clz -> clz.getAnnotation(annotationClass)) + .map(m -> m.getAnnotation(annotationClass)) + .or(() -> testInfo.getTestClass().map(c -> c.getAnnotation(annotationClass))) .map(extractor) .orElse(defaultValue); } @@ -109,7 +107,7 @@ public static Map mergeFromAnnotatedEleme .map(propertiesExtractor) .orElse(new String[0]); String[] properties = - Stream.concat(Arrays.stream(methodProperties), Arrays.stream(classProperties)) + Stream.concat(Arrays.stream(classProperties), Arrays.stream(methodProperties)) .toArray(String[]::new); if (properties.length % 2 != 0) { throw new IllegalArgumentException( diff --git a/integration-tests/src/test/java/org/apache/polaris/service/it/env/IntegrationTestsHelperTest.java b/integration-tests/src/test/java/org/apache/polaris/service/it/env/IntegrationTestsHelperTest.java index ad65f7598d..63f75fd912 100644 --- a/integration-tests/src/test/java/org/apache/polaris/service/it/env/IntegrationTestsHelperTest.java +++ b/integration-tests/src/test/java/org/apache/polaris/service/it/env/IntegrationTestsHelperTest.java @@ -19,10 +19,22 @@ package org.apache.polaris.service.it.env; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.lang.reflect.Method; import java.net.URI; import java.nio.file.Path; +import java.util.Map; +import java.util.Optional; import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -65,4 +77,144 @@ static Stream getTemporaryDirectory() { Path.of("/tmp/polaris/default"), URI.create("s3://bucket/polaris/from-env/"))); } + + @Retention(RetentionPolicy.RUNTIME) + @Target({ElementType.METHOD, ElementType.TYPE}) + @interface TestAnnotation { + String value() default ""; + + String[] properties() default {}; + } + + @TestAnnotation( + value = "classValue", + properties = {"sharedKey", "classValue", "classKey", "classValue"}) + static class AnnotatedClass { + @TestAnnotation( + value = "methodValue", + properties = {"sharedKey", "methodValue", "methodKey", "methodValue"}) + public void annotatedMethod() {} + } + + static class UnannotatedClass { + public void unannotatedMethod() {} + } + + @ParameterizedTest + @MethodSource + void extractFromAnnotatedElements(Method testMethod, Class testClass, String expected) { + TestInfo testInfo = mock(TestInfo.class); + when(testInfo.getTestMethod()).thenReturn(Optional.ofNullable(testMethod)); + when(testInfo.getTestClass()).thenReturn(Optional.ofNullable(testClass)); + + String result = + IntegrationTestsHelper.extractFromAnnotatedElements( + testInfo, TestAnnotation.class, TestAnnotation::value, "defaultValue"); + + assertThat(result).isEqualTo(expected); + } + + static Stream extractFromAnnotatedElements() throws Exception { + Method annotatedMethod = AnnotatedClass.class.getMethod("annotatedMethod"); + Method unannotatedMethod = UnannotatedClass.class.getMethod("unannotatedMethod"); + + return Stream.of( + // Method has annotation - use method value + Arguments.of(annotatedMethod, AnnotatedClass.class, "methodValue"), + // Method has no annotation, class has annotation - fall back to class value + Arguments.of(unannotatedMethod, AnnotatedClass.class, "classValue"), + // No method, class has annotation - use class value + Arguments.of(null, AnnotatedClass.class, "classValue"), + // Method has no annotation, class has no annotation - use default + Arguments.of(unannotatedMethod, UnannotatedClass.class, "defaultValue"), + // No method, no class - use default + Arguments.of(null, null, "defaultValue"), + // No method, class has no annotation - use default + Arguments.of(null, UnannotatedClass.class, "defaultValue")); + } + + @ParameterizedTest + @MethodSource + void mergeFromAnnotatedElements( + Method testMethod, + Class testClass, + Map defaults, + Map expected) { + TestInfo testInfo = mock(TestInfo.class); + when(testInfo.getTestMethod()).thenReturn(Optional.ofNullable(testMethod)); + when(testInfo.getTestClass()).thenReturn(Optional.ofNullable(testClass)); + + Map result = + IntegrationTestsHelper.mergeFromAnnotatedElements( + testInfo, TestAnnotation.class, TestAnnotation::properties, defaults); + + assertThat(result).isEqualTo(expected); + } + + static Stream mergeFromAnnotatedElements() throws Exception { + Method annotatedMethod = AnnotatedClass.class.getMethod("annotatedMethod"); + Method unannotatedMethod = UnannotatedClass.class.getMethod("unannotatedMethod"); + + return Stream.of( + // Both method and class have properties - merge with method overriding shared key + Arguments.of( + annotatedMethod, + AnnotatedClass.class, + Map.of(), + Map.of( + "sharedKey", "methodValue", "classKey", "classValue", "methodKey", "methodValue")), + // Only class has properties + Arguments.of( + unannotatedMethod, + AnnotatedClass.class, + Map.of(), + Map.of("sharedKey", "classValue", "classKey", "classValue")), + // No method, class has properties + Arguments.of( + null, + AnnotatedClass.class, + Map.of(), + Map.of("sharedKey", "classValue", "classKey", "classValue")), + // Neither has properties - return defaults + Arguments.of( + unannotatedMethod, + UnannotatedClass.class, + Map.of("defaultKey", "defaultValue"), + Map.of("defaultKey", "defaultValue")), + // No method, no class - return defaults + Arguments.of(null, null, Map.of("key", "value"), Map.of("key", "value")), + // Defaults are overridden by class properties + Arguments.of( + null, + AnnotatedClass.class, + Map.of("sharedKey", "overridden"), + Map.of("sharedKey", "classValue", "classKey", "classValue")), + // Defaults are overridden by method properties + Arguments.of( + annotatedMethod, + AnnotatedClass.class, + Map.of("methodKey", "overridden"), + Map.of( + "sharedKey", "methodValue", "classKey", "classValue", "methodKey", "methodValue"))); + } + + @TestAnnotation(properties = {"key1", "value1", "key2"}) + static class OddPropertiesClass { + public void method() {} + } + + @Test + void mergeFromAnnotatedElementsThrowsOnOddNumberOfProperties() throws Exception { + Method method = OddPropertiesClass.class.getMethod("method"); + TestInfo testInfo = mock(TestInfo.class); + when(testInfo.getTestMethod()).thenReturn(Optional.of(method)); + when(testInfo.getTestClass()).thenReturn(Optional.of(OddPropertiesClass.class)); + + assertThatThrownBy( + () -> + IntegrationTestsHelper.mergeFromAnnotatedElements( + testInfo, TestAnnotation.class, TestAnnotation::properties, Map.of())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Properties must be in key-value pairs"); + } }