jcoglan opened a new pull request, #3256:
URL: https://github.com/apache/logging-log4j2/pull/3256
Hi :wave: this is James from Neighbourhoodie, working on the STF agreed
milestone to migrate the test suite to JUnit 5.
This PR demonstrates an attempt to get a set of tests that use
`@Parameterized` from JUnit 4 and try to get them running on JUnit 5. We have
run into an odd problem where logger config files seem to be ignored and we're
not sure how to resolve it.
The class `DefaultRouteScriptAppenderTest` is parameterised like so:
```java
@RunWith(Parameterized.class)
public class DefaultRouteScriptAppenderTest {
@Parameterized.Parameters(name = "{0} {1}")
public static Object[][] getParameters() {
return new Object[][] {
{"log4j-routing-default-route-script-groovy.xml", false},
{"log4j-routing-default-route-script-javascript.xml", false},
{"log4j-routing-script-staticvars-javascript.xml", true},
{"log4j-routing-script-staticvars-groovy.xml", true},
};
}
// ...
}
```
We need to replicate the effect of this parameterisation under JUnit 5. Our
understanding is that, whereas in JUnit 4 it is the _constructor_ that is
parameterised, in JUnit 5 the `@ParameterizedTest` annotation applies to
individual test methods.
The class also uses `LoggerContextRule`, and makes use of the API of that
class to inspect the loggers/appenders that are configured. Our understanding
is that this has been replaced with the class-level `@LoggerContextSource`
annotation, which causes a `LoggerContext` to be passed into the constructor.
This PR shows an idea we had for refactoring this test class to JUnit 5.
Rather than label each of its individual tests as parameterised, and then have
to invoke code to set up a `LoggerContext` and the necessary before/after hooks
in each one, we observe that parameterisation can be achieved by subclassing.
If the `DefaultRouteScriptAppenderTest` constructor accepts the parameters that
define each "version" of the tests, then we can write subclasses of
`DefaultRouteScriptAppenderTest` that each invoke their parent's constructor
with different inputs.
Since the parameterisation is expressed in the form of classes, we can then
also use `@LoggerContextSource` on each of those classes to turn the config
filename into a `LoggerContext` implicitly and not have to write/call any
additional setup code ourselves.
In the first commit we remove the `@Parameterized.Parameters` block and add
several subclasses, each of which sets up the parent class with one of the
parameter sets, e.g.
```java
public class DefaultRouteScriptGroovyAppenderTest extends
DefaultRouteScriptAppenderTest {
public DefaultRouteScriptGroovyAppenderTest() {
super("log4j-routing-default-route-script-groovy.xml", false);
}
}
```
This works fine and the tests continue to pass.
In the next commit, we attempt to migrate `DefaultRouteScriptAppenderTest`
to JUnit 5, and as part of that we remove `LoggerContextRule` from that class
and add `LoggerContextSource` to the subclasses:
```java
@LoggerContextSource("log4j-routing-default-route-script-groovy.xml")
public class DefaultRouteScriptGroovyAppenderTest extends
DefaultRouteScriptAppenderTest {
public DefaultRouteScriptGroovyAppenderTest(LoggerContext context) {
super(context, false);
}
}
```
We have tried to migrate the use of `LoggerContextRule` methods like
`getListAppender()` to their equivalents on `LoggerContext`. However, these
tests no longer pass; e.g. this is the result of running `mvn test
-Dtest=ScriptStaticVarsJavaScriptAppenderTest`:
```
```
[ERROR] Tests run: 8, Failures: 1, Errors: 6, Skipped: 0, Time elapsed:
0.377 s <<< FAILURE! -- in
org.apache.logging.log4j.core.appender.routing.ScriptStaticVarsJavaScriptAppenderTest
[ERROR]
org.apache.logging.log4j.core.appender.routing.ScriptStaticVarsJavaScriptAppenderTest.testListAppenderPresence
-- Time elapsed: 0.012 s <<< ERROR!
java.lang.NullPointerException: Cannot invoke
"org.apache.logging.log4j.core.appender.routing.RoutingAppender.getAppenders()"
because the return value of
"org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.getRoutingAppender()"
is null
at
org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.testListAppenderPresence(DefaultRouteScriptAppenderTest.java:103)
at
java.base/java.lang.reflect.Method.invoke(Method.java:568)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at
java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
[ERROR]
org.apache.logging.log4j.core.appender.routing.ScriptStaticVarsJavaScriptAppenderTest.testNoPurgePolicy
-- Time elapsed: 0.002 s <<< ERROR!
java.lang.NullPointerException: Cannot invoke
"org.apache.logging.log4j.core.appender.routing.RoutingAppender.getPurgePolicy()"
because the return value of
"org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.getRoutingAppender()"
is null
at
org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.testNoPurgePolicy(DefaultRouteScriptAppenderTest.java:109)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
[ERROR]
org.apache.logging.log4j.core.appender.routing.ScriptStaticVarsJavaScriptAppenderTest.testRoutingPresence1
-- Time elapsed: 0.007 s <<< ERROR!
java.lang.NullPointerException: Cannot invoke
"org.apache.logging.log4j.core.appender.routing.RoutingAppender.isStarted()"
because "routingAppender" is null
at
org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.getListAppender(DefaultRouteScriptAppenderTest.java:69)
at
org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.logAndCheck(DefaultRouteScriptAppenderTest.java:85)
at
org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.testRoutingPresence1(DefaultRouteScriptAppenderTest.java:133)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
[ERROR]
org.apache.logging.log4j.core.appender.routing.ScriptStaticVarsJavaScriptAppenderTest.testRoutingPresence2
-- Time elapsed: 0.001 s <<< ERROR!
java.lang.NullPointerException: Cannot invoke
"org.apache.logging.log4j.core.appender.routing.RoutingAppender.isStarted()"
because "routingAppender" is null
at
org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.getListAppender(DefaultRouteScriptAppenderTest.java:69)
at
org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.logAndCheck(DefaultRouteScriptAppenderTest.java:85)
at
org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.testRoutingPresence2(DefaultRouteScriptAppenderTest.java:139)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
[ERROR]
org.apache.logging.log4j.core.appender.routing.ScriptStaticVarsJavaScriptAppenderTest.testNotListAppender
-- Time elapsed: 0.001 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: not <null>
at
org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.testNotListAppender(DefaultRouteScriptAppenderTest.java:96)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
[ERROR]
org.apache.logging.log4j.core.appender.routing.ScriptStaticVarsJavaScriptAppenderTest.testNoRewritePolicy
-- Time elapsed: 0 s <<< ERROR!
java.lang.NullPointerException: Cannot invoke
"org.apache.logging.log4j.core.appender.routing.RoutingAppender.getRewritePolicy()"
because the return value of
"org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.getRoutingAppender()"
is null
at
org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.testNoRewritePolicy(DefaultRouteScriptAppenderTest.java:115)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
[ERROR]
org.apache.logging.log4j.core.appender.routing.ScriptStaticVarsJavaScriptAppenderTest.testRoutingAppenderDefaultRouteKey
-- Time elapsed: 0.001 s <<< ERROR!
java.lang.NullPointerException: Cannot invoke
"org.apache.logging.log4j.core.appender.routing.RoutingAppender.getDefaultRouteScript()"
because "routingAppender" is null
at
org.apache.logging.log4j.core.appender.routing.DefaultRouteScriptAppenderTest.testRoutingAppenderDefaultRouteKey(DefaultRouteScriptAppenderTest.java:121)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
```
It seems as though the config filename passed to `@LoggerContextSource` is
being ignored -- changing the referenced files' content has no effect on the
test outcome, and using a filename for a file that does not exist does not
cause a different kind of failure. It just looks like the file is ignored
entirely, and we're not sure why.
Is there a quirk of `@LoggerContextSource` that means it doesn't work with
subclassing, or something else we're not aware of? Should we be going about
this in a different way, and if so how would you recommend turning a config
filename into a working `LoggerContext`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
