You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/12/04 01:59:16 UTC
[GitHub] [pulsar] Shoothzj opened a new pull request #13131: Make tests can run on jdk17+
Shoothzj opened a new pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131
### Motivation
- make jdk17 can run tests
### Modifications
- add jdk17 profile to open modules
### Documentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
- [x] `no-need-doc`
test changes, no need doc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi commented on a change in pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on a change in pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#discussion_r762410409
##########
File path: pom.xml
##########
@@ -1814,6 +1814,29 @@ flexible messaging model and an intuitive client API.</description>
</pluginManagement>
</build>
</profile>
+ <profile>
+ <id>jdk17</id>
+ <activation>
+ <jdk>[17,)</jdk>
Review comment:
+1 for not adding new profile, to me it's useful only if something not related only to the tests is different
##########
File path: pom.xml
##########
@@ -1814,6 +1814,29 @@ flexible messaging model and an intuitive client API.</description>
</pluginManagement>
</build>
</profile>
+ <profile>
+ <id>jdk17</id>
+ <activation>
+ <jdk>[17,)</jdk>
Review comment:
+1 for not adding a new profile, to me it's useful only if something not related only to the tests is different
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#issuecomment-985993875
/pulsarbot run-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj closed pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
Shoothzj closed pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a change in pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
lhotari commented on a change in pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#discussion_r762406940
##########
File path: pom.xml
##########
@@ -1814,6 +1814,29 @@ flexible messaging model and an intuitive client API.</description>
</pluginManagement>
</build>
</profile>
+ <profile>
+ <id>jdk17</id>
+ <activation>
+ <jdk>[17,)</jdk>
Review comment:
How is it different for JDK17? It's better to combine if we can. There will be new JDK versions coming frequently and we don't want to add a new profile for each one of them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#issuecomment-986005077
> @Shoothzj I'm working on this too. You can add other modules to open here [nicoloboschi@2894cfa](https://github.com/nicoloboschi/pulsar/commit/2894cfab0e66cd73d9a326e9b0e46d38cfe05d8d)
>
> They are all for enabling PowerMock on some tests.
You go farther than me, I will close this PR. Thanks @nicoloboschi , @lhotari
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#issuecomment-985955929
/pulsarbot run-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a change in pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on a change in pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#discussion_r762409691
##########
File path: pom.xml
##########
@@ -1814,6 +1814,29 @@ flexible messaging model and an intuitive client API.</description>
</pluginManagement>
</build>
</profile>
+ <profile>
+ <id>jdk17</id>
+ <activation>
+ <jdk>[17,)</jdk>
Review comment:
Currently we can combine this to a line, should I combine this ? We can only add profile for jdk LTS and latest version. cc @codelipenghui
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi commented on pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#issuecomment-986003765
@Shoothzj I'm working on this too. You can add other modules to open here https://github.com/nicoloboschi/pulsar/commit/2894cfab0e66cd73d9a326e9b0e46d38cfe05d8d
They are all for enabling PowerMock on some tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on a change in pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#discussion_r762372240
##########
File path: pom.xml
##########
@@ -1814,6 +1814,29 @@ flexible messaging model and an intuitive client API.</description>
</pluginManagement>
</build>
</profile>
+ <profile>
+ <id>jdk17</id>
+ <activation>
+ <jdk>[17,)</jdk>
+ </activation>
+ <properties>
+ <maven.compiler.release>${maven.compiler.target}</maven.compiler.release>
+ <!-- required for running tests on JDK17+ -->
+ <test.additional.args> --add-opens java.base/java.lang=ALL-UNNAMED </test.additional.args>
Review comment:
What are the components that are failing with Java17? We should probably fixed these instead of just enabling this here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a change in pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on a change in pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#discussion_r762409691
##########
File path: pom.xml
##########
@@ -1814,6 +1814,29 @@ flexible messaging model and an intuitive client API.</description>
</pluginManagement>
</build>
</profile>
+ <profile>
+ <id>jdk17</id>
+ <activation>
+ <jdk>[17,)</jdk>
Review comment:
Currently I can combine this to a line, should I combine this ? We can only add profile for jdk LTS and latest version. cc @codelipenghui
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a change in pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on a change in pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#discussion_r762406525
##########
File path: pom.xml
##########
@@ -1814,6 +1814,29 @@ flexible messaging model and an intuitive client API.</description>
</pluginManagement>
</build>
</profile>
+ <profile>
+ <id>jdk17</id>
+ <activation>
+ <jdk>[17,)</jdk>
Review comment:
In my opinion, it doesn’t add much complexity, but it’s clearer than putting it together. (what's jdk 11 need, what's jdk17 need)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#issuecomment-985950005
@merlimat The error is that `powermockito` can't instance it's test class anymore. Like this
```
An error occurred while instantiating class org.apache.pulsar.broker.intercept.BrokerInterceptorUtilsTest: Cannot create a new instance of test class class org.apache.pulsar.broker.intercept.BrokerInterceptorUtilsTest
at org.testng.internal.InstanceCreator.createInstanceUsingObjectFactory(InstanceCreator.java:123)
at org.testng.internal.InstanceCreator.createInstance(InstanceCreator.java:79)
at org.testng.internal.ClassImpl.getDefaultInstance(ClassImpl.java:109)
at org.testng.internal.ClassImpl.getInstances(ClassImpl.java:167)
at org.testng.TestClass.getInstances(TestClass.java:102)
at org.testng.TestClass.initTestClassesAndInstances(TestClass.java:82)
at org.testng.TestClass.init(TestClass.java:74)
at org.testng.TestClass.<init>(TestClass.java:39)
at org.testng.TestRunner.initMethods(TestRunner.java:457)
at org.testng.TestRunner.init(TestRunner.java:336)
at org.testng.TestRunner.init(TestRunner.java:289)
at org.testng.TestRunner.<init>(TestRunner.java:180)
at org.testng.SuiteRunner$DefaultTestRunnerFactory.newTestRunner(SuiteRunner.java:613)
at org.testng.SuiteRunner.init(SuiteRunner.java:178)
at org.testng.SuiteRunner.<init>(SuiteRunner.java:112)
at org.testng.TestNG.createSuiteRunner(TestNG.java:1306)
at org.testng.TestNG.createSuiteRunners(TestNG.java:1282)
at org.testng.TestNG.runSuitesLocally(TestNG.java:1131)
at org.testng.TestNG.runSuites(TestNG.java:1069)
at org.testng.TestNG.run(TestNG.java:1037)
at com.intellij.rt.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:66)
at com.intellij.rt.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:109)
Caused by: java.lang.RuntimeException: Cannot create a new instance of test class class org.apache.pulsar.broker.intercept.BrokerInterceptorUtilsTest
at org.powermock.modules.testng.internal.TestClassInstanceFactory.create(TestClassInstanceFactory.java:56)
at org.powermock.modules.testng.internal.PowerMockClassloaderObjectFactory.newInstance(PowerMockClassloaderObjectFactory.java:46)
at org.powermock.modules.testng.PowerMockObjectFactory.newInstance(PowerMockObjectFactory.java:43)
at org.testng.internal.InstanceCreator.instantiateUsingDefaultConstructor(InstanceCreator.java:193)
at org.testng.internal.InstanceCreator.createInstanceUsingObjectFactory(InstanceCreator.java:113)
... 21 more
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make protected native java.lang.Object java.lang.Object.clone() throws java.lang.CloneNotSupportedException accessible: module java.base does not "opens java.lang" to unnamed module @192b07fd
at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199)
at java.base/java.lang.reflect.Method.setAccessible(Method.java:193)
at org.powermock.reflect.internal.WhiteboxImpl.doGetAllMethods(WhiteboxImpl.java:1508)
at org.powermock.reflect.internal.WhiteboxImpl.getAllMethods(WhiteboxImpl.java:1482)
at org.powermock.reflect.internal.WhiteboxImpl.findMethodOrThrowException(WhiteboxImpl.java:862)
at org.powermock.reflect.internal.WhiteboxImpl.doInvokeMethod(WhiteboxImpl.java:822)
at org.powermock.reflect.internal.WhiteboxImpl.invokeMethod(WhiteboxImpl.java:690)
at org.powermock.reflect.Whitebox.invokeMethod(Whitebox.java:401)
at org.powermock.modules.testng.internal.TestClassInstanceFactory.createProxyTestClass(TestClassInstanceFactory.java:85)
at org.powermock.modules.testng.internal.TestClassInstanceFactory.createTestClass(TestClassInstanceFactory.java:75)
at org.powermock.modules.testng.internal.TestClassInstanceFactory.create(TestClassInstanceFactory.java:46)
... 25 more
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#issuecomment-985978059
@codelipenghui @hangc0276 @315157973 @michaeljmarshall @lhotari @eolivelli PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a change in pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
lhotari commented on a change in pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#discussion_r762406010
##########
File path: pom.xml
##########
@@ -1814,6 +1814,29 @@ flexible messaging model and an intuitive client API.</description>
</pluginManagement>
</build>
</profile>
+ <profile>
+ <id>jdk17</id>
+ <activation>
+ <jdk>[17,)</jdk>
Review comment:
Instead of adding a separate profile for JDK17, couldn't we update the existing JDK11 profile to cover both? /Cc @eolivelli @nicoloboschi
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj removed a comment on pull request #13131: Make tests can run on jdk17+
Posted by GitBox <gi...@apache.org>.
Shoothzj removed a comment on pull request #13131:
URL: https://github.com/apache/pulsar/pull/13131#issuecomment-985955929
/pulsarbot run-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org