<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">I did not check the all code, but small comment: Note that all Swing components should be always accessed on EDT. If InvokeAnDWait does not work then you can use InvokeLater().</div><div class=""><br class=""><div class=""><br class=""><div><blockquote type="cite" class=""><br class="Apple-interchange-newline"><div class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255);"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">The manual test template that I received from the team seems buggy and an older version it seems. I have modified the same per your inputs and now placed the updated Webrev at<span class="Apple-converted-space"> </span><a href="http://cr.openjdk.java.net/~pkbalakr/shashi/6949753/webrev_02/" style="color: purple; text-decoration: underline;" class="">http://cr.openjdk.java.net/~pkbalakr/shashi/6949753/webrev_02/</a>.<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Thanks and regards,<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Shashi<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><a name="_MailEndCompose" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></a></div><div class=""><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(225, 225, 225); padding: 3pt 0in 0in;" class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><b class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: windowtext;" class="">From:</span></b><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: windowtext;" class=""><span class="Apple-converted-space"> </span>Prasanta Sadhukhan<span class="Apple-converted-space"> </span><br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>Monday, June 5, 2017 12:35 PM<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Shashidhara Veerabhadraiah <<a href="mailto:shashidhara.veerabhadraiah@oracle.com" class="">shashidhara.veerabhadraiah@oracle.com</a>>; <a href="mailto:2d-dev@openjdk.java.net" class="">2d-dev@openjdk.java.net</a><br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span>Philip Race <<a href="mailto:philip.race@oracle.com" class="">philip.race@oracle.com</a>><br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>Re: [9]JDK-6949753:[TEST BUG]: java/awt/print/PageFormat/PDialogTest.java needs update by removing a infinite loop<o:p class=""></o:p></span></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">I guess there is one more problem in usage of CountDown latch. Have you seen this test fail with timeout even if you wait for 5 minutes as per your timeout period?<o:p class=""></o:p></p><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">latch.await() needs to be wait on main thread while the test needs to be executed in another thread otherwise, pageDialog being modal the control will not come to latch.await()<o:p class=""></o:p></p><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Iguess you need to do this.<o:p class=""></o:p></p><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">TestUI test = new TestUI(latch);<br class="">        Thread T1 = new Thread(test);<br class="">        T1.start();<o:p class=""></o:p></p><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">class TestUI implements Runnable {<br class="">...<br class="">@Override<br class="">    public void run() {<br class="">        try {<br class="">            createUI();<br class=""><br class="">Regards<br class="">Prasanta<o:p class=""></o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">On 6/2/2017 4:00 PM, Shashidhara Veerabhadraiah wrote:<o:p class=""></o:p></div></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Hi, I have fixed the comments below and updated the webrev @<span class="Apple-converted-space"> </span><a href="http://cr.openjdk.java.net/%7Epkbalakr/shashi/6949753/webrev_01/" style="color: purple; text-decoration: underline;" class="">http://cr.openjdk.java.net/~pkbalakr/shashi/6949753/webrev_01/</a></span><o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""> </span><o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Thanks and regards,</span><o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Shashi</span><o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""> </span><o:p class=""></o:p></div><div class=""><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(225, 225, 225); padding: 3pt 0in 0in;" class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><b class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: windowtext;" class="">From:</span></b><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: windowtext;" class=""><span class="Apple-converted-space"> </span>Prasanta Sadhukhan<span class="Apple-converted-space"> </span><br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>Friday, June 2, 2017 12:36 PM<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Shashidhara Veerabhadraiah<span class="Apple-converted-space"> </span><a href="mailto:shashidhara.veerabhadraiah@oracle.com" style="color: purple; text-decoration: underline;" class=""><shashidhara.veerabhadraiah@oracle.com></a>;<span class="Apple-converted-space"> </span><a href="mailto:2d-dev@openjdk.java.net" style="color: purple; text-decoration: underline;" class="">2d-dev@openjdk.java.net</a><br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span>Philip Race<span class="Apple-converted-space"> </span><a href="mailto:philip.race@oracle.com" style="color: purple; text-decoration: underline;" class=""><philip.race@oracle.com></a><br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>Re: [9]JDK-6949753:[TEST BUG]: java/awt/print/PageFormat/PDialogTest.java needs update by removing a infinite loop</span><o:p class=""></o:p></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""> <o:p class=""></o:p></div><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Test fix look ok. Only thing is, you can call getPrinterJob() once and reutilise instead of calling 3 times and probably there is no need of creating a function<span class="new"><span class="Apple-converted-space"> </span>createNewPrintPageSetup()</span><span class="Apple-converted-space"> </span>for it (as it calls 1 method) but it is upto you.<o:p class=""></o:p></p><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Few comments:<o:p class=""></o:p></p><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Copyright should have "," after 2017.<br class="">I guess createUI() does not have any call that throws exception so no need to have try-catch block for createUI().<br class="">Also, there is no need to catch PrinterException and rethrow RuntimeException, so you can do away with that try-catch.<br class="">Also, you can call disposeUI() in passButton and failButton actionlistener instead of in main().  Also, there is no need to do setVisible(false) in disposeUI(), dispose() will take care of that.<br class="">You can throw RuntimeException when test timed out (instead of just println and later getting test fail exception) which is different from Test Failed RuntimeException.<span class="Apple-converted-space"> </span><br class=""><br class="">Regards<br class="">Prasanta<o:p class=""></o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">On 6/1/2017 5:10 PM, Shashidhara Veerabhadraiah wrote:<o:p class=""></o:p></div></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class="">Hi All,<o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class="">Please review a fix for a test bug which contained an infinite loop to test the printer setup dialog's margin attributes retention without the manual step procedure.<o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class=""> <o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class="">The issue with PDialogTest.java which tests the printer setup dialog's margin attributes retention by having as infinite loop to keep popping up the dialog without a proper exit. The test does not cover the instruction steps necessary to properly test dialog's margin attributes retention.<o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class=""> <o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class="">The updated test file includes the standard manual test template along with test cases to cover the printer dialog's margin attributes retention feature.<o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class=""> <o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class="">Bug:<o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class=""><a href="https://bugs.openjdk.java.net/browse/JDK-6949753" style="color: purple; text-decoration: underline;" class=""><https://bugs.openjdk.java.net/browse/JDK-6949753></a><o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class=""> <o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class="">Webrev:<o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class=""><a href="http://cr.openjdk.java.net/%7Epkbalakr/shashi/6949753/webrev_00/" style="color: purple; text-decoration: underline;" class=""><http://cr.openjdk.java.net/~pkbalakr/shashi/6949753/webrev_00/></a><o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class=""> <o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class="">Note : PrintDialog on Mac does not show page margins and hence this test does not run on Mac.<o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class=""> <o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class="">Thanks and regards,<o:p class=""></o:p></pre><pre style="margin: 0in 0in 0.0001pt; font-size: 10pt; font-family: 'Courier New';" class="">Shashi<o:p class=""></o:p></pre></blockquote><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""> <o:p class=""></o:p></div></blockquote><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div></div></blockquote></div><br class=""></div></div></body></html>