Copilot commented on code in PR #16231:
URL: https://github.com/apache/dubbo/pull/16231#discussion_r3144606678


##########
dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoaderTest.java:
##########
@@ -882,4 +907,95 @@ public int getPriority() {
             return MAX_PRIORITY;
         }
     }
+
+    private <T> ExtensionLoaderTestContext<T> 
createExtensionLoaderTestContext(Class<T> type) {
+        FrameworkModel frameworkModel = new FrameworkModel();
+        ApplicationModel applicationModel = frameworkModel.newApplication();
+        return new ExtensionLoaderTestContext<>(
+                frameworkModel, 
applicationModel.getExtensionDirector().getExtensionLoader(type));
+    }
+
+    private static final class ExtensionLoaderTestContext<T> implements 
AutoCloseable {
+        private final FrameworkModel frameworkModel;
+        private final ExtensionLoader<T> extensionLoader;
+
+        private ExtensionLoaderTestContext(FrameworkModel frameworkModel, 
ExtensionLoader<T> extensionLoader) {
+            this.frameworkModel = frameworkModel;
+            this.extensionLoader = extensionLoader;
+        }
+
+        @Override
+        public void close() {
+            frameworkModel.destroy();
+        }
+    }
+
+    private static final class LogCollector implements AutoCloseable {
+        private final LoggerContext context;
+        private final Configuration configuration;
+        private final String loggerName;
+        private final LoggerConfig loggerConfig;
+        private final TestAppender appender;
+        private final LoggerConfig preLoggerConfig;
+
+        private LogCollector(
+                LoggerContext context,
+                Configuration configuration,
+                String loggerName,
+                LoggerConfig loggerConfig,
+                TestAppender appender,
+                LoggerConfig preLoggerConfig) {
+            this.context = context;
+            this.configuration = configuration;
+            this.loggerName = loggerName;
+            this.loggerConfig = loggerConfig;
+            this.appender = appender;
+            this.preLoggerConfig = preLoggerConfig;
+        }
+
+        static LogCollector attach(Class<?> loggerType) {
+            LoggerContext context = LoggerContext.getContext(false);
+            Configuration configuration = context.getConfiguration();
+            String loggerName = loggerType.getName();
+            TestAppender appender = new TestAppender("test-appender-" + 
loggerType.getSimpleName());
+            appender.start();
+            configuration.addAppender(appender);
+
+            LoggerConfig preLoggerConfig = 
configuration.getLoggerConfig(loggerName);
+            LoggerConfig loggerConfig = new LoggerConfig(loggerName, 
org.apache.logging.log4j.Level.DEBUG, false);
+            loggerConfig.addAppender(appender, 
org.apache.logging.log4j.Level.DEBUG, null);
+            configuration.addLogger(loggerName, loggerConfig);
+            context.updateLoggers();
+            return new LogCollector(context, configuration, loggerName, 
loggerConfig, appender, preLoggerConfig);
+        }
+
+        boolean contains(String expected) {
+            return appender.messages.stream().anyMatch(message -> 
message.contains(expected));
+        }
+
+        @Override
+        public void close() {
+            loggerConfig.removeAppender(appender.getName());
+            configuration.removeLogger(loggerName);
+            appender.stop();
+            configuration.getAppenders().remove(appender.getName());
+            if (preLoggerConfig != null) {
+                configuration.addLogger(loggerName, preLoggerConfig);
+            }
+            context.updateLoggers();

Review Comment:
   `preLoggerConfig` is obtained via 
`configuration.getLoggerConfig(loggerName)`, which can return a *parent* (or 
the root) `LoggerConfig` when no explicit config exists for `loggerName`. 
Re-adding that `LoggerConfig` under `loggerName` in `close()` can corrupt the 
configuration (same LoggerConfig instance mapped to a different name) and cause 
cross-test side effects. Instead, capture the previous explicit mapping (e.g., 
`configuration.getLoggers().get(loggerName)`), and on close either restore that 
explicit config (if it existed) or simply remove the temporary logger and leave 
the parent/root config untouched.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to