Skip to content

Commit

Permalink
[#noissue] Simplify OnClassLoader state
Browse files Browse the repository at this point in the history
  • Loading branch information
@emeroad
emeroad committed Apr 8, 2024
1 parent bc3b67a commit e4e8639
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 71 deletions.
6 changes: 6 additions & 0 deletions ...est-module/plugins-test/src/main/java/com/navercorp/pinpoint/test/plugin/ClassLoding.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package com.navercorp.pinpoint.test.plugin;

public enum ClassLoding {
System,
Child
}
15 changes: 9 additions & 6 deletions ...est/src/main/java/com/navercorp/pinpoint/test/plugin/DefaultPluginForkedTestInstance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Scanner;

import static com.navercorp.pinpoint.test.plugin.PluginTestConstants.CHILD_CLASS_PATH_PREFIX;
Expand All @@ -33,14 +34,16 @@ public class DefaultPluginForkedTestInstance implements PluginForkedTestInstance
private final PluginForkedTestContext context;
private final String testId;
private final List<String> libs;
private final boolean onSystemClassLoader;
private final ClassLoding type;
private final ProcessManager processManager;

public DefaultPluginForkedTestInstance(PluginForkedTestContext context, String testId, List<String> libs, boolean onSystemClassLoader) {
public DefaultPluginForkedTestInstance(PluginForkedTestContext context,
String testId, List<String> libs,
ClassLoding type) {
this.context = context;
this.testId = testId + ":" + (onSystemClassLoader ? "system" : "child") + ":" + context.getJvmVersion();
this.testId = testId + ":" + type + ":" + context.getJvmVersion();
this.libs = libs;
this.onSystemClassLoader = onSystemClassLoader;
this.type = Objects.requireNonNull(type, "type");
this.processManager = new DefaultProcessManager(context);
}

Expand All @@ -51,7 +54,7 @@ public String getTestId() {

@Override
public List<String> getClassPath() {
if (onSystemClassLoader) {
if (type == ClassLoding.System) {
List<String> libs = new ArrayList<>(context.getRequiredLibraries());
libs.addAll(this.libs);
libs.add(context.getTestClassLocation());
Expand All @@ -78,7 +81,7 @@ public List<String> getAppArgs() {

args.add(context.getTestClass().getName());

if (!onSystemClassLoader) {
if (type == ClassLoding.Child) {
StringBuilder classPath = new StringBuilder();
classPath.append(CHILD_CLASS_PATH_PREFIX);

Expand Down
50 changes: 16 additions & 34 deletions ...s-test/src/main/java/com/navercorp/pinpoint/test/plugin/DefaultPluginForkedTestSuite.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ public class DefaultPluginForkedTestSuite extends AbstractPluginForkedTestSuite
private static final Map<String, Object> RESOLVER_OPTION = createResolverOption();
private static final DependencyResolverFactory RESOLVER_FACTORY = new DependencyResolverFactory(RESOLVER_OPTION);
private final TaggedLogger logger = TestLogger.getLogger();
private final boolean testOnSystemClassLoader;
private final boolean testOnChildClassLoader;

private final ClassLoding classLoding;

private final String[] repositories;
private final String[] dependencies;

Expand All @@ -73,16 +74,7 @@ public DefaultPluginForkedTestSuite(Class<?> testClass, boolean sharedProcess) {
super(testClass);

OnClassLoader onClassLoader = testClass.getAnnotation(OnClassLoader.class);
if (onClassLoader == null) {
this.testOnChildClassLoader = true;
} else {
this.testOnChildClassLoader = onClassLoader.child();
}
if (onClassLoader == null) {
this.testOnSystemClassLoader = false;
} else {
this.testOnSystemClassLoader = onClassLoader.system();
}
this.classLoding = getClassLoding(onClassLoader);

Set<String> dependenySet = new HashSet<>();
Dependency deps = testClass.getAnnotation(Dependency.class);
Expand Down Expand Up @@ -119,6 +111,14 @@ public DefaultPluginForkedTestSuite(Class<?> testClass, boolean sharedProcess) {
this.sharedProcess = sharedProcess;
}

private ClassLoding getClassLoding(OnClassLoader onClassLoader) {
if (onClassLoader == null) {
return ClassLoding.Child;
} else {
return onClassLoader.type();
}
}

@Override
protected List<PluginForkedTestInstance> createTestCases(PluginForkedTestContext context) throws Exception {
if (dependencies != null) {
Expand Down Expand Up @@ -170,33 +170,15 @@ private DependencyResolver getDependencyResolver(String[] repositories) {
}

private PluginForkedTestInstance newSharedProcessPluginTestCase(PluginForkedTestContext context, String testId, List<String> libs, SharedProcessManager sharedProcessManager) {
if (testOnSystemClassLoader) {
if (classLoding == ClassLoding.System) {
return new SharedPluginForkedTestInstance(context, testId, libs, true, sharedProcessManager);
}
if (testOnChildClassLoader) {
return new SharedPluginForkedTestInstance(context, testId, libs, false, sharedProcessManager);
}
throw new IllegalStateException("Illegal classLoader");
return new SharedPluginForkedTestInstance(context, testId, libs, false, sharedProcessManager);
}

private List<PluginForkedTestInstance> createCasesWithJdkOnly(PluginForkedTestContext context) throws ClassNotFoundException {
List<PluginForkedTestInstance> instanceList = new ArrayList<>();

if (testOnSystemClassLoader) {
instanceList.add(new DefaultPluginForkedTestInstance(context, "", Collections.emptyList(), true));
} else {
instanceList.add(new DefaultPluginForkedTestInstance(context, "", Collections.emptyList(), false));
}
return instanceList;
DefaultPluginForkedTestInstance testInstance = new DefaultPluginForkedTestInstance(context, "", Collections.emptyList(), classLoding);
return Collections.singletonList(testInstance);
}

boolean isOnSystemClassLoader() {
if (testOnSystemClassLoader) {
return true;
}
if (testOnChildClassLoader) {
return false;
}
return false;
}
}
39 changes: 15 additions & 24 deletions ...plugins-test/src/main/java/com/navercorp/pinpoint/test/plugin/DefaultPluginTestSuite.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ public class DefaultPluginTestSuite extends AbstractPluginTestSuite {
private static final Map<String, Object> RESOLVER_OPTION = createResolverOption();
private static final DependencyResolverFactory RESOLVER_FACTORY = new DependencyResolverFactory(RESOLVER_OPTION);
private final TaggedLogger logger = TestLogger.getLogger();
private final boolean testOnSystemClassLoader;
private final boolean testOnChildClassLoader;

private final ClassLoding classLoding;

private final String[] repositories;
private final String[] dependencies;
private final Class<?> sharedClass;
Expand All @@ -72,16 +73,7 @@ public DefaultPluginTestSuite(Class<?> testClass, boolean sharedProcess) {
super(testClass);

OnClassLoader onClassLoader = testClass.getAnnotation(OnClassLoader.class);
if (onClassLoader == null) {
this.testOnChildClassLoader = true;
} else {
this.testOnChildClassLoader = onClassLoader.child();
}
if (onClassLoader == null) {
this.testOnSystemClassLoader = false;
} else {
this.testOnSystemClassLoader = onClassLoader.system();
}
this.classLoding = getClassLoding(onClassLoader);

Dependency deps = testClass.getAnnotation(Dependency.class);
this.dependencies = deps == null ? null : deps.value();
Expand All @@ -97,6 +89,14 @@ public DefaultPluginTestSuite(Class<?> testClass, boolean sharedProcess) {
this.testClassName = testClass.getName();
}

private ClassLoding getClassLoding(OnClassLoader onClassLoader) {
if (onClassLoader == null) {
return ClassLoding.Child;
} else {
return onClassLoader.type();
}
}

@Override
public PluginSharedInstance createSharedInstance(PluginTestContext context) {
if (sharedClass == null) {
Expand Down Expand Up @@ -182,7 +182,8 @@ private List<PluginTestInstance> createCasesWithDependencies(PluginTestContext c
agentClassLoader.setTransformIncludeList(context.getTransformIncludeList());
try {
thread.setContextClassLoader(agentClassLoader);
final PluginTestInstance pluginTestInstance = pluginTestInstanceFactory.create(currentClassLoader, testId, agentClassLoader, libs, context.getTransformIncludeList(), isOnSystemClassLoader());
final PluginTestInstance pluginTestInstance = pluginTestInstanceFactory.create(currentClassLoader, testId, agentClassLoader,
libs, context.getTransformIncludeList(), classLoding);
pluginTestInstanceList.add(pluginTestInstance);
} finally {
thread.setContextClassLoader(currentClassLoader);
Expand All @@ -204,22 +205,12 @@ private DependencyResolver getDependencyResolver(String[] repositories) {
private List<PluginTestInstance> createCasesWithJdkOnly(PluginTestContext context) throws ClassNotFoundException {
final PluginTestInstanceFactory pluginTestInstanceFactory = new PluginTestInstanceFactory(context);
ClassLoader contextClassLoader = ClassLoaderUtils.getContextClassLoader();
boolean onSystemClassLoader = isOnSystemClassLoader();
List<String> libs = Collections.emptyList();
List<String> transformIncludeList = Collections.emptyList();
final PluginTestInstance pluginTestInstance = pluginTestInstanceFactory.create(contextClassLoader, "", null, libs, transformIncludeList, onSystemClassLoader);
final PluginTestInstance pluginTestInstance = pluginTestInstanceFactory.create(contextClassLoader, "", null, libs, transformIncludeList, classLoding);
final List<PluginTestInstance> pluginTestInstanceList = new ArrayList<>();
pluginTestInstanceList.add(pluginTestInstance);
return pluginTestInstanceList;
}

boolean isOnSystemClassLoader() {
if (testOnSystemClassLoader) {
return true;
}
if (testOnChildClassLoader) {
return false;
}
return false;
}
}
3 changes: 1 addition & 2 deletions ...t-module/plugins-test/src/main/java/com/navercorp/pinpoint/test/plugin/OnClassLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,5 @@
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
public @interface OnClassLoader {
boolean system() default false;
boolean child() default true;
ClassLoding type() default ClassLoding.Child;
}
15 changes: 10 additions & 5 deletions ...gins-test/src/main/java/com/navercorp/pinpoint/test/plugin/PluginTestInstanceFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,25 @@
import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

public class PluginTestInstanceFactory {

private final PluginTestContext context;

public PluginTestInstanceFactory(PluginTestContext context) {
this.context = context;
this.context = Objects.requireNonNull(context, "context");
}

public PluginTestInstance create(ClassLoader parentClassLoader, String testId, PluginAgentTestClassLoader agentClassLoader, List<String> libs, List<String> transformIncludeList, boolean onSystemClassLoader) throws ClassNotFoundException {
final String id = testId + ":" + (onSystemClassLoader ? "system" : "child");
public PluginTestInstance create(ClassLoader parentClassLoader, String testId,
PluginAgentTestClassLoader agentClassLoader,
List<String> libs,
List<String> transformIncludeList,
ClassLoding classLoading) throws ClassNotFoundException {
final String id = testId + ":" + classLoading;
PluginTestInstanceCallback instanceContext = startAgent(context.getConfigFile(), agentClassLoader);
final List<File> fileList = new ArrayList<>();
for (String classPath : getClassPath(libs, onSystemClassLoader)) {
for (String classPath : getClassPath(libs, classLoading)) {
File file = new File(classPath);
fileList.add(file);
}
Expand All @@ -55,7 +60,7 @@ public PluginTestInstance create(ClassLoader parentClassLoader, String testId, P
return new DefaultPluginTestInstance(id, testClassLoader, testClass, context.isManageTraceObject(), instanceContext);
}

List<String> getClassPath(List<String> libs, boolean onSystemClassLoader) {
List<String> getClassPath(List<String> libs, ClassLoding classLoading) {
final List<String> libList = new ArrayList<>(context.getJunitLibList());
libList.addAll(libs);
libList.add(context.getTestClassLocation());
Expand Down

0 comments on commit e4e8639

Please sign in to comment.