[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