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

8199079: Test javax/swing/UIDefaults/6302464/bug6302464.java is unstable #3537

Closed
wants to merge 2 commits into from

Conversation

@prsadhuk
Copy link
Contributor

@prsadhuk prsadhuk commented Apr 16, 2021

This test was marked as unstable in CI run in samevm mode but later on was found passing in now-default othervm mode but was problemlisted only on macos by JDK-8254976.
It is found that this test started failing in macos due to JDK-8220150: [macos] macos10.14 Mojave returns anti-aliased glyphs instead of aliased B&W glyphs
which turns Antialias hint even if it is set to off from macosx10.14 onwards.
So, this test started failing when it tries to test Antialias OFF hint.

Proposed fix is to check for macosx 10.14 and greater version and bail out from this subtest.

@prrace Can you please take a look?


Progress

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

Issue

  • JDK-8199079: Test javax/swing/UIDefaults/6302464/bug6302464.java is unstable

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3537

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

Using diff file

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

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Apr 16, 2021

👋 Welcome back psadhukhan! 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 label Apr 16, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Apr 16, 2021

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

  • swing

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 swing label Apr 16, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Apr 16, 2021

Webrevs

@prsadhuk
Copy link
Contributor Author

@prsadhuk prsadhuk commented Apr 20, 2021

Any reviewers please?

}
}
if (!isMacOSX14) {
HashSet colorsAAOff = getAntialiasedColors(VALUE_TEXT_ANTIALIAS_OFF, 100);
Copy link
Contributor

@prrace prrace Apr 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens on macOS 11 ?

Copy link
Contributor Author

@prsadhuk prsadhuk Apr 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it also works in macos11

Copy link
Contributor

@prrace prrace Apr 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So on macOS 11 we enter this block since isMacOSX14 is false and the test passes ?

Copy link
Contributor Author

@prsadhuk prsadhuk Apr 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it still fails in macosx11 without this modification. macos11, System .getProperty returns 10.16 so it satisfies this macosx >=10.14 condition and it bails out from this subtest.

Copy link
Contributor

@prrace prrace Apr 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you are up to date ? There was a fix so that it returns 11

My build on my machine :
~/jdk.git/build/macosx-x86_64-server-release/jdk/bin/jshell
| Welcome to JShell -- Version 17-internal
| For an introduction type: /help intro

jshell> System.getProperty("os.version")
$1 ==> "11.2.3"

Copy link
Contributor Author

@prsadhuk prsadhuk Apr 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the repo was a bit outdated on BigSur system. I have updated the fix to check BigSur OS too.

prrace
prrace approved these changes Apr 21, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Apr 21, 2021

@prsadhuk 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:

8199079: Test javax/swing/UIDefaults/6302464/bug6302464.java is unstable

Reviewed-by: prr

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 106 new commits pushed to the master branch:

  • da86029: 8265326: Strange Characters in G1GC GC Log
  • 7879adb: 8265343: Update Debian-based cross-compilation recipes
  • 98cb81b: 8265237: String.join and StringJoiner can be improved further
  • ed477da: 8264945: Optimize the code-gen for Math.pow(x, 0.5)
  • 7146104: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement
  • b5c92ca: 8265106: IGV: Enforce en-US locale while parsing ideal graph
  • 3de0dcb: 8265483: All-caps “JAVA” in the top navigation bar
  • 739769c: 8265411: Avoid unnecessary Method::init_intrinsic_id calls
  • a22ad03: 8264983: Add gtest for JDK-8264008
  • 91b08b7: 8261779: JCK test api/javax_crypto/EncryptedPrivateKeyInfo/Ctor4.html is failing with assertion error when assertions enabled
  • ... and 96 more: https://git.openjdk.java.net/jdk/compare/2b5869ad09e8f34277a4c965abbaed3f79fc5b59...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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready label Apr 21, 2021
@prrace
Copy link
Contributor

@prrace prrace commented Apr 21, 2021

I guess this will work - until Apple come out with 12.0 when it will break again.
I'd have preferred to see you convert the first part of the string - up to before any 2nd decimal pt
into a float and then just do a comparison.
Not sure if the jtreg OSUtils have something to help.

@prsadhuk
Copy link
Contributor Author

@prsadhuk prsadhuk commented Apr 21, 2021

I guess this will work - until Apple come out with 12.0 when it will break again.
I'd have preferred to see you convert the first part of the string - up to before any 2nd decimal pt
into a float and then just do a comparison.
Not sure if the jtreg OSUtils have something to help.

Yes, could have. Next time for sure...We will have time till 12.0 to work on it...BTW, I could not find OSUtils in our repo..

@prsadhuk
Copy link
Contributor Author

@prsadhuk prsadhuk commented Apr 21, 2021

/integrate

@openjdk openjdk bot closed this Apr 21, 2021
@openjdk openjdk bot added integrated and removed ready labels Apr 21, 2021
@prsadhuk prsadhuk deleted the JDK-8199079 branch Apr 21, 2021
@openjdk openjdk bot removed the rfr label Apr 21, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Apr 21, 2021

@prsadhuk Since your change was applied there have been 108 commits pushed to the master branch:

  • 45c474a: 8168408: Test java/awt/Focus/ActualFocusedWindowTest/ActualFocusedWindowBlockingTest.java fails intermittentently on windows
  • 18ee419: 8198422: Test java/awt/font/StyledMetrics/BoldSpace.java is unstable
  • da86029: 8265326: Strange Characters in G1GC GC Log
  • 7879adb: 8265343: Update Debian-based cross-compilation recipes
  • 98cb81b: 8265237: String.join and StringJoiner can be improved further
  • ed477da: 8264945: Optimize the code-gen for Math.pow(x, 0.5)
  • 7146104: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement
  • b5c92ca: 8265106: IGV: Enforce en-US locale while parsing ideal graph
  • 3de0dcb: 8265483: All-caps “JAVA” in the top navigation bar
  • 739769c: 8265411: Avoid unnecessary Method::init_intrinsic_id calls
  • ... and 98 more: https://git.openjdk.java.net/jdk/compare/2b5869ad09e8f34277a4c965abbaed3f79fc5b59...master

Your commit was automatically rebased without conflicts.

Pushed as commit 41fc7dd.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants