On Mon, 15 Aug 2022 14:49:09 GMT, Alan Bateman <[email protected]> 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
