From 7f1ea0f40e51c344f95d198cb36bc1214de0e3d3 Mon Sep 17 00:00:00 2001 From: Daniel Kec Date: Sun, 15 Dec 2024 22:58:14 +0100 Subject: [PATCH] TestNG with always helidon fix Signed-off-by: Daniel Kec --- bom/pom.xml | 5 + .../testing/common/PinningRecorder.java | 4 + .../common/src/main/java/module-info.java | 3 +- microprofile/testing/junit5/pom.xml | 1 - .../testing/junit5/HelidonJunitExtension.java | 17 ++-- .../junit5/src/main/java/module-info.java | 1 - microprofile/testing/testng/pom.xml | 4 + .../testing/testng/HelidonTest.java | 18 +++- .../testing/testng/HelidonTestNgListener.java | 99 ++++++------------- .../testng/PinnedThreadValidation.java | 31 ------ .../testng/src/main/java/module-info.java | 2 +- .../testing/testng/TestPinnedThread.java | 66 ++++++++++--- .../programmatic/PinningExtraThreadTest.java | 41 ++++++++ .../tests/testing/testng/test-suite.xml | 7 +- 14 files changed, 170 insertions(+), 129 deletions(-) delete mode 100644 microprofile/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/PinnedThreadValidation.java create mode 100644 microprofile/tests/testing/testng/src/test/java/io/helidon/microprofile/tests/testing/testng/programmatic/PinningExtraThreadTest.java diff --git a/bom/pom.xml b/bom/pom.xml index 3db507a7651..9ca591aefad 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -1088,6 +1088,11 @@ ${helidon.version} + + io.helidon.microprofile.testing + helidon-microprofile-testing-common + ${helidon.version} + io.helidon.microprofile.testing helidon-microprofile-testing-junit5 diff --git a/microprofile/testing/common/src/main/java/io/helidon/microprofile/testing/common/PinningRecorder.java b/microprofile/testing/common/src/main/java/io/helidon/microprofile/testing/common/PinningRecorder.java index 4dbff79d0b7..7ab787767e6 100644 --- a/microprofile/testing/common/src/main/java/io/helidon/microprofile/testing/common/PinningRecorder.java +++ b/microprofile/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 index 421605272fd..8e1c27b22d0 100644 --- 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; } \ No newline at end of file diff --git a/microprofile/testing/junit5/pom.xml b/microprofile/testing/junit5/pom.xml index 2c8c1136aaf..a8249b6f1ce 100644 --- a/microprofile/testing/junit5/pom.xml +++ b/microprofile/testing/junit5/pom.xml @@ -41,7 +41,6 @@ io.helidon.microprofile.testing helidon-microprofile-testing-common - 4.2.0-SNAPSHOT io.helidon.microprofile.cdi diff --git a/microprofile/testing/junit5/src/main/java/io/helidon/microprofile/testing/junit5/HelidonJunitExtension.java b/microprofile/testing/junit5/src/main/java/io/helidon/microprofile/testing/junit5/HelidonJunitExtension.java index 12b49c71000..8373fe1c652 100644 --- a/microprofile/testing/junit5/src/main/java/io/helidon/microprofile/testing/junit5/HelidonJunitExtension.java +++ b/microprofile/testing/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 index ddd5f2e8f56..7ce449e5a70 100644 --- 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 index 9f781af4fc2..d5097d30970 100644 --- a/microprofile/testing/testng/pom.xml +++ b/microprofile/testing/testng/pom.xml @@ -38,6 +38,10 @@ helidon-microprofile-server true + + io.helidon.microprofile.testing + helidon-microprofile-testing-common + io.helidon.microprofile.cdi helidon-microprofile-cdi diff --git a/microprofile/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/HelidonTest.java b/microprofile/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/HelidonTest.java index 8f86fb6f7a7..9e2da341a0e 100644 --- a/microprofile/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/HelidonTest.java +++ b/microprofile/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. *

