This sounds like a neat case for use of feature flags rather than
caching system properties.
On Tue, 31 Dec 2019 at 16:23, Volkan Yazıcı <volkan.yaz...@gmail.com> wrote:
>
> [Thanks so much for the prompt reply @Ralph.]
>
> In the plugin, I have certain behavior triggered by constants:
>
> if (Constants.ENABLE_THREADLOCALS) {
> return toJsonViaTla(logEvent);
> } else {
> return toJsonViaObjectPool(logEvent);
> }
>
> To evaluate each case, I repeat my JUnit 4 tests as follows:
>
> <plugin>
> <groupId>org.apache.maven.plugins</groupId>
> <artifactId>maven-surefire-plugin</artifactId>
> <configuration>
> <skip>true</skip>
> </configuration>
> <executions>
> <!-- TLA-enabled execution -->
> <execution>
> <id>tla-enabled</id>
> <goals>
> <goal>test</goal>
> </goals>
> <configuration>
> <skip>${maven.test.skip}</skip>
> <systemPropertyVariables>
>
> <log4j2.enable.threadlocals>true</log4j2.enable.threadlocals>
> <log4j2.is.webapp>false</log4j2.is.webapp>
> </systemPropertyVariables>
> </configuration>
> </execution>
> <!-- TLA-disabled execution -->
> <execution>
> <id>tla-disabled</id>
> <goals>
> <goal>test</goal>
> </goals>
> <configuration>
> <skip>${maven.test.skip}</skip>
> <systemPropertyVariables>
>
> <log4j2.enable.threadlocals>false</log4j2.enable.threadlocals>
> <log4j2.is.webapp>true</log4j2.is.webapp>
> </systemPropertyVariables>
> </configuration>
> </execution>
> </executions>
> </plugin>
>
> Prior to settling on a solution with maven-surefire-plugin, I tried
> the following failed attempt:
>
> class PluginTest {
>
> @Test
> public void test_tla_triggered_behavior() {
> ...
> }
>
> }
>
> public class TlaEnabledPluginTest extends PluginTest {
>
> static {
> enableTla();
> }
>
> @BeforeClass
> public static void enableTla() {
> System.setProperty("log4j2.enable.threadlocals", "true");
> System.setProperty("log4j2.is.webapp", "false");
> }
>
> }
>
> public class TlaDisabledPluginTest extends PluginTest {
>
> static {
> disableTla();
> }
>
> @BeforeClass
> public static void disableTla() {
> System.setProperty("log4j2.enable.threadlocals", "false");
> System.setProperty("log4j2.is.webapp", "true");
> }
>
> }
>
> Putting the failure of this approach aside, it also another
> shortcoming: one cannot execute individual @Test methods in IDE.
>
> Rather than all these hacks, I wish I would have received the
> TLA-enabled flag in a Constants object placed within
> l.core.config.Configuration that is injected into the plugin builder.
> This way I could have reused my test methods with just different
> Configuration parameters:
>
> if (config.getConstants().isTlaEnabled()) {
> return toJsonViaTla(logEvent);
> } else {
> return toJsonViaObjectPool(logEvent);
> }
>
> public class PluginTest {
>
> @Test
> public void test_tla_triggered_behavior() {
>
> Configuration tlaEnabledConfiguration = new
> DefaultConfiguration();
> tlaEnabledConfiguration.getConstants().setTlaEnabled(true);
> test_tla_triggered_behavior(tlaEnabledConfiguration);
>
> Configuration tlaDisabledConfiguration = new
> DefaultConfiguration();
> tlaEnabledConfiguration.getConstants().setTlaEnabled(false);
> test_tla_triggered_behavior(tlaDisabledConfiguration);
> }
>
> private void test_tla_triggered_behavior(Cofiguration config) {
> ...
> }
>
> }
>
> Is this sufficient to demonstrate my case?
>
> On Tue, Dec 31, 2019 at 10:52 PM Ralph Goers <ralph.go...@dslextreme.com>
> wrote:
> >
> > Can you provide a PR that demonstrates what you would like to do? It is
> > easier for me to evaluate the pros and cons of that then guessing what the
> > implementation would look like.
> >
> > Ralph
> >
> > > On Dec 31, 2019, at 2:43 PM, Volkan Yazıcı <volkan.yaz...@gmail.com>
> > > wrote:
> > >
> > > A majority of the Log4j 2.0 constants are structured by means of
> > > "public final" fields initialized via system properties. While this
> > > serves great for accessibility, IMHO, becomes an obstacle for tests.
> > > For instance, I have needed two maven-surefire-plugin executions for
> > > log4j2-logstash-layout: one with TLA enabled and another one with TLA
> > > disabled. I have tried invoking System.setProperty() to influence
> > > these constants for tests via both "static { }" blocks and
> > > @BeforeClass methods, but neither kicks in earlier than the Log4j
> > > constant class initialization itself. Is it just me that couldn't get
> > > it working or this approach indeed hampers testing? If the latter, for
> > > Log4j 3.0, I propose using injection to pass configuration state
> > > rather than accessing it via globals.
> > >
> >
> >
--
Matt Sicker <boa...@gmail.com>
