IntegrationTestsHelper: fix extract & merge logic

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 <A extends Annotation, T> T extractFromAnnotatedElements(
       TestInfo testInfo, Class<A> annotationClass, Function<A, T> 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 <A extends Annotation> Map<String, String> 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(

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<Arguments> 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<Arguments> 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<String, String> defaults,
+      Map<String, String> expected) {
+    TestInfo testInfo = mock(TestInfo.class);
+    when(testInfo.getTestMethod()).thenReturn(Optional.ofNullable(testMethod));
+    when(testInfo.getTestClass()).thenReturn(Optional.ofNullable(testClass));
+
+    Map<String, String> result =
+        IntegrationTestsHelper.mergeFromAnnotatedElements(
+            testInfo, TestAnnotation.class, TestAnnotation::properties, defaults);
+
+    assertThat(result).isEqualTo(expected);
+  }
+
+  static Stream<Arguments> 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");
+  }
 }