-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix test directive in i18533 #19802
Fix test directive in i18533 #19802
Conversation
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.
oops, I didn't check this syntax before, but it seems PR CI only runs java 17 anyway. So this is the correct syntax looking at e.g. tests/pos/t12396/A_1.java
0d5eb86
to
c463c84
Compare
@@ -1,5 +1,4 @@ | |||
//> using javacOpt --enable-preview --source 17 | |||
//> test: -jvm 17+ |
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.
Apparently, we should only put one of them if tests are in a folder
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.
"historically", a "compilation group" for tests foo_1.scala
, bar_1.scala
get the same options because they are compiled together, so only one is needed per group. (Not sure if de-duplication is needed for anything.)
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 would expect to drop duplicate directives or raise an error if they're directives that contradicts each other.
c463c84
to
3e99d90
Compare
tests/neg/i18533.check
Outdated
@@ -1,8 +1,8 @@ | |||
-- Error: tests/neg/i18533/Non-SCALA_ONLY.java:4:7 --------------------------------------------------------------------- |
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.
The order is important here (case sensitivity), since it's failing on Windows...
3e99d90
to
687264a
Compare
687264a
to
84024e9
Compare
Backports #19802 to the LTS branch. PR submitted by the release tooling. [skip ci]
[test_windows_full]