You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/08/07 13:18:03 UTC
[GitHub] [geode] mivanac opened a new pull request, #7832: GEODE-10407: check if class allready loaded
mivanac opened a new pull request, #7832:
URL: https://github.com/apache/geode/pull/7832
<!-- Thank you for submitting a contribution to Apache Geode. -->
<!-- In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
-->
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
<!-- Note:
Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
-->
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] mivanac merged pull request #7832: GEODE-10407: check if class allready loaded
Posted by GitBox <gi...@apache.org>.
mivanac merged PR #7832:
URL: https://github.com/apache/geode/pull/7832
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] albertogpz commented on a diff in pull request #7832: GEODE-10407: check if class allready loaded
Posted by GitBox <gi...@apache.org>.
albertogpz commented on code in PR #7832:
URL: https://github.com/apache/geode/pull/7832#discussion_r952726733
##########
geode-deployment/geode-deployment-legacy/src/integrationTest/java/org/apache/geode/internal/deployment/JarDeployerIntegrationTest.java:
##########
@@ -202,6 +208,26 @@ public void undeploy() throws Exception {
.isInstanceOf(ClassNotFoundException.class);
}
+ @Test
+ public void deploy2JarsSetCurrentClassloaderTo1stAndLoadClassFrom2nd() throws Exception {
+
+ // deploy def-1.0.jar
+ // deploy base.jar
Review Comment:
I would move this comment to right before the code statement.
##########
geode-deployment/geode-deployment-legacy/src/integrationTest/java/org/apache/geode/internal/deployment/JarDeployerIntegrationTest.java:
##########
@@ -216,4 +242,27 @@ private static String createClassContent(String version, String functionName) {
+ "public void execute(FunctionContext context) {context.getResultSender().lastResult(\""
+ version + "\");}}";
}
+
+ private static String create1ClassContent(String functionName1) {
Review Comment:
functionName1 should be className1.
##########
geode-deployment/geode-deployment-legacy/src/integrationTest/java/org/apache/geode/internal/deployment/JarDeployerIntegrationTest.java:
##########
@@ -216,4 +242,27 @@ private static String createClassContent(String version, String functionName) {
+ "public void execute(FunctionContext context) {context.getResultSender().lastResult(\""
+ version + "\");}}";
}
+
+ private static String create1ClassContent(String functionName1) {
+ return "package jddunit.function1;"
+ + "public class "
+ + functionName1 + " extends Exception {"
+ + "private static final long serialVersionUID = 1L;"
+ + "public " + functionName1 + "(String message) {"
+ + " super(message);"
+ + "}"
+ + "}";
+ }
+
+ private static String create2ClassContent(String functionName1, String functionName2) {
Review Comment:
functionName1 should be className1 and functionName2 should be className2
##########
geode-deployment/geode-deployment-legacy/src/integrationTest/java/org/apache/geode/internal/deployment/JarDeployerIntegrationTest.java:
##########
@@ -202,6 +208,26 @@ public void undeploy() throws Exception {
.isInstanceOf(ClassNotFoundException.class);
}
+ @Test
+ public void deploy2JarsSetCurrentClassloaderTo1stAndLoadClassFrom2nd() throws Exception {
+
+ // deploy def-1.0.jar
+ // deploy base.jar
+ jarDeployer.deploy(semanticJarVersion1);
+ DeployedJar deployedJar = jarDeployer.deploy(baseJar);
+
+ assertThat(deployedJar).isNotNull();
+
+ ClassPathLoader oldLoader = ClassPathLoader.getLatest();
+
+ ClassLoader cl = oldLoader.getClassloaderForArtifact("def"); // set current classloader to 1st
+
+ cl.loadClass("jddunit.function2.ExceptionB"); // load extended class and base class
+ cl.loadClass("jddunit.function1.ExceptionA"); // load base class
Review Comment:
I think it is best to have the comments before the code statements. Otherwise, if the line of code grows, the comment may be split in several lines.
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org