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

Phil Race philip.race at oracle.com
Mon Jun 12 17:44:42 UTC 2017


The dialog does not show "Letter" because you said so. It shows it 
because the printer reports it supports letter.

-phil.

On 06/12/2017 10:44 AM, Shashidhara Veerabhadraiah wrote:
>
> Just to clarify Phil for the part of cross platform print dialog, here 
> is that dialog representation:
>
> Since this dialog can appear on any platform, the Paper class would 
> represent the backend object for the front end dialog properties(of a 
> generic printer) as shown in the pic above. Hence the use of the raw 
> Paper object than obtaining it from the page format.
>
> At the same time, as you pointed out, the test is not reliable with 
> the actual conditions. The only problem I see is that the moment we 
> change the test case to stay close to the actual scenario we may have 
> certain failures because the settings are different for different 
> locales and it may be difficult to adapt to every possible locales.
>
> Thanks and regards,
>
> Shashi
>
> *From:*Phil Race
> *Sent:* Monday, June 12, 2017 10:01 PM
> *To:* Shashidhara Veerabhadraiah 
> <shashidhara.veerabhadraiah at oracle.com>; Prasanta Sadhukhan 
> <prasanta.sadhukhan at oracle.com>
> *Cc:* 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
>
> */>[Shashi] Yes. I intend to use the default paper object as this is a 
> test related to the cross platform default printer dialog./*
> */> In the modified test file, I have set the size of the physical 
> paper instead of relying it on the default setting which may/*
> */> vary as you pointed out, depending on the locale./*
> */>Now that we have our own paper object(with a constant paper size) 
> and based on the margin setting(which are const/*
> */> it won’t cause an undesirable behavior like going into negative 
> space./*
> Sorry, that is not a valid thing to do. The print dialog is free to ignore this
> or workaround the incompatibility of that paper with the supported media so your test is not reliable.
> And I don't see what the cross platform print dialog has to do with it.
> It is, or should be, just as aware of the printer sizes as the native one.
> -phil.
>
> On 06/12/2017 03:34 AM, Shashidhara Veerabhadraiah wrote:
>
>     Hi Phil, Please see below for the comments:
>
>     The updated Webrev is at:
>
>     http://cr.openjdk.java.net/~aghaisas/shashi/6949753/webrev_05/
>     <http://cr.openjdk.java.net/%7Eaghaisas/shashi/6949753/webrev_05/>
>
>
> Modified in what way from the previous version ?
>
> -phil.
>
>
>     Thanks and regards,
>     Shashi
>
>     *From:*Phil Race
>     *Sent:* Saturday, June 10, 2017 3:07 AM
>     *To:* Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com>
>     <mailto:prasanta.sadhukhan at oracle.com>; Shashidhara Veerabhadraiah
>     <shashidhara.veerabhadraiah at oracle.com>
>     <mailto:shashidhara.veerabhadraiah at oracle.com>
>     *Cc:* 2d-dev at openjdk.java.net <mailto: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
>
>     private static void setValuesForPrintPageSetup(PageFormat
>     pageFormat,  int
>
>     118             marginValue) throws PrinterException {
>
>     119         Paper paper = new Paper();
>
>     double paperHeight = paper.getHeight();
>
>     122         double paperWidth = paper.getWidth();
>
>     123         double paperX = paper.getImageableX();
>
>     124         double paperY = paper.getImageableY();
>
>     125         paper.setImageableArea(paperX * marginValue, paperY *
>     marginValue,
>
>     126                 paperWidth - (paperX * 2 * marginValue),
>
>     127                 paperHeight - (paperY * 2 * marginValue));
>
>     105         setValuesForPrintPageSetup(pageFormat, 3);
>
>     I see you call new Paper() above
>
>     https://docs.oracle.com/javase/8/docs/api/java/awt/print/Paper.html#Paper--
>
>     Did you really intend to use a default paper instead of getting
>     the one
>
>     from the pageFormat ? On some label printer your Letter Paper may not
>
>     even be supported. US (aka NA) Letter is 8.5" wide.
>
>     Also although it probably will work out OK the maths isn't checking
>
>     for boundary problems.
>
>     default margin will be 1" so that's what you'll get for paperX and
>     paperY
>
>     Using your value of 3 we set the imagable area such that
>
>     imageable X = 1 * 3 = 3
>
>     imageableWidth = 8.5 - (1 * 2 *3) = 8.5 - 6 = 2.5;
>
>     Fortunately that worked out positive .. but it does not seem to be
>     enforced.
>
>     If we'd used 5 it would be a different story :
>
>     ix = 5, iw = 8.5 - ( 1 * 2 * 5) = -1.5
>
>     The implementation will (should) clamp it to non-negative but it
>
>     might still be better to have some defensive logic of your own.
>
>     */[Shashi] Yes. I intend to use the default paper object as this
>     is a test related to the cross platform default printer dialog. In
>     the modified test file, I have set the size of the physical paper
>     instead of relying it on the default setting which may vary as you
>     pointed out, depending on the locale./*
>
>     */Now that we have our own paper object(with a constant paper
>     size) and based on the margin setting(which are constants), it
>     won’t cause an undesirable behavior like going into negative space./*
>
>     nit: there's a missing space here
>
>       
>
>     75                 } catch(PrinterException e) {
>
>     */[Shashi] This is fixed now./*
>
>     -phil.
>
>     On 06/09/2017 03:27 AM, Prasanta Sadhukhan wrote:
>
>         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>
>             <mailto:prasanta.sadhukhan at oracle.com>
>             *Cc:* 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>
>             *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/20170612/f7866203/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image002.jpg
Type: image/jpeg
Size: 31683 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20170612/f7866203/image002-0001.jpg>


More information about the 2d-dev mailing list