chesnokoff commented on code in PR #11615:
URL: https://github.com/apache/ignite/pull/11615#discussion_r1825648205
##########
modules/core/src/main/java/org/apache/ignite/logger/java/JavaLogger.java:
##########
@@ -134,7 +138,7 @@ public class JavaLogger implements IgniteLoggerEx {
* Creates new logger.
*/
public JavaLogger() {
- this(!isConfigured());
+ this(true);
Review Comment:
if isConfigured() == true then we also should initialize it for setting cfg
variable
##########
modules/log4j2/src/test/java/org/apache/ignite/logger/log4j2/Log4j2ConfigUpdateTest.java:
##########
@@ -147,6 +153,45 @@ private void checkConfigUpdate(Log4J2LoggerSupplier
logSupplier) throws Exceptio
Log4J2Logger.cleanup();
}
+ /**
+ * Checks that Log4JLogger is updated after configuration file is changed.
+ * @param logSupplier Function returning a logger instance to be tested.
+ */
+ @SuppressWarnings("ResultOfMethodCallIgnored")
+ private void checkDifferentInstances(Log4J2LoggerSupplier logSupplier)
throws Exception {
+ Log4J2Logger.cleanup();
+
+ File infoCfgFile = new File(U.getIgniteHome(), LOG_CONFIG_INFO);
+ File debugCfgFile = new File(U.getIgniteHome(), LOG_CONFIG_DEBUG);
+ File logFile = new File(U.getIgniteHome(), LOG_DEST);
+
+ System.out.println("INFO config: " + infoCfgFile);
+ System.out.println("DEBUG config: " + debugCfgFile);
+ System.out.println("Log file: " + logFile);
+
+ if (logFile.delete())
+ System.out.println("Old log file was deleted.");
+
+ Log4J2Logger log = logSupplier.get(infoCfgFile);
+ assertTrue(log.toString().contains(infoCfgFile.getPath()));
+
+ log.info("Accepted info");
+ log.debug("Ignored debug");
+
+ log = logSupplier.get(debugCfgFile);
Review Comment:
Configuration of log is still infoCfgFile because cfg field is inited only
once. That's why "Accepted debug" won't be logged
##########
modules/log4j2/src/test/java/org/apache/ignite/logger/log4j2/Log4j2ConfigUpdateTest.java:
##########
@@ -147,6 +153,45 @@ private void checkConfigUpdate(Log4J2LoggerSupplier
logSupplier) throws Exceptio
Log4J2Logger.cleanup();
}
+ /**
+ * Checks that Log4JLogger is updated after configuration file is changed.
+ * @param logSupplier Function returning a logger instance to be tested.
+ */
+ @SuppressWarnings("ResultOfMethodCallIgnored")
+ private void checkDifferentInstances(Log4J2LoggerSupplier logSupplier)
throws Exception {
+ Log4J2Logger.cleanup();
+
+ File infoCfgFile = new File(U.getIgniteHome(), LOG_CONFIG_INFO);
+ File debugCfgFile = new File(U.getIgniteHome(), LOG_CONFIG_DEBUG);
+ File logFile = new File(U.getIgniteHome(), LOG_DEST);
+
+ System.out.println("INFO config: " + infoCfgFile);
+ System.out.println("DEBUG config: " + debugCfgFile);
+ System.out.println("Log file: " + logFile);
+
+ if (logFile.delete())
+ System.out.println("Old log file was deleted.");
+
+ Log4J2Logger log = logSupplier.get(infoCfgFile);
+ assertTrue(log.toString().contains(infoCfgFile.getPath()));
+
+ log.info("Accepted info");
+ log.debug("Ignored debug");
+
+ log = logSupplier.get(debugCfgFile);
+ assertTrue(log.toString().contains(debugCfgFile.getPath()));
+
+ log.debug("Accepted debug");
+
+ String logContent = U.readFileToString(logFile.getPath(), "UTF-8");
+
+ assertTrue(logContent.contains("[INFO] Accepted info"));
+ assertFalse(logContent.contains("[DEBUG] Ignored debug"));
+ assertTrue(logContent.contains("[DEBUG] Accepted debug"));
Review Comment:
This assert fails
##########
modules/core/src/test/java/org/apache/ignite/logger/java/JavaLoggerTest.java:
##########
@@ -36,6 +36,31 @@ public class JavaLoggerTest {
@SuppressWarnings({"FieldCanBeLocal"})
private IgniteLogger log;
+ @Test
+ public void testDefaultConstructorWithDefaultConfig() {
+ IgniteLogger log1 = new JavaLogger();
+ IgniteLogger log2 = log1.getLogger(getClass());
+
+ assertTrue(log1.toString().contains("JavaLogger"));
+ assertTrue(log1.toString().contains(JavaLogger.DFLT_CONFIG_PATH));
+
+ assertTrue(log2.toString().contains("JavaLogger"));
+ assertTrue(log2.toString().contains(JavaLogger.DFLT_CONFIG_PATH));
+ }
+
+ @Test
+ public void testDefaultConstructorWithProperty() {
+ System.setProperty("java.util.logging.config.file",
JavaLogger.DFLT_CONFIG_PATH);
+ IgniteLogger log1 = new JavaLogger();
+ IgniteLogger log2 = log1.getLogger(getClass());
+
+ assertTrue(log1.toString().contains("JavaLogger"));
+ assertTrue(log1.toString().contains(JavaLogger.DFLT_CONFIG_PATH));
Review Comment:
Assert failed and was fixed by `this(true)` change in default constructor
--
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
