-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[Forward testing] Test JDK 24-ea build 24 (2024/11/14) #23501
base: master
Are you sure you want to change the base?
Conversation
9f6711d
to
5fc99f9
Compare
5fc99f9
to
9d011de
Compare
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.
Looks good to go and run for testing..
Note to others ... we wont merge this ...
a834a55
to
b97f836
Compare
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.
We will have to adjust this over time .. so not ready to merge for a while .. but important for updating and monitoring progress towards 24
6492488
to
12a1333
Compare
12a1333
to
3dad9e2
Compare
Since the ASM update has landed. Now this doesn't require any code/infrastructure changes |
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.
mark as draft
@losipiuk draft doesn't run CI, right? |
It didn’t when the feature launched, but it does now. |
3dad9e2
to
d2384d3
Compare
d2384d3
to
375add6
Compare
375add6
to
7fc457c
Compare
Updated to beta 20 |
35ab167
to
a23c7d9
Compare
f214df6
to
8fa6213
Compare
@@ -465,12 +465,12 @@ public void testTimeWithTimeZone() | |||
Time.valueOf( | |||
LocalTime.of(15, 39, 7)).getTime() /* 15:39:07 = 00:39:07 - +01:00 shift + Bahia_Banderas's shift (-8) (modulo 24h which we "un-modulo" below) */ | |||
- DAYS.toMillis(1) /* because we use currently 'shifted' representation, not possible to create just using LocalTime */ | |||
+ HOURS.toMillis(1) /* because there was offset shift on 1970-01-01 in America/Bahia_Banderas */); |
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.
Political timezones don't affect time w/ tz. These comments are no longer applicable.
@@ -101,7 +101,7 @@ public void testCurrentDateTimezone() | |||
millis += timeIncrement) { | |||
Instant instant = Instant.ofEpochMilli(millis); | |||
assertCurrentDateAtInstant(kievTimeZoneKey, instant); | |||
assertCurrentDateAtInstant(bahiaBanderasTimeZoneKey, instant); | |||
assertCurrentDateAtInstant(bajaSurTimeZoneKey, instant); |
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.
No sure what this test buys us. I would just drop it.
@@ -397,7 +397,7 @@ public void testLastDayOfMonth() | |||
assertThat(assertions.function("last_day_of_month", "TIMESTAMP '2001-08-22 03:04:05.321 +07:09'")) | |||
.matches("DATE '2001-08-31'"); | |||
|
|||
ImmutableList.of("+05:45", "+00:00", "-05:45", "Asia/Tokyo", "Europe/London", "America/Los_Angeles", "America/Bahia_Banderas").forEach(timeZone -> { | |||
ImmutableList.of("+05:45", "+00:00", "-05:45", "Asia/Tokyo", "Europe/London", "America/Los_Angeles", "Mexico/BajaSur").forEach(timeZone -> { |
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.
Same here
2a9d995
to
5d973f1
Compare
f1b284d
to
5b72102
Compare
5b72102
to
5f1e3dd
Compare
6600d1c
to
d9c43b2
Compare
private static Subject current() | ||
{ | ||
try { | ||
Method current = Subject.class.getDeclaredMethod("current"); | ||
return (Subject) current.invoke(null); | ||
} | ||
catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException ignored) { | ||
// Fallback to pre-Java 17 method | ||
return Subject.getSubject(AccessController.getContext()); | ||
} | ||
} |
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.
Maybe it's time to bump the Java version from the ancient 8 to something less ancient in trino-client
?
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.
(Moved to #24197 (comment))
350bf60
to
21f07ad
Compare
055e705
to
35a3c7f
Compare
Fixes #23498
https://jdk.java.net/24/
Description
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: