Simplify @testfactory tests to @ParameterizedTest

Convert tests that used @testfactory + DynamicTest + nested TestCase
classes into standard @ParameterizedTest + @MethodSource. The cases
were all known at compile time with the same body per case, so the
factory machinery wasn't earning its complexity.

Also reshape the parseCoreResource/parseRegularResource cases in
KubernetesRequestUtilsTest to use named KubernetesResource locals in
the Stream<Arguments> provider instead of positional 7-argument
tuples.

OverheadTests is intentionally left as @testfactory since its cases
are runtime-discovered via Configs.all().

Author: trask

instrumentation/kubernetes-client-7.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/kubernetesclient/v7_0/KubernetesRequestUtilsTest.java

@@ -6,6 +6,7 @@
 package io.opentelemetry.javaagent.instrumentation.kubernetesclient.v7_0;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
@@ -38,46 +39,27 @@ void isResourceRequest() {
         .isTrue();
   }
 
-  @ParameterizedTest
+  @ParameterizedTest(name = "{0}")
   @MethodSource("parseCoreResourceArguments")
-  void parseCoreResource(
-      String urlPath,
-      String apiGroup,
-      String apiVersion,
-      String resource,
-      String subResource,
-      String namespace,
-      String name)
+  void parseCoreResource(String name, String urlPath, KubernetesResource expected)
       throws ParseKubernetesResourceException {
-    assertThat(KubernetesResource.parseCoreResource(urlPath).getApiGroup()).isEqualTo(apiGroup);
-    assertThat(KubernetesResource.parseCoreResource(urlPath).getApiVersion()).isEqualTo(apiVersion);
-    assertThat(KubernetesResource.parseCoreResource(urlPath).getResource()).isEqualTo(resource);
-    assertThat(KubernetesResource.parseCoreResource(urlPath).getSubResource())
-        .isEqualTo(subResource);
-    assertThat(KubernetesResource.parseCoreResource(urlPath).getNamespace()).isEqualTo(namespace);
-    assertThat(KubernetesResource.parseCoreResource(urlPath).getName()).isEqualTo(name);
+    assertResourceEquals(KubernetesResource.parseCoreResource(urlPath), expected);
   }
 
-  @ParameterizedTest
+  @ParameterizedTest(name = "{0}")
   @MethodSource("parseRegularResourceArguments")
-  void parseRegularResource(
-      String urlPath,
-      String apiGroup,
-      String apiVersion,
-      String resource,
-      String subResource,
-      String namespace,
-      String name)
+  void parseRegularResource(String name, String urlPath, KubernetesResource expected)
       throws ParseKubernetesResourceException {
-    assertThat(KubernetesResource.parseRegularResource(urlPath).getApiGroup()).isEqualTo(apiGroup);
-    assertThat(KubernetesResource.parseRegularResource(urlPath).getApiVersion())
-        .isEqualTo(apiVersion);
-    assertThat(KubernetesResource.parseRegularResource(urlPath).getResource()).isEqualTo(resource);
-    assertThat(KubernetesResource.parseRegularResource(urlPath).getSubResource())
-        .isEqualTo(subResource);
-    assertThat(KubernetesResource.parseRegularResource(urlPath).getNamespace())
-        .isEqualTo(namespace);
-    assertThat(KubernetesResource.parseRegularResource(urlPath).getName()).isEqualTo(name);
+    assertResourceEquals(KubernetesResource.parseRegularResource(urlPath), expected);
+  }
+
+  private static void assertResourceEquals(KubernetesResource actual, KubernetesResource expected) {
+    assertThat(actual.getApiGroup()).isEqualTo(expected.getApiGroup());
+    assertThat(actual.getApiVersion()).isEqualTo(expected.getApiVersion());
+    assertThat(actual.getResource()).isEqualTo(expected.getResource());
+    assertThat(actual.getSubResource()).isEqualTo(expected.getSubResource());
+    assertThat(actual.getNamespace()).isEqualTo(expected.getNamespace());
+    assertThat(actual.getName()).isEqualTo(expected.getName());
   }
 
   @ParameterizedTest
