[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-jmx-dev
Subject:    Re: jmx-dev RFR: 8328273: sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java failed with
From:       Jaikiran Pai <jpai () openjdk ! org>
Date:       2024-04-02 1:11:58
Message-ID: M5IE06-x28aoYA4ytia_4L8lJxYy_vpJN56otZkWSEk=.c51dbba7-a928-4f5d-86da-ef1d87798217 () github ! com
[Download RAW message or body]

On Mon, 1 Apr 2024 02:02:40 GMT, Jaikiran Pai <jpai@openjdk.org> wrote:

> Can I please get a review of this change which proposes to fix the test failure \
> reported in https://bugs.openjdk.org/browse/JDK-8328273? 
> As noted in that issue, the \
> `sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java` intermittently fails \
> with a port already in use error. The test attempts to find a free port and then \
> uses it during the test. The interesting part is that the test already has a loop \
> of 10 attempts to retry the test if the port wasn't actually free. So for the test \
> to fail, it would then mean that each of the 10 attempts of using a free port \
> failed (which should be extremely rare and should almost never happen).  
> I didn't have an answer for that until today and had it on my TODO to look further. \
> Credit goes to Kevin @kevinjwalls for identifying the issue - turns out this is the \
> exact same issue that Kevin fixed in https://github.com/openjdk/jdk/pull/18470 for \
> a different test. After noticing that fix, I spotted the same typo in the exception \
> message check in this test. That explains why it wasn't retrying at most 10 times. \
> The test was thus immediately failing on first attempt whenever the chosen free \
> port was in use. 
> I have run this test with a test repeat of 50 with this change and the test now \
> passes always. Without this change and a test repeat of 50, the test failed 2 \
> times. I've additionally searched for any other similar typos in other tests and \
> haven't found any (I searched for the string "Exception thrown by the agent :").

Thank you Dan for the review. 

Since this is a test-only trivial fix, I'll go ahead with the integration.

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

PR Comment: https://git.openjdk.org/jdk/pull/18561#issuecomment-2030898184


[prev in list] [next in list] [prev in thread] [next in thread]