[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