Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8274465: Mark javax/swing/text/ParagraphView/6364882/bug6364882.java as headful #5744

Closed
wants to merge 1 commit into from

Conversation

DamonFool
Copy link
Member

@DamonFool DamonFool commented Sep 28, 2021

Hi all,

javax/swing/text/ParagraphView/6364882/bug6364882.java was observed failing on our non-GUI platforms.
So I guess it should be marked as headful.

Thanks.
Best regards,
Jie


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8274465: Mark javax/swing/text/ParagraphView/6364882/bug6364882.java as headful

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5744/head:pull/5744
$ git checkout pull/5744

Update a local copy of the PR:
$ git checkout pull/5744
$ git pull https://git.openjdk.java.net/jdk pull/5744/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5744

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5744.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Sep 28, 2021

👋 Welcome back jiefu! 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 openjdk bot added the rfr Pull request is ready for review label Sep 28, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Sep 28, 2021

@DamonFool 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.

@openjdk openjdk bot added the client client-libs-dev@openjdk.org label Sep 28, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Sep 28, 2021

Webrevs

@mrserb
Copy link
Member

@mrserb mrserb commented Sep 29, 2021

What exception is caused the test failure? This test was updated to be headless in the #5661

@DamonFool
Copy link
Member Author

@DamonFool DamonFool commented Sep 29, 2021

#5661

This test passed on our GUI platforms but failed without GUI.

STDERR:
java.lang.reflect.InvocationTargetException
        at java.desktop/java.awt.EventQueue.invokeAndWait(EventQueue.java:1371)
        at java.desktop/java.awt.EventQueue.invokeAndWait(EventQueue.java:1346)
        at java.desktop/javax.swing.SwingUtilities.invokeAndWait(SwingUtilities.java:1480)
        at bug6364882.main(bug6364882.java:83)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
        at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.AssertionError: paragraph doesn't have 6 rows of text
        at bug6364882.checkJustification(bug6364882.java:137)
        at bug6364882.lambda$main$0(bug6364882.java:87)
        at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:308)
        at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:773)
        at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:720)
        at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:714)
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
        at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
        at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:742)
        at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
        at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
        at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
        at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
        at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
        at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

@mrserb
Copy link
Member

@mrserb mrserb commented Sep 29, 2021

This is a plain test failure, since exception is not a HeadlessException caused by some API required the desktop session.
headful key should not apply to it.

@DamonFool
Copy link
Member Author

@DamonFool DamonFool commented Sep 29, 2021

This is a plain test failure, since exception is not a HeadlessException caused by some API required the desktop session. headful key should not apply to it.

So any idea to fix it?
Thanks.

@mrserb
Copy link
Member

@mrserb mrserb commented Sep 29, 2021

It is possible that the test is too strict or this is a jdk bug. But I am not sure it is possible to investigate this issue based on the bug description in the JBS.
@aivanov-jdk please take a look.

@DamonFool
Copy link
Member Author

@DamonFool DamonFool commented Sep 29, 2021

It is possible that the test is too strict or this is a jdk bug. But I am not sure it is possible to investigate this issue based on the bug description in the JBS. @aivanov-jdk please take a look.

Can I problemlist this test?
Then we'll have enough time to investigate the root cause.
Thanks.

@aivanov-jdk
Copy link
Member

@aivanov-jdk aivanov-jdk commented Sep 29, 2021

@DamonFool could you attach the image to the bug report?

The image editorPane.png is automatically saved for analysis if the test fails, you should find in it the test artefacts along with the jtreg test log.

Could you elaborate on what our non-GUI platforms are? What is the OS? What is the CPU?

@aivanov-jdk
Copy link
Member

@aivanov-jdk aivanov-jdk commented Sep 29, 2021

It is possible that the test is too strict or this is a jdk bug. But I am not sure it is possible to investigate this issue based on the bug description in the JBS. @aivanov-jdk please take a look.

It could be too strict to the width of the fourth line of text. Yet it's not the case here. If the font size is tiny, it could be possible that the long text fits in less lines than four.

The image is required for analysis. And it looks the image isn't saved if assertion error is thrown.

@aivanov-jdk
Copy link
Member

@aivanov-jdk aivanov-jdk commented Sep 29, 2021

The image editorPane.png is automatically saved for analysis if the test fails, you should find in it the test artefacts along with the jtreg test log.

It is not saved on AssertionError — my bad.
I updated the test so that it saves the editorPane.png image if any exception occurs.

Could you please take the updated version of bug6364882.java from my JDK-8274465 branch or copy the bug6364882.java file? Then re-run the test and attach the image. Please also provide the console output of the test, it will contain the view dumps as well as the font used.

@DamonFool
Copy link
Member Author

@DamonFool DamonFool commented Sep 29, 2021

The image editorPane.png is automatically saved for analysis if the test fails, you should find in it the test artefacts along with the jtreg test log.

It is not saved on AssertionError — my bad. I updated the test so that it saves the editorPane.png image if any exception occurs.

Could you please take the updated version of bug6364882.java from my JDK-8274465 branch or copy the bug6364882.java file? Then re-run the test and attach the image. Please also provide the console output of the test, it will contain the view dumps as well as the font used.

Good!
Thanks @aivanov-jdk for your help.

My colleague @xpbob would like to follow this issue.
We agree with you that the test shouldn't be headful.

Since it's near our National Day, many of us would go on vacation.
He may reply your questions either tonight or the day after tomorrow.
Thanks.

@aivanov-jdk
Copy link
Member

@aivanov-jdk aivanov-jdk commented Sep 29, 2021

There are relevant comments raised by @prrace in #5661.

For convenience, I'm posting them here:

@prrace said:
I've looked at the test a tad more and two comments

  1. if it fails, it dumps an image. You can provide that image
  2. It is sensitive to the actual font. If your default font for whatever reason is of a size thatonemore or one less word will fit on a line, then it will fail. This does look like a test problem to me.

@aivanov-jdk replied:

Thank you, @prrace, for your comments.

  1. Unfortunately, it doesn't save the image if an exception is thrown. I overlooked this fact.
  2. I agree I should've made it more flexible to accommodate for possible font size differences.

I'll fix these issues and create my own PR for JDK-8274465.

@aivanov-jdk
Copy link
Member

@aivanov-jdk aivanov-jdk commented Sep 29, 2021

@DamonFool, I have created a new PR #5761 which should fix the test failure.

Could you please test whether it fixes the failure?

Please, also perform the previous experiment first and attach the image of the failure to JBS.

@DamonFool
Copy link
Member Author

@DamonFool DamonFool commented Sep 29, 2021

Will discuss it in #5761 .
So close this one.

@DamonFool DamonFool closed this Sep 29, 2021
@DamonFool
Copy link
Member Author

@DamonFool DamonFool commented Sep 30, 2021

@DamonFool, I have created a new PR #5761 which should fix the test failure.

Could you please test whether it fixes the failure?

Please, also perform the previous experiment first and attach the image of the failure to JBS.

The png has been uploaded in the JBS.
Now I am testing your new patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client client-libs-dev@openjdk.org rfr Pull request is ready for review
3 participants