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

Update MockitoTest's build.xml to use the same jdk version as to playlist.xml #5359

Merged
merged 4 commits into from
Jun 3, 2024

Conversation

sophiaxu0424
Copy link
Contributor

@sophiaxu0424 sophiaxu0424 force-pushed the issue3043 branch 5 times, most recently from c749a23 to ff55fe9 Compare May 30, 2024 15:25
Signed-off-by: sophiaxu0424 <[email protected]>
@sophiaxu0424 sophiaxu0424 force-pushed the issue3043 branch 9 times, most recently from 424b497 to a13d16b Compare May 30, 2024 17:37
@sophiaxu0424
Copy link
Contributor Author

sophiaxu0424 commented May 30, 2024

Running with nightly jdk11, and it passed:
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/40988/console

Still need to dive into the case with jdk8 for fixing the 55.0 version error in build.xml

@sophiaxu0424
Copy link
Contributor Author

sophiaxu0424 commented May 30, 2024

Trying to testing on jdk8, but both failed with origin master branch, and this PR branch, expected SKIPPED, but keeps failing.
Testing with origin master branch, nightly jdk8
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/41000/ failed with compile error

Testing with this PR issue3043 branch, nightly jdk8
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/41001/ failed with compile error

@sophiaxu0424 sophiaxu0424 marked this pull request as ready for review May 30, 2024 20:09
@sophiaxu0424
Copy link
Contributor Author

@llxia @LongyuZhang Would you please kindly check and see if other test should be prepared? TIA!

Copy link
Contributor

@LongyuZhang LongyuZhang left a comment

Choose a reason for hiding this comment

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

Since this PR is to fix JDK8 build failure, the grinder test needs to run with JDK8 other tests (reproduce what caused failure in the origin issue) to prove this change fixed it.

@sophiaxu0424
Copy link
Contributor Author

Since this PR is to fix JDK8 build failure, the grinder test needs to run with JDK8 other tests (reproduce what caused failure in the origin issue) to prove this change fixed it.

Hi Longyu,

The original mockito test case with jdk11+ limitations, has compile failure and can't be skipped.
while when we remove the jdk11+ limitation, the reported issue class file has wrong version 55.0, should be 52.0 could be reproduced, and this reported issue came from the mockito (third-party) not our side, and we are waiting for them to fix.

I am wondering that maybe we should add this jdk8 to disabled list to avoid this compile failure? please advice as well. thanks!

@LongyuZhang
Copy link
Contributor

@sophiaxu0424 To validate this changes fixed the JDK8 problem, you can run your branch in a grinder with

  • DYNAMIC_COMPILE set to unchecked;
  • JDK is JDK 8
  • Target is floatSanityTests
    Then make sure this grinder passed, and the console contains info of either MockitoTest skipped or not shown at all. Thanks.

@sophiaxu0424
Copy link
Contributor Author

Thanks Longyu, the required floatSanityTests on jdk8 has passed: https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/41093/

Copy link
Contributor

@llxia llxia left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@LongyuZhang LongyuZhang left a comment

Choose a reason for hiding this comment

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

LGTM

@LongyuZhang LongyuZhang merged commit dbd8bdf into adoptium:master Jun 3, 2024
1 check passed
@sophiaxu0424 sophiaxu0424 deleted the issue3043 branch June 3, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants