Hi, Vyom Thank you for the review and comments, update webrev as below and comment inline
webrev: http://cr.openjdk.java.net/~xyin/8198882/webrev.02/ <http://cr.openjdk.java.net/~xyin/8198882/webrev.02/> > On 13 Jul 2018, at 1:46 PM, vyom tewari <vyom.tew...@oracle.com> wrote: > > Hi Chris, > > Thanks for doing this overall looks good to me, few minor comments. > 1-> DNSTestUtils.java, please start the server first and then set the > "TEST_DNS_SERVER_THREAD". This will not make much difference but we will > avoid setting "TEST_DNS_SERVER_THREAD" env variable if server fails to start. > 129 env.put(TEST_DNS_SERVER_THREAD, inst); > 130 inst.start(); Fixed, thanks > > 2-> I noticed that copyright date (Copyright (c) 2000, 2018,) but webrev > tells all the tests are new, please fix copyright date as well. Thanks for checking this. Since this task is part of umbrella enhancement JDK-8191011 <https://bugs.openjdk.java.net/browse/JDK-8191011> JNDI SQE tests co-location, for those added tests which are migrated from SQE tests, the copyright date will follow the guidance SQE Test copyright year + migration copyright year if the 2 year are not same, for dump files (like *.dns) are new added under our new framework so just use current copyright year, hope that explains :), thanks Regards, Chris > > Thanks, > Vyom > > On Thursday 12 July 2018 02:08 PM, Chris Yin wrote: >> Please have a review to new webrev as below, some code refactoring on lib >> parts and enhanced DNSServer to handle retry request which will make the >> tests more stable, thanks >> >> http://cr.openjdk.java.net/~xyin/8198882/webrev.01/ >> <http://cr.openjdk.java.net/%7Exyin/8198882/webrev.01/> >> >> Regards, >> Chris >> >>> On 22 Mar 2018, at 11:16 AM, Chris Yin <xu.y....@oracle.com >>> <mailto:xu.y....@oracle.com>> wrote: >>> >>> Please review the changes to add 10 JNDI tests to >>> com/sun/jndi/dns/AttributeTests/, thanks >>> >>> bug: https://bugs.openjdk.java.net/browse/JDK-8198882 >>> <https://bugs.openjdk.java.net/browse/JDK-8198882> >>> webrev: http://cr.openjdk.java.net/~xyin/8198882/webrev.00/ >>> <http://cr.openjdk.java.net/%7Exyin/8198882/webrev.00/> >>> >>> Regards, >>> Chris >> >
