8283664: Remove jtreg tag manual=yesno for java/awt/print/PrinterJob/PrintTextTest.java #21716

gredler · 2024-10-25T15:48:58Z

There are multiple issue with this test case

Parser error due to yesno in @run main/manual=yesno
User can only compare the UI rendering and compare with the print out. User can't mark the test as pass or fail due to pass or fail buttons are missing.
When the test is executed using jtreg after user click on the print button on the print dialog the whole test UIs ( frames) gets dispose and user cannot compare the printout with the UI. But this works as expected in test is running individually using java PrintTextTest

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8283664: Remove jtreg tag manual=yesno for java/awt/print/PrinterJob/PrintTextTest.java (Bug - P4)

Reviewers

Harshitha Onkar (@honkar-jdk - Reviewer)
Alexey Ivanov (@aivanov-jdk - Reviewer)
Andrey Turbanov (@turbanoff - Committer) Review applies to 1a110a58

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21716/head:pull/21716
$ git checkout pull/21716

Update a local copy of the PR:
$ git checkout pull/21716
$ git pull https://git.openjdk.org/jdk.git pull/21716/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 21716

View PR using the GUI difftool:
$ git pr show -t 21716

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21716.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2024-10-25T15:49:27Z

👋 Welcome back gredler! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-10-25T15:49:56Z

@gredler This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8283664: Remove jtreg tag manual=yesno for java/awt/print/PrinterJob/PrintTextTest.java

Reviewed-by: honkar, aivanov, aturbanov

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 183 new commits pushed to the master branch:

2c7bea1: 8343772: Expected IAPE not thrown in KDF.getInstance (TCK)
fac89f4: 8343493: Perform module checks during MetaspaceShared::map_archives()
ccda815: 8342303: Segfault in update_inherited_vtable: AppCDS, old bytecode, and redefineClasses
4f5a241: 8343491: javax/management/remote/mandatory/connection/DeadLockTest.java failing with NoSuchObjectException: no such object in table
56c588b: 8343417: (fs) BasicFileAttributeView.setTimes uses microsecond precision with NOFOLLOW_LINKS
d3c042f: 8343770: Build fails due to use of sun.misc.Unsafe in LoopOverRandom
1d117f6: 8343394: Make MemorySessionImpl.state a stable field
d2b681d: 8343730: JMX cleanups
ac82a8f: 8343610: InOutPathTest jpackage test produces invalid app image on macOS
f0b251d: 8343531: Improve print_location for invalid heap pointers
... and 173 more: https://git.openjdk.org/jdk/compare/0853aee3b377cf9f17340a85f600651db42e6999...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@honkar-jdk, @aivanov-jdk, @turbanoff) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk · 2024-10-25T15:50:34Z

@gredler The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2024-10-25T15:53:33Z

Webrevs

honkar-jdk

Is the manual test verified on all platforms?

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java

gredler · 2024-10-31T21:38:48Z

Is the manual test verified on all platforms?

This bug (JDK-8283664) only covers getting the test back to a runnable state. There is a separate entry (JDK-8148334) for fixing bugs exposed by this test which have not yet been fixed. I did verify that the bugs documented in JDK-8148334 do still exhibit -- so JDK-8148334 should remain open for now.

gredler · 2024-11-05T15:53:30Z

@honkar-jdk I've made the requested changes, is there anything else that should be addressed?

@prrace Would you be able to provide the second review for this PR?

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java

aivanov-jdk · 2024-11-05T16:50:18Z

Is the manual test verified on all platforms?

This bug (JDK-8283664) only covers getting the test back to a runnable state. There is a separate entry (JDK-8148334) for fixing bugs exposed by this test which have not yet been fixed. I did verify that the bugs documented in JDK-8148334 do still exhibit -- so JDK-8148334 should remain open for now.

The test may need to be problem-listed against 8148334 as it's known to fail.

gredler · 2024-11-05T17:48:08Z

The test may need to be problem-listed against 8148334 as it's known to fail.

Should that be part of this PR? If so, do I just add this line to test/jdk/ProblemList.txt?

java/awt/print/PrinterJob/PrintTextTest.java 8148334 generic-all

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java

aivanov-jdk · 2024-11-05T21:10:45Z

The test may need to be problem-listed against 8148334 as it's known to fail.

Should that be part of this PR? If so, do I just add this line to test/jdk/ProblemList.txt?

java/awt/print/PrinterJob/PrintTextTest.java 8148334 generic-all

I think it should. We know the test fails because the printed version differs from the displayed one, there's a bug to track it. Thus, the test should be in the problem list.

Perhaps, the only reason it isn't PL'ed is because the test couldn't be run.

The PL entry line looks good to me.

aivanov-jdk

Looks good to me.

The only things left are problem-listing and updating the switch statement.

gredler · 2024-11-05T23:15:34Z

The only things left are problem-listing and updating the switch statement.

Thanks again for the review, these last points should now be resolved.

aivanov-jdk

Looks good to me.
Thank you.

gredler · 2024-11-06T17:31:07Z

@honkar-jdk Can you approve the PR if there are no further issues from your point of view? Thanks!

honkar-jdk

@gredler Changes LGTM.
Updated JDK-8148334 OS field to "generic" since the issue is observed on multiple platforms and to match the problemlist entry.

gredler · 2024-11-06T18:07:32Z

/integrate

openjdk · 2024-11-06T18:08:13Z

@gredler
Your change (at version 1e8f91f) is now ready to be sponsored by a Committer.

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java

gredler · 2024-11-07T00:26:52Z

/integrate

openjdk · 2024-11-07T00:27:14Z

@gredler This pull request has not yet been marked as ready for integration.

aivanov-jdk

I see you've handled nearly all formatting issues, therefore you should resolve the remaining ones too.

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java

test/jdk/ProblemList.txt

gredler · 2024-11-07T20:05:07Z

/integrate

openjdk · 2024-11-07T20:05:56Z

@gredler
Your change (at version 597ef05) is now ready to be sponsored by a Committer.

gredler · 2024-11-07T20:08:00Z

@honkar-jdk I think this one is ready to go, could you sponsor it?

honkar-jdk · 2024-11-07T20:19:27Z

/sponsor

openjdk · 2024-11-07T20:19:51Z

Going to push as commit 84c99fb.
Since your change was applied there have been 183 commits pushed to the master branch:

2c7bea1: 8343772: Expected IAPE not thrown in KDF.getInstance (TCK)
fac89f4: 8343493: Perform module checks during MetaspaceShared::map_archives()
ccda815: 8342303: Segfault in update_inherited_vtable: AppCDS, old bytecode, and redefineClasses
4f5a241: 8343491: javax/management/remote/mandatory/connection/DeadLockTest.java failing with NoSuchObjectException: no such object in table
56c588b: 8343417: (fs) BasicFileAttributeView.setTimes uses microsecond precision with NOFOLLOW_LINKS
d3c042f: 8343770: Build fails due to use of sun.misc.Unsafe in LoopOverRandom
1d117f6: 8343394: Make MemorySessionImpl.state a stable field
d2b681d: 8343730: JMX cleanups
ac82a8f: 8343610: InOutPathTest jpackage test produces invalid app image on macOS
f0b251d: 8343531: Improve print_location for invalid heap pointers
... and 173 more: https://git.openjdk.org/jdk/compare/0853aee3b377cf9f17340a85f600651db42e6999...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-11-07T20:20:05Z

@honkar-jdk @gredler Pushed as commit 84c99fb.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Modernize java/awt/print/PrinterJob/PrintTextTest.java

f126a30

openjdk bot added the rfr Pull request is ready for review label Oct 25, 2024

openjdk bot added the client client-libs-dev@openjdk.org label Oct 25, 2024

honkar-jdk reviewed Oct 31, 2024

View reviewed changes

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java Outdated Show resolved Hide resolved

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java Outdated Show resolved Hide resolved

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java Outdated Show resolved Hide resolved

