On Tue, 4 Jun 2024 07:12:27 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> unused import > > test/jdk/java/lang/instrument/NativeMethodPrefixAgent.java line 34: > >> 32: * @modules java.management >> 33: * java.instrument >> 34: * @run shell/timeout=240 MakeJAR2.sh NativeMethodPrefixAgent >> NativeMethodPrefixApp 'Can-Retransform-Classes: true' >> 'Can-Set-Native-Method-Prefix: true' > > Just a question. The original test had a timeout:`timeout=240`. My guess is > that the default timeout was not enough. Do you think, it is not needed > anymore or you just missed to add it? > The same question applies to the other test. Hello Serguei, that `timeout=240` appears to have been added as part of https://bugs.openjdk.org/browse/JDK-6528548 to several of these tests back in Java 7 days because the `shell` test which was creating the jar file was timing out. The actual test itself, the one which uses the generated jar to run the agent, was completing within a second or two. With the current state of the PR I have run tier3 in our CI and there too both these tests complete within a second on all platforms. So I decided not to copy over this timeout as part of this change. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19495#discussion_r1625628639