-
-
Notifications
You must be signed in to change notification settings - Fork 312
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 ProblemList_openjdk17.txt with jvm_compiler excludes for windows-x86" #3006
Conversation
@King-kay - can you please put a more descriptive title so that its easier to know the contents of the change? Something like: "Update ProblemList_openjdk17.txt with jvm_compiler excludes for windows-x86" |
Grinder for testing: https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/1707/ |
i've added the exclusion statement for the new tests |
for windows-x86 platform
compiler/codegen/ClearArrayTest.java windows-x86 | ||
compiler/c2/TestJumpTable.java windows-x86 | ||
compiler/loopstripmining/TestPinnedUseInOuterLSMUnusedBySfpt.java windows-x86 | ||
compiler/vectorapi/TestNoInline.java windows-x86 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see the format in the other lines of the file,
nameOfClassToExclude URLofIssueFailedTestIsTrackedBy platformsOnWhichToExclude
so your changes on these lines need to also have the issue URL in them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the URL
did I do it properly this time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes @King-kay - each line that you have added appears to have a className anIssueURL and a platform (3 things per line), which is the proper and expected format for these files, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change looks good, but because others have also been changing this same file, you will need to resolve the conflicts. To solve the conflicts please do git pull --rebase
and push your PR again.
@King-kay Could you do the rebase as suggested by @smlambert #3006 (review) ? Thanks. |
I already did @sophia-guo |
@King-kay there may be other changes merged. Looks like there are still conflicts. |
#runtime_nestmate | ||
runtime/Nestmates/protectionDomain/TestDifferentProtectionDomains.java https://bugs.openjdk.java.net/browse/JDK-8269135 windows-x86 | ||
|
||
>>>>>>> master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line must come from some conflicts. If you could remove it that will be perfect. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, i took it out, does it look okay now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @King-kay . LGTM
@@ -332,6 +332,32 @@ jdk/incubator/vector/Short64VectorTests.java https://github.com/adoptium/aqa-tes | |||
jdk/incubator/vector/ShortMaxVectorTests.java https://github.com/adoptium/aqa-tests/issues/2874 linux-arm | |||
############################################################################ | |||
|
|||
#runtime_nestmate | |||
runtime/Nestmates/protectionDomain/TestDifferentProtectionDomains.java https://bugs.openjdk.java.net/browse/JDK-8269135 windows-x86 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this exclude line be removed? thought this PR was just to add the jvm_compiler excludes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought you asked it to be removed, as it was from a later pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, okay. Hard to keep things straight with this many PRs. :)
Fix #3004
"Update ProblemList_openjdk17.txt with jvm_compiler excludes for windows-x86"