On Thu, 13 Mar 2025 19:02:08 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
> The nsk_jvmti_setFailStatus() sometimes is called after test check results.
> In these cases the warning logs are generated and hide the real failure
> reasons. Also, I think it is a error-prone way to set and check error, since
> check might be just forgotten. Also, the test execution after failure might
> be incorrect and also make failure analysis harder.
> So I think it makes sense to always fail when nsk_jvmti_setFailStatus() is
> called.
>
> If this is going to work I'll rename it later and add add optional message to
> be more informative.
The fix looks okay in general. I've posted one comment about typos and one
question.
test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/hotswap/HotSwap.cpp line 157:
> 155: NSK_DISPLAY0("CompiledMethodLoad event recieved in dead phase");
> 156: return;
> 157: }
Nit: Typos at L148: `GetMethodNamme` => `GetMethodName`, `is work` => `works`,
`phasem` => `phase`
A suggestion is to reformulate the comment as below:
// GetMethodName works in live phase only so just exit if the event is
generated too late
Also, I wonder if we want to abort/fail in all cases when `phase ==
JVMTI_PHASE_DEAD`.
-------------
PR Review: https://git.openjdk.org/jdk/pull/24040#pullrequestreview-2700233519
PR Review Comment: https://git.openjdk.org/jdk/pull/24040#discussion_r2004307114
