C0urante commented on code in PR #13182:
URL: https://github.com/apache/kafka/pull/13182#discussion_r1123292197
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/TestPlugins.java:
##########
@@ -111,20 +115,62 @@ public enum TestPlugin {
/**
* A plugin which shares a jar file with {@link
TestPlugin#MULTIPLE_PLUGINS_IN_JAR_THING_ONE}
*/
- MULTIPLE_PLUGINS_IN_JAR_THING_TWO("multiple-plugins-in-jar",
"test.plugins.ThingTwo");
+ MULTIPLE_PLUGINS_IN_JAR_THING_TWO("multiple-plugins-in-jar",
"test.plugins.ThingTwo"),
+ /**
+ * A plugin which is incorrectly packaged, and is missing a superclass
definition.
+ */
+ FAIL_TO_INITIALIZE_MISSING_SUPERCLASS("fail-to-initialize",
"test.plugins.MissingSuperclass", false, REMOVE_CLASS_FILTER),
+ /**
+ * A plugin which is packaged with other incorrectly packaged plugins,
but itself has no issues loading.
+ */
+ FAIL_TO_INITIALIZE_CO_LOCATED("fail-to-initialize",
"test.plugins.CoLocatedPlugin", true, REMOVE_CLASS_FILTER),
+ /**
+ * A connector which is incorrectly packaged, and throws during static
initialization.
+ */
+
FAIL_TO_INITIALIZE_STATIC_INITIALIZER_THROWS_CONNECTOR("fail-to-initialize",
"test.plugins.StaticInitializerThrowsConnector", false, REMOVE_CLASS_FILTER),
+ /**
+ * A plugin which is incorrectly packaged, which throws an exception
from the {@link Versioned#version()} method.
+ */
+
FAIL_TO_INITIALIZE_VERSION_METHOD_THROWS_CONNECTOR("fail-to-initialize",
"test.plugins.VersionMethodThrowsConnector", false, REMOVE_CLASS_FILTER),
Review Comment:
I think `BAD_PACKAGING` is probably the best here.
I want to make sure that whoever reads these tests in the future isn't
actively misled by anything in them. It's less important that the
descriptions/variable names perfectly capture intent, and more important that
they don't imply something incorrect.
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java:
##########
@@ -439,6 +457,22 @@ public static <T> String versionFor(Class<? extends T>
pluginKlass) throws Refle
versionFor(pluginKlass.getDeclaredConstructor().newInstance()) :
UNDEFINED_VERSION;
}
+ private static String reflectiveErrorDescription(Throwable t) {
+ if (t instanceof NoSuchMethodException) {
+ return ": Plugin class must have a default constructor, and cannot
be a non-static inner class";
+ } else if (t instanceof SecurityException) {
+ return ": Security settings must allow reflective instantiation of
plugin classes";
+ } else if (t instanceof IllegalAccessException) {
+ return ": Plugin class default constructor must be public";
+ } else if (t instanceof ExceptionInInitializerError) {
+ return ": Plugin class should not throw exception during static
initialization";
+ } else if (t instanceof InvocationTargetException) {
+ return ": Constructor must complete without throwing an exception";
+ } else {
+ return "";
Review Comment:
This is fine, thanks 👍
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
