On Thu, 24 Feb 2022 12:17:54 GMT, Kevin Walls <[email protected]> wrote:
>> Test fails occasionally due to a port clash.
>> Presumably the port that was returned by Utils.getFreePort(), is no longer
>> free.
>> The test creates a ProcessBuilder with the parameters for JMX, including
>> port number, and uses that to create a new Process.
>> It should retry with a new port if we fail due to a port in use, for some
>> limited number of attempts.
>>
>> main already has some retry logic, but not working:
>> it checks for an InvocationTargetException to contain a BindException, but
>> it simply gets a BindException, thrown by TestAppRun.start().
>> TestAppRun.start() runs the new process and scans for errors, but on failure
>> its predicate has only seen the first line of a failure, so a BindException
>> is never recognised and thrown.
>> Also main does not limit the retries, and handling the port retry in main()
>> is duplicated, for each run of the test method.
>>
>> So...
>>
>> Make the error-scanning predicate in TestAppRun recognise a "port in use"
>> message and throw a BindExeption. This is a notification to the caller that
>> it failed, it's not the actual BindException as that was thrown in a
>> different process.
>> Make the testDefaultAgent method (the main part of the test) handle retrying
>> with a new port, a limited number of times.
>
> Kevin Walls has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Clarify predicate usage.
Marked as reviewed by msheppar (Reviewer).
test/jdk/javax/management/remote/mandatory/connection/DefaultAgentFilterTest.java
line 174:
> 172: private static final int FREE_PORT_ATTEMPTS = 10;
> 173:
> 174: private static void testDefaultAgent(String propertyFile) throws
> Exception {
This is a good refactoring change as it also removes some broken logic in the
current retry strategy
-------------
PR: https://git.openjdk.java.net/jdk/pull/7589