@@ -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/microprofile/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/HelidonTestNgListener.java b/microprofile/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/HelidonTestNgListener.java index 283c6018b56..c50a6552008 100644 --- a/microprofile/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/HelidonTestNgListener.java +++ b/microprofile/testing/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 classLevelExtensions = new ArrayList<>(); private List 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 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 getAnnotation(Class testClass, Class 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[] getAnnotations(Class testClass, Class annotClass, List metaAnnotations) { @@ -463,15 +460,6 @@ private static boolean hasAnnotation(AnnotatedElement element, Set testClass) implements Extension { @@ -687,27 +675,4 @@ private static final class SingletonLiteral extends AnnotationLiteral 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/microprofile/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/PinnedThreadValidation.java b/microprofile/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/PinnedThreadValidation.java deleted file mode 100644 index e3be0b51783..00000000000 --- a/microprofile/testing/testng/src/main/java/io/helidon/microprofile/testing/testng/PinnedThreadValidation.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright (c) 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.helidon.microprofile.testing.testng; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Inherited; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** - * An annotation making this test class to fail at the end if a pinned virtual thread was detected. - */ -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.TYPE) -@Inherited -public @interface PinnedThreadValidation { -} diff --git a/microprofile/testing/testng/src/main/java/module-info.java b/microprofile/testing/testng/src/main/java/module-info.java index 6a5d76347fe..ae58c427fef 100644 --- 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; diff --git a/microprofile/tests/testing/testng/src/test/java/io/helidon/microprofile/tests/testing/testng/TestPinnedThread.java b/microprofile/tests/testing/testng/src/test/java/io/helidon/microprofile/tests/testing/testng/TestPinnedThread.java index 6660b31425a..775cda0446c 100644 --- a/microprofile/tests/testing/testng/src/test/java/io/helidon/microprofile/tests/testing/testng/TestPinnedThread.java +++ b/microprofile/tests/testing/testng/src/test/java/io/helidon/microprofile/tests/testing/testng/TestPinnedThread.java @@ -16,25 +16,61 @@ package io.helidon.microprofile.tests.testing.testng; -import io.helidon.microprofile.testing.testng.PinnedThreadValidation; +import java.util.Arrays; -import org.testng.annotations.Ignore; +import io.helidon.microprofile.testing.common.PinningException; +import io.helidon.microprofile.tests.testing.testng.programmatic.PinningExtraThreadTest; + +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.testng.Assert; +import org.testng.TestListenerAdapter; +import org.testng.TestNG; import org.testng.annotations.Test; -@PinnedThreadValidation -class TestPinnedThread { +import static org.hamcrest.CoreMatchers.startsWith; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsCollectionContaining.hasItem; + +public class TestPinnedThread { + + private static final String EXPECTED_PINNING_METHOD_NAME = "lambda$testPinningExtraThread$0"; @Test - @Ignore("Enable to verify pinned threads fails") - void test() throws InterruptedException { - Thread.ofVirtual().start(() -> { - synchronized (this) { - try { - Thread.sleep(1000); - } catch (InterruptedException e) { - Thread.interrupted(); - } - } - }).join(); + void testListener() { + TestNG testng = new TestNG(); + testng.setTestClasses(new Class[] {PinningExtraThreadTest.class}); + TestListenerAdapter tla = new TestListenerAdapter(); + testng.addListener(tla); + PinningException pinningException = Assert.expectThrows(PinningException.class, testng::run); + assertThat(pinningException.getMessage(), startsWith("Pinned virtual threads were detected:")); + assertThat("Method with pinning is missing from stack strace.", Arrays.asList(pinningException.getStackTrace()), + hasItem(new StackTraceElementMatcher(EXPECTED_PINNING_METHOD_NAME))); + } + + private static class StackTraceElementMatcher extends BaseMatcher { + + private final String methodName; + + StackTraceElementMatcher(String methodName) { + this.methodName = methodName; + } + + @Override + public boolean matches(Object o) { + return methodName.equals(((StackTraceElement) o).getMethodName()); + } + + @Override + public void describeMismatch(Object o, Description description) { + description.appendText("method ").appendValue(methodName) + .appendText(" does not match stack trace element ") + .appendValue(o); + } + + @Override + public void describeTo(Description description) { + + } } } diff --git a/microprofile/tests/testing/testng/src/test/java/io/helidon/microprofile/tests/testing/testng/programmatic/PinningExtraThreadTest.java b/microprofile/tests/testing/testng/src/test/java/io/helidon/microprofile/tests/testing/testng/programmatic/PinningExtraThreadTest.java new file mode 100644 index 00000000000..feac99894cc --- /dev/null +++ b/microprofile/tests/testing/testng/src/test/java/io/helidon/microprofile/tests/testing/testng/programmatic/PinningExtraThreadTest.java @@ -0,0 +1,41 @@ +/* + * Copyright (c) 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.microprofile.tests.testing.testng.programmatic; + +import io.helidon.microprofile.testing.testng.HelidonTest; + +import org.testng.annotations.Test; + +/** + * Test executed programmatically from TestPinnedThread Test. + */ +@HelidonTest +public class PinningExtraThreadTest { + + @Test + void testPinningExtraThread() throws InterruptedException { + Thread.ofVirtual().start(() -> { + synchronized (this) { + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + Thread.interrupted(); + } + } + }).join(); + } +} diff --git a/microprofile/tests/testing/testng/test-suite.xml b/microprofile/tests/testing/testng/test-suite.xml index a42203a85eb..3371780e453 100644 --- a/microprofile/tests/testing/testng/test-suite.xml +++ b/microprofile/tests/testing/testng/test-suite.xml @@ -1,7 +1,7 @@ + + \ No newline at end of file