On Mon, 15 Aug 2022 14:49:09 GMT, Alan Bateman <al...@openjdk.org> wrote:
>>> > Shouldn't we throw a SkippedException in this case? >>> >>> It's the child VM that skips so throwing SkippedException would require >>> special handling in the parent. >> >> Like this? >> >> diff --git >> a/test/jdk/java/lang/Thread/jni/AttachCurrentThread/AttachTest.java >> b/test/jdk/java/lang/Thread/jni/AttachCurrentThread/AttachTest.java >> index b93e7918014..156bbe35c62 100644 >> --- a/test/jdk/java/lang/Thread/jni/AttachCurrentThread/AttachTest.java >> +++ b/test/jdk/java/lang/Thread/jni/AttachCurrentThread/AttachTest.java >> @@ -48,6 +48,8 @@ import java.util.stream.Stream; >> import jdk.test.lib.process.ProcessTools; >> import jdk.test.lib.process.OutputAnalyzer; >> >> +import jtreg.SkippedException; >> + >> public class AttachTest { >> static final String TEST_CLASSES = System.getProperty("test.classes"); >> static final String JAVA_LIBRARY_PATH = >> System.getProperty("java.library.path"); >> @@ -63,6 +65,9 @@ public class AttachTest { >> .executeTestJava(opts) >> .outputTo(System.out) >> .errorTo(System.out); >> + if (outputAnalyzer.getOutput().contains("Test skipped, no native >> linker on this platform")) { >> + throw new SkippedException("Test skipped, no native linker on >> this platform"); >> + } >> outputAnalyzer.shouldHaveExitValue(0); >> } >> } >> >> Do I need to add this change? > >> > > Shouldn't we throw a SkippedException in this case? >> > >> > >> > It's the child VM that skips so throwing SkippedException would require >> > special handling in the parent. >> >> Like this? >> >> ``` >> diff --git >> a/test/jdk/java/lang/Thread/jni/AttachCurrentThread/AttachTest.java >> b/test/jdk/java/lang/Thread/jni/AttachCurrentThread/AttachTest.java >> index b93e7918014..156bbe35c62 100644 >> --- a/test/jdk/java/lang/Thread/jni/AttachCurrentThread/AttachTest.java >> +++ b/test/jdk/java/lang/Thread/jni/AttachCurrentThread/AttachTest.java >> @@ -48,6 +48,8 @@ import java.util.stream.Stream; >> import jdk.test.lib.process.ProcessTools; >> import jdk.test.lib.process.OutputAnalyzer; >> >> +import jtreg.SkippedException; >> + >> public class AttachTest { >> static final String TEST_CLASSES = System.getProperty("test.classes"); >> static final String JAVA_LIBRARY_PATH = >> System.getProperty("java.library.path"); >> @@ -63,6 +65,9 @@ public class AttachTest { >> .executeTestJava(opts) >> .outputTo(System.out) >> .errorTo(System.out); >> + if (outputAnalyzer.getOutput().contains("Test skipped, no native >> linker on this platform")) { >> + throw new SkippedException("Test skipped, no native linker on >> this platform"); >> + } >> outputAnalyzer.shouldHaveExitValue(0); >> } >> } >> ``` >> >> Do I need to add this change? > > That would work too but I think what you have in the PR now is okay. Thanks to @AlanBateman for the review. ------------- PR: https://git.openjdk.org/jdk/pull/9877