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

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8221303: sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java fails due to
From:       Chris Plummer <chris.plummer () oracle ! com>
Date:       2019-07-17 22:58:56
Message-ID: 660499be-e531-3bae-adb3-e89342b00af3 () oracle ! com
[Download RAW message or body]

What does output.reportDiagnosticSummary() print out then the port is 
already in use, and have you made this happen with your fixes in place?

Chris

On 7/17/19 3:20 PM, Daniil Titov wrote:
> Hi Chris and Alex,
>
> Please review a new version of the fix that moves the diagnostic output for the test failure to run()
> method after the number of retry attempts is exceeded. It also includes other corrections that
> you suggested.
>
> Thanks!
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.02/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
>
> --Best regards,
> Daniil
>
>
> On 7/17/19, 1:47 PM, "Chris Plummer" <chris.plummer@oracle.com> wrote:
>
>      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@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
>      >>      >
>      >>      >
>      >>
>      >>
>      
>      
>
>


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