apalan60 commented on code in PR #19920:
URL: https://github.com/apache/kafka/pull/19920#discussion_r2134771638
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/LoggersTest.java:
##########
@@ -41,12 +45,19 @@ public class LoggersTest {
private static final long INITIAL_TIME = 1696951712135L;
private Loggers.Log4jLoggers loggers;
private Time time;
+ Level originalRootLevel;
@BeforeEach
public void setup() {
+ originalRootLevel = LogManager.getRootLogger().getLevel();
time = new MockTime(0, INITIAL_TIME, 0);
loggers = (Loggers.Log4jLoggers) Loggers.newInstance(time);
}
+
+ @AfterEach
+ public void teardown() {
+ Configurator.setAllLevels(LogManager.ROOT_LOGGER_NAME,
originalRootLevel);
Review Comment:
@chia7712
Thank you for catching this!
After rethinking the solution, I confirmed that calling `setLevel` in
LoggersTest will overwrite every child loggers (see
[[1]](https://github.com/apache/kafka/blob/666571216bc87f528e1496f4b9d5c557afb9c62d/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Loggers.java#L189)).
That means any test in `LoggersTest` that invokes `setLevel` risks leaking its
changes into other test classes.
Instead of recording and restoring each logger's previous level, reloading
the Log4j configuration after each test may be a simpler solution. This ensures
that any global Log4j configuration changes made in `LoggersTest` are fully
reset, and that the restored configuration matches what other tests would see
when run independently, which maintains proper test isolation.
I’ve updated the teardown to call
`LoggerContext.getContext(false).reconfigure()`. Please let me know if you have
any further feedback!
--
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]
