WiP: Run all tests on JDK 17 #10004
WiP: Run all tests on JDK 17 #10004
Conversation
wendigo
force-pushed
the
serafin/jdk17-tests
branch
3 times, most recently
from
eef0cf5 to
341aedd
Compare
wendigo
force-pushed
the
serafin/jdk17-tests
branch
from
db5ca40 to
71d8f96
Compare
wendigo
force-pushed
the
serafin/jdk17-tests
branch
from
65fc26e to
935ba34
Compare
wendigo
force-pushed
the
serafin/jdk17-tests
branch
from
935ba34 to
4b4b1b4
Compare
wendigo
force-pushed
the
serafin/jdk17-tests
branch
from
4b4b1b4 to
cda6617
Compare
wendigo
force-pushed
the
serafin/jdk17-tests
branch
from
cda6617 to
8cf618d
Compare
wendigo
force-pushed
the
serafin/jdk17-tests
branch
from
8cf618d to
821a235
Compare
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-surefire-plugin</artifactId> | ||
| <configuration> | ||
| <!-- This is required for Phoenix tests to work --> |
There was a problem hiding this comment.
This is required by the JVM in which Phoenix runs - correct? Trino isn't the one performing these accesses?
There was a problem hiding this comment.
Actually it's used by the phoenix client so these flags will be also required when we switch runtime requirement to 17
There was a problem hiding this comment.
Thanks, we'll need to mention this in release notes then when we switch to JDK 17 (by merging this PR)
wendigo
force-pushed
the
serafin/jdk17-tests
branch
2 times, most recently
from
9431392 to
a0fe8c8
Compare
|
Can you rebase/squash and drop the “Speed up resolved function” commit (which seems good but unrelated)? |
| @@ -407,7 +407,7 @@ public boolean isNull(ResultSet resultSet, int columnIndex) | |||
| public long readLong(ResultSet resultSet, int columnIndex) | |||
| throws SQLException | |||
| { | |||
| return resultSet.getObject(columnIndex, LocalDate.class).toEpochDay(); | |||
| return requireNonNull(resultSet.getObject(columnIndex, LocalDate.class)).toEpochDay(); | |||
There was a problem hiding this comment.
If we’re expecting an NPE with a specific message, let’s change the test, or modify this code to throw a better exception (maybe SQLException with a known error message).
| */ | ||
| package org.apache.phoenix.shaded.org.apache.zookeeper.client; | ||
|
|
||
| /** |
| */ | ||
| package org.apache.phoenix.shaded.org.apache.zookeeper.client; | ||
|
|
||
| /** |
| ZULU_17("/usr/lib/jvm/zulu-17", List.of( | ||
| "-Xmx2G", | ||
| "-Xmx2304M", |
There was a problem hiding this comment.
This seems oddly specific. Add a comment?
| @@ -20,9 +20,14 @@ | |||
| ZULU_11("/usr/lib/jvm/zulu-11", List.of( | |||
| "-Xmx2G", | |||
| "-XX:+UseG1GC", | |||
There was a problem hiding this comment.
Remove since parallel GC overrides
| @@ -1818,6 +1818,27 @@ | |||
| </build> | |||
|
|
|||
| <profiles> | |||
| <profile> | |||
There was a problem hiding this comment.
Please move this to the Phoenix POMs. We don’t want to enable this for everything. It can allow regressions.
|
Please re-order the commits so the enable one comes after the fixes — allowing bisect to work. |
wendigo
force-pushed
the
serafin/jdk17-tests
branch
from
a0fe8c8 to
f02c527
Compare
No description provided.