Copilot commented on code in PR #6355:
URL: https://github.com/apache/ignite-3/pull/6355#discussion_r2250744275
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/LogInspector.java:
##########
@@ -313,11 +314,15 @@ public void stop() {
appender.stop();
removeAppender(appender, config);
+
+ config = null;
}
private static synchronized void addAppender(Appender appender,
Configuration config) {
for (LoggerConfig loggerConfig : config.getLoggers().values()) {
- loggerConfig.addAppender(appender, null, null);
+ if (!loggerConfig.isAdditive()) {
+ loggerConfig.addAppender(appender, null, null);
+ }
Review Comment:
The condition `!loggerConfig.isAdditive()` will prevent additive loggers
from having the appender added. This appears incorrect - typically you would
want to add appenders to additive loggers, not non-additive ones. Consider
removing the negation or clarifying the intended behavior.
```suggestion
loggerConfig.addAppender(appender, null, null);
```
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/LogInspector.java:
##########
@@ -313,11 +314,15 @@ public void stop() {
appender.stop();
removeAppender(appender, config);
+
+ config = null;
Review Comment:
Setting `config = null` in the `stop()` method could cause
NullPointerException if `stop()` is called multiple times or if other methods
try to access `config` after stopping. Consider adding a null check or using a
boolean flag to track the stopped state.
```suggestion
config = null;
started = false;
```
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
