RFR of JDK-8188897: java/rmi/registry/reexport/Reexport.java failed with Port already in use

Hamlin Li huaming.li at oracle.com
Wed Apr 4 03:35:30 UTC 2018


Hi Joe, Roger,

Thank you for reviewing, I have refactored the test more and fix as you 
suggested.

please review http://cr.openjdk.java.net/~mli/8188897/webrev.01/

Thank you

-Hamlin


On 04/04/2018 10:42 AM, Joseph D. Darcy wrote:
> Hello,
>
> Some general comments on the coding for tests like this:
>
> * It is preferable to avoid sleep in tests to avoid increasing the 
> minimum amount of time a test takes to run. This helps limit the 
> overall time it takes the test suite to run.
>
> * If timeouts are used, it is recommend to factor the maximum time 
> waited with the jtreg timeout scaling factor; I don't recall its exact 
> name. In other words, many of our tests are run on heavily loaded 
> systems and the tests take longer than run than on typical laptops and 
> workstations so jtreg is invoked with an timeout scaling factor. 
> Individual tests should be sensitive to this scaling factor for any 
> internal timeout that need to be used.
>
> HTH,
>
> -Joe
>
> On 4/3/2018 7:48 AM, Roger Riggs wrote:
>> Hi Hamlin,
>>
>> Instead of a simple time delay, it would be useful to wait for the 
>> RegistryVM to terminate.
>> In killRegistry: 149,  adding subreg.waitFor() should be sufficient.
>>
>> 58: If using a 'for' loop it would be easier to understand if it 
>> included the usual start, increment and termination.
>> Instead of burying it in the exception handler.
>>
>> 59, 102, 104: the introduction of the kill boolean makes the test 
>> harder to understand and seems to be unnecessary.
>>  the killRegistry() method already will only kill the subprocess if 
>> it still is alive.
>>
>> Roger
>>
>> On 4/2/2018 6:33 AM, Hamlin Li wrote:
>>> would you please review the following patch?
>>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8188897
>>>
>>> webrev: http://cr.openjdk.java.net/~mli/8188897/webrev.00/
>>>
>>>
>>> Thank you
>>>
>>> -Hamlin
>>>
>>
>



More information about the core-libs-dev mailing list