[OpenJDK 2D-Dev] [9]JDK-6949753:[TEST BUG]: java/awt/print/PageFormat/PDialogTest.java needs update by removing a infinite loop

Prasanta Sadhukhan prasanta.sadhukhan at oracle.com
Fri Jun 9 10:27:39 UTC 2017


looks good to me.

Regards
Prasanta
On 6/9/2017 3:49 PM, Shashidhara Veerabhadraiah wrote:
>
> Hi All, Please find the updated Webrev with fixes for the comments @ 
> http://cr.openjdk.java.net/~pkbalakr/shashi/6949753/webrev_04/ 
> <http://cr.openjdk.java.net/%7Epkbalakr/shashi/6949753/webrev_04/>
>
> Thanks and regards,
> Shashi
>
> *From:*Philip Race
> *Sent:* Thursday, June 8, 2017 3:32 AM
> *To:* Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com>
> *Cc:* Shashidhara Veerabhadraiah 
> <shashidhara.veerabhadraiah at oracle.com>; 2d-dev at openjdk.java.net
> *Subject:* Re: [9]JDK-6949753:[TEST BUG]: 
> java/awt/print/PageFormat/PDialogTest.java needs update by removing a 
> infinite loop
>
> .. and please make sure all lines are <= 80 chars as per the coding 
> standards.
>
> -phil.
>
> On 6/6/17, 11:59 PM, Prasanta Sadhukhan wrote:
>
>     do_test() does not need to be under EDT as it invokes printer
>     pagedialog and not swing components. Actually, createUI() needs to
>     be under EDT which has not been done.
>
>     Also,
>
>     79         SwingUtilities.invokeAndWait(() -> {
>
>       80             test.disposeUI();
>
>       81         });
>
>       82     }
>
>     should be called before you throw RuntimeException when test times
>     out .
>
>     There is no need of calling this after
>
>     75         if (test.testResult == false) {
>
>       76             throw new RuntimeException("Test Failed.");
>
>       77         }
>
>     as it has already been called in pass/fail actionlistener.
>
>     Also, put a sleep after T1.start() and do_test() otherwise since
>     they are in separate thread, in mycase, pagedialog is displayed
>     before test instructions dialog.
>
>     Regards
>
>     Prasanta
>
>     On 6/7/2017 11:50 AM, Shashidhara Veerabhadraiah wrote:
>
>         Hi All,
>
>         I have altered the manual test template per the comments.
>
>         1.Have moved the test instructions window under newly created
>         thread.
>
>         2.Have moved the print dialog(main test module) under EDT.
>
>         3.Timer management shall be done on the main thread.
>
>         I have placed the updated Webrev @
>         http://cr.openjdk.java.net/~pkbalakr/shashi/6949753/webrev_03/
>         <http://cr.openjdk.java.net/%7Epkbalakr/shashi/6949753/webrev_03/>
>
>         Please let me know if any comments on it.
>
>         Thanks and regards,
>
>         Shashi
>
>         *From:*Prasanta Sadhukhan
>         *Sent:* Tuesday, June 6, 2017 11:52 AM
>         *To:* Shashidhara Veerabhadraiah
>         <shashidhara.veerabhadraiah at oracle.com>
>         <mailto:shashidhara.veerabhadraiah at oracle.com>;
>         2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>
>         *Cc:* Philip Race <philip.race at oracle.com>
>         <mailto:philip.race at oracle.com>
>         *Subject:* Re: [9]JDK-6949753:[TEST BUG]:
>         java/awt/print/PageFormat/PDialogTest.java needs update by
>         removing a infinite loop
>
>         As I told, pageDialog is modal so latch.await() will not be
>         called if user does not close the page dialog or do any
>         interaction. The actual test
>
>         59         PageFormat pageFormat = new PageFormat();
>
>           60
>
>           61         createNewPrintPageSetup(pageFormat);
>
>          62
>
>           63         setValuesForPrintPageSetup(pageFormat, 2);
>
>           64
>
>           65         createNewPrintPageSetup(pageFormat);
>
>           66
>
>           67         setValuesForPrintPageSetup(pageFormat, 3);
>
>           68
>
>           69         createNewPrintPageSetup(pageFormat);
>
>
>         should be done in other thread.
>
>         Regards
>         Prasanta
>
>         On 6/6/2017 11:24 AM, Shashidhara Veerabhadraiah wrote:
>
>             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
>             http://cr.openjdk.java.net/~pkbalakr/shashi/6949753/webrev_02/
>             <http://cr.openjdk.java.net/%7Epkbalakr/shashi/6949753/webrev_02/>.
>
>             Thanks and regards,
>
>             Shashi
>
>             *From:*Prasanta Sadhukhan
>             *Sent:* Monday, June 5, 2017 12:35 PM
>             *To:* Shashidhara Veerabhadraiah
>             <shashidhara.veerabhadraiah at oracle.com>
>             <mailto:shashidhara.veerabhadraiah at oracle.com>;
>             2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>
>             *Cc:* Philip Race <philip.race at oracle.com>
>             <mailto:philip.race at oracle.com>
>             *Subject:* Re: [9]JDK-6949753:[TEST BUG]:
>             java/awt/print/PageFormat/PDialogTest.java needs update by
>             removing a infinite loop
>
>             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?
>
>             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()
>
>             Iguess you need to do this.
>
>             TestUI test = new TestUI(latch);
>                     Thread T1 = new Thread(test);
>                     T1.start();
>
>             class TestUI implements Runnable {
>             ...
>             @Override
>                 public void run() {
>                     try {
>                         createUI();
>
>             Regards
>             Prasanta
>
>             On 6/2/2017 4:00 PM, Shashidhara Veerabhadraiah wrote:
>
>                 Hi, I have fixed the comments below and updated the
>                 webrev @
>                 http://cr.openjdk.java.net/~pkbalakr/shashi/6949753/webrev_01/
>                 <http://cr.openjdk.java.net/%7Epkbalakr/shashi/6949753/webrev_01/>
>
>                 Thanks and regards,
>
>                 Shashi
>
>                 *From:*Prasanta Sadhukhan
>                 *Sent:* Friday, June 2, 2017 12:36 PM
>                 *To:* Shashidhara Veerabhadraiah
>                 <shashidhara.veerabhadraiah at oracle.com>
>                 <mailto:shashidhara.veerabhadraiah at oracle.com>;
>                 2d-dev at openjdk.java.net <mailto:2d-dev at openjdk.java.net>
>                 *Cc:* Philip Race <philip.race at oracle.com>
>                 <mailto:philip.race at oracle.com>
>                 *Subject:* Re: [9]JDK-6949753:[TEST BUG]:
>                 java/awt/print/PageFormat/PDialogTest.java needs
>                 update by removing a infinite loop
>
>                 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
>                 functioncreateNewPrintPageSetup() for it (as it calls
>                 1 method) but it is upto you.
>
>                 Few comments:
>
>                 Copyright should have "," after 2017.
>                 I guess createUI() does not have any call that throws
>                 exception so no need to have try-catch block for
>                 createUI().
>                 Also, there is no need to catch PrinterException and
>                 rethrow RuntimeException, so you can do away with that
>                 try-catch.
>                 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.
>                 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.
>
>                 Regards
>                 Prasanta
>
>                 On 6/1/2017 5:10 PM, Shashidhara Veerabhadraiah wrote:
>
>                     Hi All,
>
>                     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.
>
>                       
>
>                     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.
>
>                       
>
>                     The updated test file includes the standard manual test template along with test cases to cover the printer dialog's margin attributes retention feature.
>
>                       
>
>                     Bug:
>
>                     <https://bugs.openjdk.java.net/browse/JDK-6949753>
>                     <https://bugs.openjdk.java.net/browse/JDK-6949753>
>
>                       
>
>                     Webrev:
>
>                     <http://cr.openjdk.java.net/~pkbalakr/shashi/6949753/webrev_00/>
>                     <http://cr.openjdk.java.net/%7Epkbalakr/shashi/6949753/webrev_00/>
>
>                       
>
>                     Note : PrintDialog on Mac does not show page margins and hence this test does not run on Mac.
>
>                       
>
>                     Thanks and regards,
>
>                     Shashi
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20170609/c2e6aee5/attachment-0001.html>


More information about the 2d-dev mailing list