PDialogTest.java needs update by removing a infinite loop

Phil Race Fri, 09 Jun 2017 14:40:03 -0700

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)); 105setValuesForPrintPageSetup(pageFormat, 3); I see you call new Paper()abovehttps://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 onefrom the pageFormat ? On some label printer your Letter Paper may noteven be supported. US (aka NA) Letter is 8.5" wide. Also although itprobably will work out OK the maths isn't checking for boundaryproblems. default margin will be 1" so that's what you'll get for paperXand paperY Using your value of 3 we set the imagable area such thatimageable 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 beenforced. 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 tonon-negative but it might still be better to have some defensive logicof your own. nit: there's a missing space here

75 } catch(PrinterException e) { -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.sadhuk...@oracle.com>

*Cc:* Shashidhara Veerabhadraiah<shashidhara.veerabhadra...@oracle.com>; 2d-dev@openjdk.java.net*Subject:* Re: [9]JDK-6949753:[TEST BUG]:java/awt/print/PageFormat/PDialogTest.java needs update by removing ainfinite loop

.. and please make sure all lines are <= 80 chars as per the codingstandards.


-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.veerabhadra...@oracle.com>
        <mailto:shashidhara.veerabhadra...@oracle.com>;
        2d-dev@openjdk.java.net <mailto:2d-dev@openjdk.java.net>
        *Cc:* Philip Race <philip.r...@oracle.com>
        <mailto:philip.r...@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.veerabhadra...@oracle.com>
            <mailto:shashidhara.veerabhadra...@oracle.com>;
            2d-dev@openjdk.java.net <mailto:2d-dev@openjdk.java.net>
            *Cc:* Philip Race <philip.r...@oracle.com>
            <mailto:philip.r...@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.veerabhadra...@oracle.com>
                <mailto:shashidhara.veerabhadra...@oracle.com>;
                2d-dev@openjdk.java.net <mailto:2d-dev@openjdk.java.net>
                *Cc:* Philip Race <philip.r...@oracle.com>
                <mailto:philip.r...@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

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

Reply via email to