Update copyright year, imports, array style, frame title

204f78b

aivanov-jdk suggested changes Nov 5, 2024

View reviewed changes

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java Show resolved Hide resolved

Separate paint logic from test class

c181680

aivanov-jdk suggested changes Nov 5, 2024

View reviewed changes

Use Font constants, addTab(), @OverRide, final and private modifiers

43fda0e

aivanov-jdk suggested changes Nov 5, 2024

View reviewed changes

Use constants in switch, update problem list

1e8f91f

aivanov-jdk approved these changes Nov 6, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 6, 2024

honkar-jdk approved these changes Nov 6, 2024

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Nov 6, 2024

turbanoff reviewed Nov 6, 2024

View reviewed changes

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java Outdated Show resolved Hide resolved

turbanoff reviewed Nov 6, 2024

View reviewed changes

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java Outdated Show resolved Hide resolved

turbanoff reviewed Nov 6, 2024

View reviewed changes

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java Outdated Show resolved Hide resolved

Whitespace fixes

1a110a5

openjdk bot removed sponsor Pull request is ready to be sponsored ready Pull request is ready to be integrated labels Nov 7, 2024

gredler requested a review from turbanoff November 7, 2024 10:25

turbanoff approved these changes Nov 7, 2024

View reviewed changes

gredler requested review from aivanov-jdk and honkar-jdk November 7, 2024 11:01

aivanov-jdk suggested changes Nov 7, 2024

View reviewed changes

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java Outdated Show resolved Hide resolved

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java Outdated Show resolved Hide resolved

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java Outdated Show resolved Hide resolved

aivanov-jdk reviewed Nov 7, 2024

View reviewed changes

test/jdk/java/awt/print/PrinterJob/PrintTextTest.java Show resolved Hide resolved

Whitespace fixes

597ef05

aivanov-jdk approved these changes Nov 7, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 7, 2024

honkar-jdk reviewed Nov 7, 2024

View reviewed changes

test/jdk/ProblemList.txt Show resolved Hide resolved

honkar-jdk approved these changes Nov 7, 2024

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Nov 7, 2024

openjdk bot added the integrated Pull request has been integrated label Nov 7, 2024

openjdk bot closed this Nov 7, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Nov 7, 2024

gredler mentioned this pull request Nov 13, 2024

8344637: Fix Page8 of manual test java/awt/print/PrinterJob/PrintTextTest.java on Linux and Windows #21980

Closed

3 tasks

8283664: Remove jtreg tag manual=yesno for java/awt/print/PrinterJob/PrintTextTest.java #21716

8283664: Remove jtreg tag manual=yesno for java/awt/print/PrinterJob/PrintTextTest.java #21716

Uh oh!

Conversation

gredler commented Oct 25, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Oct 25, 2024

Uh oh!

openjdk bot commented Oct 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Oct 25, 2024

Uh oh!

mlbridge bot commented Oct 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

honkar-jdk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gredler commented Oct 31, 2024

Uh oh!

gredler commented Nov 5, 2024

Uh oh!

Uh oh!

aivanov-jdk commented Nov 5, 2024

Uh oh!

gredler commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aivanov-jdk commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aivanov-jdk left a comment

Choose a reason for hiding this comment

Uh oh!

gredler commented Nov 5, 2024

Uh oh!

aivanov-jdk left a comment

Choose a reason for hiding this comment

Uh oh!

gredler commented Nov 6, 2024

Uh oh!

honkar-jdk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gredler commented Nov 6, 2024

Uh oh!

openjdk bot commented Nov 6, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gredler commented Nov 7, 2024

Uh oh!

openjdk bot commented Nov 7, 2024

Uh oh!

aivanov-jdk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gredler commented Oct 25, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Oct 25, 2024 •

edited

Loading

mlbridge bot commented Oct 25, 2024 •

edited

Loading

gredler commented Nov 5, 2024 •

edited

Loading

aivanov-jdk commented Nov 5, 2024 •

edited

Loading

honkar-jdk left a comment •

edited

Loading