jmx-dev 8221303: sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java fails due to java.rmi.server.ExportException: Port already in use

[Chris Plummer]

Hi Daniil,

I think you can remove "Ok" from the following message:

  237 System.out.println("DEBUG: OK. Spawned java process terminated 
with expected exit code of "
  238                         + STOP_PROCESS_EXIT_VAL);

It's somewhat misleading since the test can still fail. I think the 
following is also misleading:

  249             if (testFailed) {
  250                 output.reportDiagnosticSummary();
  251                 if (output.getStderr().contains("Port already in 
use")) {
  252                     // Need to retry
  253                     return true;
  254                 }
  255             }

The test can still pass after this happens, right? And I think there are 
other "Test FAILURE" error message that can appear just before this. 
Perhaps the code above should add a println that indicates that there 
will be a retry attempt since the failure was due to the port being in use.

Otherwise looks good.

thanks,

Chris

On 7/17/19 1:30 PM, Alex Menkov wrote:
> Hi Daniil,
>
> The fix looks good in general.
> Couple cosmetic notes (no new webrev required):
>
>  162                 needRetry = runTest();
>  163             }
>  164             while (needRetry && (attempts++ < MAX_RETRY_ATTEMTS));
> Please move "while" to the prev line:
>  163             } while (needRetry && (attempts++ < MAX_RETRY_ATTEMTS));
>
>
>  242                     System.out.println("Test FAILURE on" + name + 
> " reason: The expected line \"" + READY_MSG
>  243                             + "\" is not present in the process 
> output");
> Please add space: "Test FAILURE on " + name
>
> --alex
>
> On 07/17/2019 12:46, Daniil Titov wrote:
>> Hi Chris,
>>
>>>     It's a little unclear to me why you moved from ProcessThread to
>>>    TestProcessThread + Process. An explanation of that would make it 
>>> easier
>>>    to understand many of the changes.
>>
>> There are two reasons for that:
>> 1)  For every network interface the test starts a separate thread 
>> that runs the test for this interface. We want that if the test fails
>>      due to the bind error the thread not to exit but try to repeat 
>> the test several times. jdk.test.lib.thread.ProcessThread doesn't 
>> allow this.
>> 2) To filter out the cases when the test fails due to the bind error 
>> we need to parse the process output. jdk.test.lib.thread.ProcessThread
>>    registers its own pumps for stdin and sdtout  (by calling 
>> ProcessTools.startProcess()) that consume all output of the process 
>> and prevent
>>    the output analyzer from collecting this output.
>>   Thanks!
>> --Daniil
>>
>> On 7/17/19, 12:11 PM, "Chris Plummer" <chris.plummer at oracle.com> wrote:
>>
>>      Hi Daniil,
>>           It's a little unclear to me why you moved from 
>> ProcessThread to
>>      TestProcessThread + Process. An explanation of that would make 
>> it easier
>>      to understand many of the changes.
>>           thanks,
>>           Chris
>>           On 7/11/19 10:16 AM, Daniil Titov wrote:
>>      > Please review the change that fixes an intermittent failure of 
>> sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java
>>      > test due to ports collision. The tests finds all network 
>> interfaces and for every interface starts a separate process that tests
>>      > the connection to JMX agent server for a specific 
>> ports/interface combination.
>>      >
>>      > The test was changed to retry in case of the failure. If the 
>> subprocess fails to bind and the number
>>      > of retry attempts doesn't exceed the limit a new pair of 
>> random ports is selected and the test is run again.
>>      >
>>      > Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.01/
>>      > Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
>>      >
>>      > Thanks!
>>      > --Daniil
>>      >
>>      >
>>
>>