TestLoggerFactoryExtension may not work with every test setup

[jamietanna]

I've got an issue with a codebase I work on where the registration of the TestLoggerFactoryExtension leads to failing tests.

The scenario in question incluides a number of @Nested tests, with structure like so:

@AfterEach() {
  TestLoggerFactory.clear();
}

@Nested
class Something {
  @BeforeEach
    void setup() {
      sut.doSomethingThatLogs();
    }

  @Test
    void verifyMessage() {
      assertThat(testLogger.(...).getMessage()).isEqualTo("An error message");
    }


  @Test
    void verifyArgument() {
      assertThat(testLogger.(...).getArguments()).contains(...);
    }
}

@Nested
class Another {
  // ...
}

I'm currently trying different options for the callback class (after/before) but it doesn't seem to be working, so I'm instead needing to exclude this extension from being auto-registered.

Edit: This could just be a quirk of how we're doing things, but this has definitely worked with the code we've got above, with the old library 🤔

[valfirst]

That's weird, the actual number of changes in the "production" code between 2.1.1 and 2.2.0 is really small: slf4j-test-2.1.1...slf4j-test-2.2.0.

@jamietanna Do you have any sample project to share in order to reproduce the issue?

[jamietanna]

Oh sorry I meant the old project as in pre fork

[jamietanna]

Minimal example:

package com.github.valfirst.slf4jtest;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static com.github.valfirst.slf4jtest.Assertions.assertThat;

@ExtendWith(TestLoggerFactoryExtension.class)
class NestedExample {

    private static final TestLogger TEST_LOGGER = TestLoggerFactory.getTestLogger(ClassUnderTest.class);

    @AfterEach
    void tearDown() {
        TestLoggerFactory.clear();
    }

    @Nested
    class Something {
        @BeforeEach
        void setup() {
            ClassUnderTest.doTheThing();
        }

        @Test
        void info() {
            assertThat(TEST_LOGGER).hasLogged(LoggingEvent.info("The message"));
        }

        @Test
        void error() {
            assertThat(TEST_LOGGER).hasLogged(LoggingEvent.error("Oh no!"));
        }
    }

    public static class ClassUnderTest {
        private static final Logger LOGGER = LoggerFactory.getLogger(ClassUnderTest.class);

        public static void doTheThing() {
            LOGGER.info("The message");
            LOGGER.error("Oh no!");
        }
    }
}

[karstenspang]

The TestLoggerFactoryExtension is a BeforeTestExecutionCallback, which is executed after the @BeforeEach method. This has nothing to do with nested test classes.

Changing it into a BeforeEachCallback would solve this issue, since such callbacks are called before the @BeforeEach method. I think that it makes sense to do so, as you would probably want to reset anything first, and then set up your test in the @BeforeEach.

[valfirst]

I was thinking about adding flag to the annotation, which will specify lifecycle step at which clean up should happen

[karstenspang]

Could be an idea.

If you want it to behave exactly like the JUnit 4 rule, it should implement both @BeforeEachCallback and @AfterEachCallback, clearing the test factory in both methods.