[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 16:30:31 UTC 2017
*/>[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>; Shashidhara
> Veerabhadraiah <shashidhara.veerabhadraiah 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
>
> 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/c8287c6a/attachment-0001.html>
More information about the 2d-dev
mailing list