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

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8206187: javax/management/remote/mandatory/connection/DefaultAgentFilterTest.java fails
From:       Mark Sheppard <msheppar () openjdk ! java ! net>
Date:       2022-02-24 23:40:12
Message-ID: AhGodJNbqA6Y7AM_3xGmLKvJOWA3g45QrWZlKRmOtCo=.38825ff5-9132-4a4e-bd27-ce31a2016d5c () github ! com
[Download RAW message or body]

On Thu, 24 Feb 2022 15:02:40 GMT, Kevin Walls <kevinw@openjdk.org> 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: 
> break to terminate retry loop when succesful

Marked as reviewed by msheppar (Reviewer).

A good spot by yourself ... as the focus was on the BindException correction and the \
pass condition is an Exception being thrown, it was easy to miss the break for a \
failure condition ... a similar issue existed in the current test albeit only if an \
InvocationTargetException with a nested BindException was thrown and the retry \
resulted in the  InvalidClassException or UnmarcshalledException !!

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

PR: https://git.openjdk.java.net/jdk/pull/7589


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