RFR: 8334167: Test java/lang/instrument/NativeMethodPrefixApp.java timed out [v2]

[Jaikiran Pai]

On Mon, 15 Jul 2024 07:00:12 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>> 
>>  - David's suggestion - remove cosmetic/style changes
>>  - no need to timeout=240
>>  - include stdio.h in test native library
>>  - merge latest from master branch
>>  - 8334167: Test java/lang/instrument/NativeMethodPrefixApp.java timed out
>
> test/jdk/java/lang/instrument/NativeMethodPrefixApp.java line 44:
> 
>> 42:  * @comment The test uses asmlib/Instrumentor.java which relies on ClassFile API PreviewFeature.
>> 43:  * @run main/native/timeout=240 NativeMethodPrefixApp roleDriver
>> 44:  * @comment The test uses a higher timeout to prevent test timeouts noted in JDK-6528548
> 
> Is /timeout=240 (and the comment) needed now. If I read the old issue correctly it was due to use a JDK mounted on a network file system.

In https://github.com/openjdk/jdk/pull/19495#discussion_r1625470816 we had considered removing the higher timeout from this test. But then decided to let it stay at that time. I think we don't need it anymore, not even when the test runs with `-Xcomp`. The recent runs in our CI for this test haven't shown the necessity of using this higher timeout. I have updated the PR to remove this. I plan to run additional tests in our CI to be sure that the removal of the timeout doesn't cause unexpected failures.

> test/jdk/java/lang/instrument/libNativeMethodPrefix.c line 24:
> 
>> 22:  */
>> 23: 
>> 24: #include "jni.h"
> 
> I assume this needs `#include <stdio.h>`.

Updated the PR to include this.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20154#discussion_r1677457539
PR Review Comment: https://git.openjdk.org/jdk/pull/20154#discussion_r1677454337