refactor test a bit + fix things

Author: SylvainJuge

smoke-tests/src/test/java/com/example/javaagent/smoketest/AppServerProvidersTest.java

@@ -60,15 +60,17 @@ static void afterAll() {
   @Test
   void tomcat() {
     target =
-        startTarget(
-            "tomcat:latest",
-            container ->
-                container
-                    .withExposedPorts(PORT)
-                    .waitingFor(Wait.forListeningPorts(PORT))
-                    .withCopyFileToContainer(
-                        MountableFile.forHostPath(testAppWar),
-                        "/usr/local/tomcat/webapps/app.war"));
+        testTarget("tomcat:latest")
+            .withBaseEnvironmentVariables(true)
+            .customize(
+                container ->
+                    container
+                        .withExposedPorts(PORT)
+                        .waitingFor(Wait.forListeningPorts(PORT))
+                        .withCopyFileToContainer(
+                            MountableFile.forHostPath(testAppWar),
+                            "/usr/local/tomcat/webapps/app.war"))
+            .start();
 
     await()
         .atMost(Duration.ofSeconds(30))

smoke-tests/src/test/java/com/example/javaagent/smoketest/SmokeTest.java

@@ -108,55 +108,79 @@ static void setupSpec() {
     backend.start();
   }
 
-  protected static GenericContainer<?> startTarget(
-      String image, Consumer<GenericContainer<?>> customizeContainer) {
+  public static TestTarget testTarget(String image) {
+    return new TestTarget(image);
+  }
 
-    @SuppressWarnings("resource")
-    GenericContainer<?> target =
-        new GenericContainer<>(image)
-            .withNetwork(network)
-            .withLogConsumer(new Slf4jLogConsumer(logger))
-            .withCopyFileToContainer(MountableFile.forHostPath(AGENT_PATH), JAVAAGENT_JAR_PATH);
+  public static class TestTarget {
+    private final String image;
+    private Consumer<GenericContainer<?>> customizeContainer;
+    private boolean baseEnvVariables;
 
-    StringBuilder jvmArgs = new StringBuilder();
+    public TestTarget(String image) {
+      this.image = image;
+    }
 
-    if (JavaExecutable.isDebugging()) {
-      // when debugging, automatically use verbose debug logging
-      target.withEnv("OTEL_JAVAAGENT_DEBUG", "true");
+    public TestTarget customize(Consumer<GenericContainer<?>> customizeContainer) {
+      this.customizeContainer = customizeContainer;
+      return this;
+    }
 
-      if (JavaExecutable.isListeningDebuggerStarted(TARGET_DEBUG_PORT, "target")) {
-        target = addDockerDebugHost(target);
-        jvmArgs
-            .append(JavaExecutable.jvmDebugArgument("remote-localhost", TARGET_DEBUG_PORT))
-            .append(" ");
-      }
-      // Use very long startup delay when debugging as the remote JVM is likely stopped before the
-      // app has started
-      target.waitingFor(Wait.defaultWaitStrategy().withStartupTimeout(Duration.ofMinutes(10)));
+    public TestTarget withBaseEnvironmentVariables(boolean value) {
+      this.baseEnvVariables = value;
+      return this;
     }
 
-    jvmArgs.append(JavaExecutable.jvmAgentArgument(JAVAAGENT_JAR_PATH));
-    target.withEnv("JAVA_TOOL_OPTIONS", jvmArgs.toString());
+    public GenericContainer<?> start() {
+      @SuppressWarnings("resource")
+      GenericContainer<?> target =
+          new GenericContainer<>(image)
+              .withNetwork(network)
+              .withLogConsumer(new Slf4jLogConsumer(logger))
+              .withCopyFileToContainer(MountableFile.forHostPath(AGENT_PATH), JAVAAGENT_JAR_PATH);
+
+      StringBuilder jvmArgs = new StringBuilder();
+
+      if (JavaExecutable.isDebugging()) {
+        // when debugging, automatically use verbose debug logging
+        target.withEnv("OTEL_JAVAAGENT_DEBUG", "true");
+
+        if (JavaExecutable.isListeningDebuggerStarted(TARGET_DEBUG_PORT, "target")) {
+          target = addDockerDebugHost(target);
+          jvmArgs
+              .append(JavaExecutable.jvmDebugArgument("remote-localhost", TARGET_DEBUG_PORT))
+              .append(" ");
+        }
+        // Use very long startup delay when debugging as the remote JVM is likely stopped before the
+        // app has started
+        target.waitingFor(Wait.defaultWaitStrategy().withStartupTimeout(Duration.ofMinutes(10)));
+      }
 
-    customizeContainer.accept(target);
+      jvmArgs.append(JavaExecutable.jvmAgentArgument(JAVAAGENT_JAR_PATH));
+      target.withEnv("JAVA_TOOL_OPTIONS", jvmArgs.toString());
+
+      if (baseEnvVariables) {
+        target
+            // batch span processor: very small batch size for testing
+            .withEnv("OTEL_BSP_MAX_EXPORT_BATCH", "1")
+            // batch span processor: very short delay for testing
+            .withEnv("OTEL_BSP_SCHEDULE_DELAY", "10")
+            // use grpc endpoint as default is now http/protobuf with agent 2.x
+            .withEnv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc")
+            .withEnv("OTEL_PROPAGATORS", "tracecontext,baggage")
+            .withEnv("OTEL_EXPORTER_OTLP_ENDPOINT", BACKEND_ENDPOINT);
+      }
 
-    Objects.requireNonNull(target).start();
+      if (customizeContainer != null) {
+        customizeContainer.accept(target);
+      }
 
-    startedContainers.add(target);
+      Objects.requireNonNull(target).start();
 
-    return target;
-  }
+      startedContainers.add(target);
 
-  protected static void setBaseEnvironmentVariables(GenericContainer<?> target) {
-    target
-        // batch span processor: very small batch size for testing
-        .withEnv("OTEL_BSP_MAX_EXPORT_BATCH", "1")
-        // batch span processor: very short delay for testing
-        .withEnv("OTEL_BSP_SCHEDULE_DELAY", "10")
-        // use grpc endpoint as default is now http/protobuf with agent 2.x
-        .withEnv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc")
-        .withEnv("OTEL_PROPAGATORS", "tracecontext,baggage")
-        .withEnv("OTEL_EXPORTER_OTLP_ENDPOINT", BACKEND_ENDPOINT);
+      return target;
+    }
   }
 
   private static GenericContainer<?> addDockerDebugHost(GenericContainer<?> target) {

smoke-tests/src/test/java/com/example/javaagent/smoketest/TestAppSmokeTest.java

@@ -37,17 +37,15 @@ public static void startTestApp(Consumer<GenericContainer<?>> customizeContainer
   public static void startTestApp(
       Consumer<GenericContainer<?>> customizeContainer, boolean defaultEnv) {
     target =
-        startTarget(
-            TEST_APP_IMAGE,
-            customizeContainer.andThen(
-                container -> {
-                  if (defaultEnv) {
-                    setBaseEnvironmentVariables(container);
-                  }
-                  container
-                      .withExposedPorts(PORT)
-                      .waitingFor(Wait.forHttp("/health").forPort(PORT));
-                }));
+        testTarget(TEST_APP_IMAGE)
+            .withBaseEnvironmentVariables(defaultEnv)
+            .customize(
+                customizeContainer.andThen(
+                    container ->
+                        container
+                            .withExposedPorts(PORT)
+                            .waitingFor(Wait.forHttp("/health").forPort(PORT))))
+            .start();
   }
 
   protected static String getContainerId() {