@@ -104,73 +86,59 @@ private static Stream<Arguments> k8sRequestVerbsArguments() {
   }
 
   private static Stream<Arguments> parseRegularResourceArguments() {
+    KubernetesResource deploymentsList =
+        new KubernetesResource("apps", "v1", "deployments", null, null, null);
+    KubernetesResource namespacedDeployments =
+        new KubernetesResource("apps", "v1", "deployments", null, "default", null);
+    KubernetesResource namedDeployment =
+        new KubernetesResource("apps", "v1", "deployments", null, "default", "foo");
+    KubernetesResource namedDeploymentStatus =
+        new KubernetesResource("apps", "v1", "deployments", "status", "default", "foo");
+    KubernetesResource foosList =
+        new KubernetesResource("example.io", "v1alpha1", "foos", null, null, null);
+    KubernetesResource namespacedFoos =
+        new KubernetesResource("example.io", "v1alpha1", "foos", null, "default", null);
+    KubernetesResource namedFoo =
+        new KubernetesResource("example.io", "v1alpha1", "foos", null, "default", "foo");
+    KubernetesResource namedFooStatus =
+        new KubernetesResource("example.io", "v1alpha1", "foos", "status", "default", "foo");
     return Stream.of(
-        Arguments.of("/apis/apps/v1/deployments", "apps", "v1", "deployments", null, null, null),
-        Arguments.of(
-            "/apis/apps/v1/namespaces/default/deployments",
-            "apps",
-            "v1",
-            "deployments",
-            null,
-            "default",
-            null),
-        Arguments.of(
-            "/apis/apps/v1/namespaces/default/deployments/foo",
-            "apps",
-            "v1",
-            "deployments",
-            null,
-            "default",
-            "foo"),
-        Arguments.of(
+        arguments("cluster-scoped list", "/apis/apps/v1/deployments", deploymentsList),
+        arguments(
+            "namespaced list", "/apis/apps/v1/namespaces/default/deployments", namespacedDeployments),
+        arguments(
+            "namespaced named", "/apis/apps/v1/namespaces/default/deployments/foo", namedDeployment),
+        arguments(
+            "namespaced named subresource",
             "/apis/apps/v1/namespaces/default/deployments/foo/status",
-            "apps",
-            "v1",
-            "deployments",
-            "status",
-            "default",
-            "foo"),
-        Arguments.of(
-            "/apis/example.io/v1alpha1/foos", "example.io", "v1alpha1", "foos", null, null, null),
-        Arguments.of(
+            namedDeploymentStatus),
+        arguments("custom resource cluster-scoped list", "/apis/example.io/v1alpha1/foos", foosList),
+        arguments(
+            "custom resource namespaced list",
             "/apis/example.io/v1alpha1/namespaces/default/foos",
-            "example.io",
-            "v1alpha1",
-            "foos",
-            null,
-            "default",
-            null),
-        Arguments.of(
+            namespacedFoos),
+        arguments(
+            "custom resource namespaced named",
             "/apis/example.io/v1alpha1/namespaces/default/foos/foo",
-            "example.io",
-            "v1alpha1",
-            "foos",
-            null,
-            "default",
-            "foo"),
-        Arguments.of(
+            namedFoo),
+        arguments(
+            "custom resource namespaced named subresource",
             "/apis/example.io/v1alpha1/namespaces/default/foos/foo/status",
-            "example.io",
-            "v1alpha1",
-            "foos",
-            "status",
-            "default",
-            "foo"));
+            namedFooStatus));
   }
 
   private static Stream<Arguments> parseCoreResourceArguments() {
+    KubernetesResource podsList = new KubernetesResource("", "v1", "pods", null, null, null);
+    KubernetesResource namespacedPods =
+        new KubernetesResource("", "v1", "pods", null, "default", null);
+    KubernetesResource namedPod = new KubernetesResource("", "v1", "pods", null, "default", "foo");
+    KubernetesResource namedPodExec =
+        new KubernetesResource("", "v1", "pods", "exec", "default", "foo");
     return Stream.of(
-        Arguments.of("/api/v1/pods", "", "v1", "pods", null, null, null),
-        Arguments.of("/api/v1/namespaces/default/pods", "", "v1", "pods", null, "default", null),
-        Arguments.of(
-            "/api/v1/namespaces/default/pods/foo", "", "v1", "pods", null, "default", "foo"),
-        Arguments.of(
-            "/api/v1/namespaces/default/pods/foo/exec",
-            "",
-            "v1",
-            "pods",
-            "exec",
-            "default",
-            "foo"));
+        arguments("cluster-scoped list", "/api/v1/pods", podsList),
+        arguments("namespaced list", "/api/v1/namespaces/default/pods", namespacedPods),
+        arguments("namespaced named", "/api/v1/namespaces/default/pods/foo", namedPod),
+        arguments(
+            "namespaced named subresource", "/api/v1/namespaces/default/pods/foo/exec", namedPodExec));
   }
 }

instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/HostIdResourceTest.java

@@ -11,93 +11,63 @@
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
 import static java.util.Collections.singletonMap;
-import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
 import io.opentelemetry.sdk.resources.Resource;
 import java.nio.file.Path;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
 import org.assertj.core.api.MapAssert;
-import org.junit.jupiter.api.DynamicTest;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 class HostIdResourceTest {
 
-  private static class LinuxTestCase {
-    private final String name;
-    private final String expectedValue;
-    private final Function<Path, List<String>> pathReader;
-
-    private LinuxTestCase(
-        String name, String expectedValue, Function<Path, List<String>> pathReader) {
-      this.name = name;
-      this.expectedValue = expectedValue;
-      this.pathReader = pathReader;
-    }
-  }
-
-  private static class WindowsTestCase {
-    private final String name;
-    private final String expectedValue;
-    private final Supplier<List<String>> queryWindowsRegistry;
-
-    private WindowsTestCase(
-        String name, String expectedValue, Supplier<List<String>> queryWindowsRegistry) {
-      this.name = name;
-      this.expectedValue = expectedValue;
-      this.queryWindowsRegistry = queryWindowsRegistry;
-    }
+  @ParameterizedTest(name = "{0}")
+  @MethodSource("createResourceLinuxCases")
+  void createResourceLinux(
+      String name, String expectedValue, Function<Path, List<String>> pathReader) {
+    HostIdResource hostIdResource = new HostIdResource(() -> "linux", pathReader, null);
+    assertHostId(expectedValue, hostIdResource);
   }
 
-  @TestFactory
-  Collection<DynamicTest> createResourceLinux() {
+  private static Stream<Arguments> createResourceLinuxCases() {
     return Stream.of(
-            new LinuxTestCase("default", "test", path -> singletonList("test")),
-            new LinuxTestCase("empty file or error reading", null, path -> emptyList()))
-        .map(
-            testCase ->
-                DynamicTest.dynamicTest(
-                    testCase.name,
-                    () -> {
-                      HostIdResource hostIdResource =
-                          new HostIdResource(() -> "linux", testCase.pathReader, null);
+        arguments("default", "test", (Function<Path, List<String>>) path -> singletonList("test")),
+        arguments(
+            "empty file or error reading",
+            null,
+            (Function<Path, List<String>>) path -> emptyList()));
+  }
 
-                      assertHostId(testCase.expectedValue, hostIdResource);
-                    }))
-        .collect(toList());
+  @ParameterizedTest(name = "{0}")
+  @MethodSource("createResourceWindowsCases")
+  void createResourceWindows(
+      String name, String expectedValue, Supplier<List<String>> queryWindowsRegistry) {
+    HostIdResource hostIdResource =
+        new HostIdResource(() -> "Windows 95", null, queryWindowsRegistry);
+    assertHostId(expectedValue, hostIdResource);
   }
 
-  @TestFactory
-  Collection<DynamicTest> createResourceWindows() {
+  private static Stream<Arguments> createResourceWindowsCases() {
     return Stream.of(
-            new WindowsTestCase(
-                "default",
-                "test",
+        arguments(
+            "default",
+            "test",
+            (Supplier<List<String>>)
                 () ->
                     asList(
                         "HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Cryptography",
                         "    MachineGuid    REG_SZ    test")),
-            new WindowsTestCase("short output", null, Collections::emptyList))
-        .map(
-            testCase ->
-                DynamicTest.dynamicTest(
-                    testCase.name,
-                    () -> {
-                      HostIdResource hostIdResource =
-                          new HostIdResource(
-                              () -> "Windows 95", null, testCase.queryWindowsRegistry);
-
-                      assertHostId(testCase.expectedValue, hostIdResource);
-                    }))
-        .collect(toList());
+        arguments("short output", null, (Supplier<List<String>>) Collections::emptyList));
   }
 
   private static void assertHostId(String expectedValue, HostIdResource hostIdResource) {