TestNG with always helidon fix

Signed-off-by: Daniel Kec <[email protected]>
helidon-io · Dec 15, 2024 · 7f1ea0f · 7f1ea0f
1 parent 43b8fb0
commit 7f1ea0f
Show file tree

Hide file tree

Showing 14 changed files with 170 additions and 129 deletions.
diff --git a/bom/pom.xml b/bom/pom.xml
@@ -1088,6 +1088,11 @@
                 <version>${helidon.version}</version>
             </dependency>
             <!-- Testing -->
+            <dependency>
+                <groupId>io.helidon.microprofile.testing</groupId>
+                <artifactId>helidon-microprofile-testing-common</artifactId>
+                <version>${helidon.version}</version>
+            </dependency>
             <dependency>
                 <groupId>io.helidon.microprofile.testing</groupId>
                 <artifactId>helidon-microprofile-testing-junit5</artifactId>

diff --git a/.../testing/common/src/main/java/io/helidon/microprofile/testing/common/PinningRecorder.java b/.../testing/common/src/main/java/io/helidon/microprofile/testing/common/PinningRecorder.java
@@ -26,6 +26,10 @@
  */
 public class PinningRecorder implements AutoCloseable {
 
+    /**
+     * Default threshold for considering carrier thread blocking as pinning.
+     */
+    public static final long DEFAULT_THRESHOLD = 20;
     private static final String JFR_EVENT_VIRTUAL_THREAD_PINNED = "jdk.VirtualThreadPinned";
     private final RecordingStream recordingStream = new RecordingStream();
     private volatile PinningException pinningException;

diff --git a/microprofile/testing/common/src/main/java/module-info.java b/microprofile/testing/common/src/main/java/module-info.java
@@ -19,5 +19,6 @@
  */
 module io.helidon.microprofile.testing.common {
     requires jdk.jfr;
-    exports io.helidon.microprofile.testing.common;
+    exports io.helidon.microprofile.testing.common
+            to io.helidon.microprofile.testing.junit5, io.helidon.microprofile.testing.testng;
 }
diff --git a/microprofile/testing/junit5/pom.xml b/microprofile/testing/junit5/pom.xml
@@ -41,7 +41,6 @@
         <dependency>
             <groupId>io.helidon.microprofile.testing</groupId>
             <artifactId>helidon-microprofile-testing-common</artifactId>
-            <version>4.2.0-SNAPSHOT</version>
         </dependency>
         <dependency>
             <groupId>io.helidon.microprofile.cdi</groupId>

diff --git a/...ng/junit5/src/main/java/io/helidon/microprofile/testing/junit5/HelidonJunitExtension.java b/...ng/junit5/src/main/java/io/helidon/microprofile/testing/junit5/HelidonJunitExtension.java
@@ -36,7 +36,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -138,14 +137,12 @@ public void beforeAll(ExtensionContext context) {
         HelidonTest testAnnot = testClass.getAnnotation(HelidonTest.class);
         if (testAnnot != null) {
             resetPerTest = testAnnot.resetPerTest();
+            if (testAnnot.pinningDetection()) {
+                pinningRecorder = new PinningRecorder();
+                pinningRecorder.record(Duration.ofMillis(testAnnot.pinningThreshold()));
+            }
         }
 
-        pinningRecorder = new PinningRecorder();
-        if (testAnnot.pinningDetection()) {
-            pinningRecorder.record(Duration.ofMillis(Optional.ofNullable(testAnnot)
-                                                             .map(HelidonTest::pinningThreshold)
-                                                             .orElse(20L)));
-        }
 
         DisableDiscovery discovery = getAnnotation(testClass, DisableDiscovery.class, metaAnnotations);
         if (discovery != null) {
@@ -438,8 +435,10 @@ public void afterAll(ExtensionContext context) {
         stopContainer();
         releaseConfig();
         callAfterStop();
-        pinningRecorder.close();
-        pinningRecorder = null;
+        if (pinningRecorder != null) {
+            pinningRecorder.close();
+            pinningRecorder = null;
+        }
     }
 
     @Override

diff --git a/microprofile/testing/junit5/src/main/java/module-info.java b/microprofile/testing/junit5/src/main/java/module-info.java
@@ -24,7 +24,6 @@
     requires io.helidon.microprofile.cdi;
     requires jakarta.inject;
     requires org.junit.jupiter.api;
-    requires jdk.jfr;
 
     requires transitive jakarta.cdi;
     requires transitive jakarta.ws.rs;

diff --git a/microprofile/testing/testng/pom.xml b/microprofile/testing/testng/pom.xml
@@ -38,6 +38,10 @@
             <artifactId>helidon-microprofile-server</artifactId>
             <optional>true</optional>
         </dependency>
+        <dependency>
+            <groupId>io.helidon.microprofile.testing</groupId>
+            <artifactId>helidon-microprofile-testing-common</artifactId>
+        </dependency>
         <dependency>
             <groupId>io.helidon.microprofile.cdi</groupId>
             <artifactId>helidon-microprofile-cdi</artifactId>

diff --git a/...file/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/HelidonTest.java b/...file/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/HelidonTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,6 +21,8 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
+import static io.helidon.microprofile.testing.common.PinningRecorder.DEFAULT_THRESHOLD;
+
 /**
  * An annotation making this test class a CDI bean with support for injection.
  * <p>
@@ -48,4 +50,18 @@
      * @return whether to reset container per test method
      */
     boolean resetPerTest() default false;
+
+    /**
+     * Time threshold for carrier thread blocking to be considered as pinning.
+     *
+     * @return threshold in milliseconds, {@code 20} is default
+     */
+    long pinningThreshold() default DEFAULT_THRESHOLD;
+
+    /**
+     * Whether to turn on pinning detection during {@code @HelidonTest}.
+     *
+     * @return true for turning detection on, {@code false} is default
+     */
+    boolean pinningDetection() default true;
 }
diff --git a/...ng/testng/src/main/java/io/helidon/microprofile/testing/testng/HelidonTestNgListener.java b/...ng/testng/src/main/java/io/helidon/microprofile/testing/testng/HelidonTestNgListener.java
@@ -26,6 +26,7 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.net.URL;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -39,6 +40,7 @@
 import io.helidon.config.mp.MpConfigSources;
 import io.helidon.microprofile.server.JaxRsCdiExtension;
 import io.helidon.microprofile.server.ServerCdiExtension;
+import io.helidon.microprofile.testing.common.PinningRecorder;
 
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.context.Dependent;
@@ -59,8 +61,6 @@
 import jakarta.inject.Inject;
 import jakarta.inject.Singleton;
 import jakarta.ws.rs.client.ClientBuilder;
-import jdk.jfr.consumer.RecordedEvent;
-import jdk.jfr.consumer.RecordingStream;
 import org.eclipse.microprofile.config.Config;
 import org.eclipse.microprofile.config.spi.ConfigBuilder;
 import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
@@ -90,27 +90,30 @@ public class HelidonTestNgListener implements IClassListener, ITestListener {
     private List<AddExtension> classLevelExtensions = new ArrayList<>();
     private List<AddBean> classLevelBeans = new ArrayList<>();
     private ConfigMeta classLevelConfigMeta = new ConfigMeta();
-    private RecordingStream recordingStream;
     private boolean classLevelDisableDiscovery = false;
+    private boolean helidonTest;
     private boolean resetPerTest;
-    private boolean pinnedThreadValidation;
 
     private Class<?> testClass;
     private Object testInstance;
     private ConfigProviderResolver configProviderResolver;
     private Config config;
     private SeContainer container;
-    private PinningException pinningException;
+    private PinningRecorder pinningRecorder;
 
     @Override
     public void onBeforeClass(ITestClass iTestClass) {
         testClass = iTestClass.getRealClass();
+        HelidonTest testAnnot = testClass.getAnnotation(HelidonTest.class);
+        if (testAnnot == null) {
+            return;
+        }
+        helidonTest = true;
+        resetPerTest = testAnnot.resetPerTest();
 
         List<Annotation> metaAnnotations = extractMetaAnnotations(testClass);
 
         AddConfig[] configs = getAnnotations(testClass, AddConfig.class, metaAnnotations);
-        pinnedThreadValidation = testClass.getAnnotation(PinnedThreadValidation.class) != null;
-        startRecordingStream();
 
         classLevelConfigMeta.addConfig(configs);
         classLevelConfigMeta.configuration(getAnnotation(testClass, Configuration.class, metaAnnotations));
@@ -123,9 +126,9 @@ public void onBeforeClass(ITestClass iTestClass) {
         AddBean[] beans = getAnnotations(testClass, AddBean.class, metaAnnotations);
         classLevelBeans.addAll(Arrays.asList(beans));
 
-        HelidonTest testAnnot = testClass.getAnnotation(HelidonTest.class);
-        if (testAnnot != null) {
-            resetPerTest = testAnnot.resetPerTest();
+        if (testAnnot.pinningDetection()) {
+            pinningRecorder = new PinningRecorder();
+            pinningRecorder.record(Duration.ofMillis(testAnnot.pinningThreshold()));
         }
 
         DisableDiscovery discovery = getAnnotation(testClass, DisableDiscovery.class, metaAnnotations);
@@ -164,15 +167,25 @@ public void onBeforeClass(ITestClass iTestClass) {
 
     @Override
     public void onAfterClass(ITestClass testClass) {
+        if (!helidonTest) {
+            return;
+        }
+
         if (!resetPerTest) {
             releaseConfig();
             stopContainer();
         }
-        closeRecordingStream();
+        if (pinningRecorder != null) {
+            pinningRecorder.close();
+            pinningRecorder = null;
+        }
     }
 
     @Override
     public void onTestStart(ITestResult result) {
+        if (!helidonTest) {
+            return;
+        }
 
         if (resetPerTest) {
             Method method = result.getMethod().getConstructorOrMethod().getMethod();
@@ -205,6 +218,10 @@ public void onTestStart(ITestResult result) {
 
     @Override
     public void onTestFailure(ITestResult iTestResult) {
+        if (!helidonTest) {
+            return;
+        }
+
         if (resetPerTest) {
             releaseConfig();
             stopContainer();
@@ -213,6 +230,10 @@ public void onTestFailure(ITestResult iTestResult) {
 
     @Override
     public void onTestSuccess(ITestResult iTestResult) {
+        if (!helidonTest) {
+            return;
+        }
+
         if (resetPerTest) {
             releaseConfig();
             stopContainer();
@@ -366,30 +387,6 @@ private <T extends Annotation> T getAnnotation(Class<?> testClass, Class<T> anno
         return annotation;
     }
 
-    private void startRecordingStream() {
-        if (pinnedThreadValidation) {
-            pinningException = null;
-            recordingStream = new RecordingStream();
-            recordingStream.enable("jdk.VirtualThreadPinned").withStackTrace();
-            recordingStream.onEvent("jdk.VirtualThreadPinned", this::record);
-            recordingStream.startAsync();
-        }
-    }
-
-    private void closeRecordingStream() {
-        if (pinnedThreadValidation) {
-            try {
-                // Flush ending events
-                recordingStream.stop();
-                if (pinningException != null) {
-                    throw pinningException;
-                }
-            } finally {
-                recordingStream.close();
-            }
-        }
-    }
-
     @SuppressWarnings("unchecked")
     private <T extends Annotation> T[] getAnnotations(Class<?> testClass, Class<T> annotClass,
             List<Annotation> metaAnnotations) {
@@ -463,15 +460,6 @@ private static boolean hasAnnotation(AnnotatedElement element, Set<Class<? exten
         return false;
     }
 
-    void record(RecordedEvent event) {
-        PinningException e = new PinningException(event);
-        if (pinningException == null) {
-            pinningException = e;
-        } else {
-            pinningException.addSuppressed(e);
-        }
-    }
-
     @SuppressWarnings("CdiManagedBeanInconsistencyInspection")
     private record TestInstanceExtension(Object testInstance, Class<?> testClass) implements Extension {
 
@@ -687,27 +675,4 @@ private static final class SingletonLiteral extends AnnotationLiteral<Singleton>
         static final SingletonLiteral INSTANCE = new SingletonLiteral();
     }
 
-    private static class PinningException extends AssertionError {
-        private final RecordedEvent recordedEvent;
-
-        PinningException(RecordedEvent recordedEvent) {
-            this.recordedEvent = recordedEvent;
-            if (recordedEvent.getStackTrace() != null) {
-                StackTraceElement[] stackTraceElements = recordedEvent.getStackTrace().getFrames().stream()
-                        .map(f -> new StackTraceElement(f.getMethod().getType().getName(),
-                                                        f.getMethod().getName(),
-                                                        f.getMethod().getType().getName() + ".java",
-                                                        f.getLineNumber()))
-                        .toArray(StackTraceElement[]::new);
-                super.setStackTrace(stackTraceElements);
-            }
-        }
-
-        @Override
-        public String getMessage() {
-            return "Pinned virtual threads were detected:\n"
-                    + recordedEvent.toString();
-        }
-    }
-
 }
diff --git a/...g/testng/src/main/java/io/helidon/microprofile/testing/testng/PinnedThreadValidation.java b/...g/testng/src/main/java/io/helidon/microprofile/testing/testng/PinnedThreadValidation.java
diff --git a/microprofile/testing/testng/src/main/java/module-info.java b/microprofile/testing/testng/src/main/java/module-info.java
@@ -27,13 +27,13 @@
     requires jakarta.cdi;
     requires jakarta.inject;
     requires jakarta.ws.rs;
-    requires jdk.jfr;
     requires microprofile.config.api;
     requires org.testng;
 
     requires static io.helidon.microprofile.server;
     requires static jersey.cdi1x;
     requires static jersey.weld2.se;
+    requires io.helidon.microprofile.testing.common;
 
     exports io.helidon.microprofile.testing.testng;