On Thu, 24 Feb 2022 15:02:40 GMT, Kevin Walls <kev...@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
