Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[#noissue] Simplify OnClassLoader state #10860

Merged
merged 1 commit into from
Apr 8, 2024
Merged

[#noissue] Simplify OnClassLoader state #10860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ...est-module/plugins-test/src/main/java/com/navercorp/pinpoint/test/plugin/ClassLoding.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading