timoninmaxim commented on code in PR #11615:
URL: https://github.com/apache/ignite/pull/11615#discussion_r1832180105
##########
modules/core/src/test/java/org/apache/ignite/logger/java/JavaLoggerTest.java:
##########
@@ -17,31 +17,154 @@
package org.apache.ignite.logger.java;
+import java.io.File;
+import java.io.IOException;
import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.logging.ConsoleHandler;
+import java.util.logging.Filter;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogManager;
+import java.util.logging.Logger;
import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.configuration.IgniteConfiguration;
import org.apache.ignite.internal.logger.IgniteLoggerEx;
import org.apache.ignite.internal.util.typedef.internal.U;
import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
import org.apache.ignite.testframework.junits.common.GridCommonTest;
import org.junit.Test;
-import static org.junit.Assert.assertTrue;
-
/**
* Java logger test.
*/
@GridCommonTest(group = "Logger")
-public class JavaLoggerTest {
- /** */
- @SuppressWarnings({"FieldCanBeLocal"})
- private IgniteLogger log;
+public class JavaLoggerTest extends GridCommonAbstractTest {
+ /**
+ * Path to jul configuration with DEBUG enabled.
+ */
+ private static final String LOG_CONFIG_DEBUG =
"modules/core/src/test/config/jul-debug.properties";
+
Review Comment:
Please, remove excess blank lines
##########
modules/core/src/test/java/org/apache/ignite/logger/java/JavaLoggerTest.java:
##########
@@ -84,4 +207,13 @@ public void testLogInitialize() throws Exception {
assert !log.fileName().contains("%");
assert log.fileName().contains("other-app");
}
+
+ /**
+ * Configure java.util.logging.config.file property
+ */
+ private void setJavaLoggerConfig() throws IOException {
+ File file = new File(U.getIgniteHome(), LOG_CONFIG_DEBUG);
+ System.setProperty("java.util.logging.config.file", file.getPath());
+ LogManager.getLogManager().readConfiguration();
Review Comment:
I suppose user shouldn't explicitly read configuration. It is invoked inside
LogManager while user tries to log first message, isn't it?
##########
modules/core/src/test/java/org/apache/ignite/logger/java/JavaLoggerTest.java:
##########
@@ -84,4 +207,13 @@ public void testLogInitialize() throws Exception {
assert !log.fileName().contains("%");
assert log.fileName().contains("other-app");
}
+
+ /**
+ * Configure java.util.logging.config.file property
+ */
+ private void setJavaLoggerConfig() throws IOException {
Review Comment:
Method can be inlined
##########
modules/core/src/test/java/org/apache/ignite/logger/java/JavaLoggerTest.java:
##########
@@ -17,31 +17,154 @@
package org.apache.ignite.logger.java;
+import java.io.File;
+import java.io.IOException;
import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.logging.ConsoleHandler;
+import java.util.logging.Filter;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogManager;
+import java.util.logging.Logger;
import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.configuration.IgniteConfiguration;
import org.apache.ignite.internal.logger.IgniteLoggerEx;
import org.apache.ignite.internal.util.typedef.internal.U;
import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
import org.apache.ignite.testframework.junits.common.GridCommonTest;
import org.junit.Test;
-import static org.junit.Assert.assertTrue;
-
/**
* Java logger test.
*/
@GridCommonTest(group = "Logger")
-public class JavaLoggerTest {
- /** */
- @SuppressWarnings({"FieldCanBeLocal"})
- private IgniteLogger log;
+public class JavaLoggerTest extends GridCommonAbstractTest {
+ /**
+ * Path to jul configuration with DEBUG enabled.
+ */
+ private static final String LOG_CONFIG_DEBUG =
"modules/core/src/test/config/jul-debug.properties";
+
+
+ /**
+ * Reset JavaLogger
+ */
+ @Override protected void afterTest() throws Exception {
+ GridTestUtils.setFieldValue(JavaLogger.class, JavaLogger.class,
"inited", false);
+ }
+
+
+ /**
+ * Check JavaLogger default constructor
+ */
+ @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));
+ }
+
+ /**
+ * Check JavaLogger constructor from java.util.logging.config.file property
+ */
+ @Test
+ public void testDefaultConstructorWithProperty() throws Exception {
+ setJavaLoggerConfig();
+ IgniteLogger log1 = new JavaLogger();
+ IgniteLogger log2 = log1.getLogger(getClass());
+
+ assertTrue(log1.toString().contains("JavaLogger"));
+ assertTrue(log1.toString().contains(LOG_CONFIG_DEBUG));
+ assertTrue(log1.isDebugEnabled());
+
+ assertTrue(log2.toString().contains("JavaLogger"));
+ assertTrue(log2.toString().contains(LOG_CONFIG_DEBUG));
+ assertTrue(log1.isDebugEnabled());
+ System.clearProperty("java.util.logging.config.file");
+ }
+
+ /**
+ * Check JavaLogger constructor with custom logger
+ */
+ @Test
+ public void testDefaultConstructorWithCustomLogger() {
+ IgniteLogger log = new
JavaLogger(Logger.getLogger(getClass().getSimpleName()));
+ assertTrue(log.toString().contains("JavaLogger"));
+ assertTrue(log.toString().contains("null"));
+ }
+
+ /**
+ * Check Grid logging
+ */
+ @Test
+ public void testGridLoggingWithDefaultLogger() throws Exception {
+ JavaLogger log = new JavaLogger();
+ assertTrue(log.isQuiet());
+ AtomicBoolean hasLog = new AtomicBoolean(false);
+ Filter filter = record -> {
+ String msg = record.getMessage();
+ if (msg.contains("JavaLogger") && msg.contains("config=") &&
msg.contains(JavaLogger.DFLT_CONFIG_PATH))
+ hasLog.set(true);
+ return true;
+ };
+
+ Logger rootLog = LogManager.getLogManager().getLogger("");
+ for (Handler hnd : rootLog.getHandlers()) {
+ if (hnd instanceof JavaLoggerFileHandler) {
+ hnd.setFilter(filter);
+ break;
+ }
+ }
+
+ IgniteConfiguration cfg =
getConfiguration(getTestIgniteInstanceName());
+ cfg.setGridLogger(log);
+ startGrid(cfg);
+
+ doSleep(2_000);
+ assertTrue(hasLog.get());
+ stopAllGrids();
+ }
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testGridLoggingWithCustomLogger() throws Exception {
+ AtomicBoolean hasLog = new AtomicBoolean(false);
+ Filter filter = record -> {
Review Comment:
Use LogListener instead
##########
modules/core/src/test/java/org/apache/ignite/logger/java/JavaLoggerTest.java:
##########
@@ -17,31 +17,154 @@
package org.apache.ignite.logger.java;
+import java.io.File;
+import java.io.IOException;
import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.logging.ConsoleHandler;
+import java.util.logging.Filter;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogManager;
+import java.util.logging.Logger;
import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.configuration.IgniteConfiguration;
import org.apache.ignite.internal.logger.IgniteLoggerEx;
import org.apache.ignite.internal.util.typedef.internal.U;
import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
import org.apache.ignite.testframework.junits.common.GridCommonTest;
import org.junit.Test;
-import static org.junit.Assert.assertTrue;
-
/**
* Java logger test.
*/
@GridCommonTest(group = "Logger")
-public class JavaLoggerTest {
- /** */
- @SuppressWarnings({"FieldCanBeLocal"})
- private IgniteLogger log;
+public class JavaLoggerTest extends GridCommonAbstractTest {
+ /**
+ * Path to jul configuration with DEBUG enabled.
+ */
+ private static final String LOG_CONFIG_DEBUG =
"modules/core/src/test/config/jul-debug.properties";
+
+
+ /**
+ * Reset JavaLogger
+ */
+ @Override protected void afterTest() throws Exception {
+ GridTestUtils.setFieldValue(JavaLogger.class, JavaLogger.class,
"inited", false);
+ }
+
+
+ /**
+ * Check JavaLogger default constructor
Review Comment:
Sentences in comments and javadocs should end with a dot sign.
##########
modules/core/src/test/java/org/apache/ignite/logger/java/JavaLoggerTest.java:
##########
@@ -17,31 +17,154 @@
package org.apache.ignite.logger.java;
+import java.io.File;
+import java.io.IOException;
import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.logging.ConsoleHandler;
+import java.util.logging.Filter;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogManager;
+import java.util.logging.Logger;
import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.configuration.IgniteConfiguration;
import org.apache.ignite.internal.logger.IgniteLoggerEx;
import org.apache.ignite.internal.util.typedef.internal.U;
import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
import org.apache.ignite.testframework.junits.common.GridCommonTest;
import org.junit.Test;
-import static org.junit.Assert.assertTrue;
-
/**
* Java logger test.
*/
@GridCommonTest(group = "Logger")
-public class JavaLoggerTest {
- /** */
- @SuppressWarnings({"FieldCanBeLocal"})
- private IgniteLogger log;
+public class JavaLoggerTest extends GridCommonAbstractTest {
+ /**
+ * Path to jul configuration with DEBUG enabled.
+ */
+ private static final String LOG_CONFIG_DEBUG =
"modules/core/src/test/config/jul-debug.properties";
+
+
+ /**
+ * Reset JavaLogger
+ */
+ @Override protected void afterTest() throws Exception {
+ GridTestUtils.setFieldValue(JavaLogger.class, JavaLogger.class,
"inited", false);
+ }
+
+
+ /**
+ * Check JavaLogger default constructor
+ */
+ @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));
+ }
+
+ /**
+ * Check JavaLogger constructor from java.util.logging.config.file property
+ */
+ @Test
+ public void testDefaultConstructorWithProperty() throws Exception {
+ setJavaLoggerConfig();
+ IgniteLogger log1 = new JavaLogger();
+ IgniteLogger log2 = log1.getLogger(getClass());
+
+ assertTrue(log1.toString().contains("JavaLogger"));
+ assertTrue(log1.toString().contains(LOG_CONFIG_DEBUG));
+ assertTrue(log1.isDebugEnabled());
+
+ assertTrue(log2.toString().contains("JavaLogger"));
+ assertTrue(log2.toString().contains(LOG_CONFIG_DEBUG));
+ assertTrue(log1.isDebugEnabled());
+ System.clearProperty("java.util.logging.config.file");
+ }
+
+ /**
+ * Check JavaLogger constructor with custom logger
+ */
+ @Test
+ public void testDefaultConstructorWithCustomLogger() {
+ IgniteLogger log = new
JavaLogger(Logger.getLogger(getClass().getSimpleName()));
+ assertTrue(log.toString().contains("JavaLogger"));
+ assertTrue(log.toString().contains("null"));
+ }
+
+ /**
+ * Check Grid logging
+ */
+ @Test
+ public void testGridLoggingWithDefaultLogger() throws Exception {
+ JavaLogger log = new JavaLogger();
+ assertTrue(log.isQuiet());
+ AtomicBoolean hasLog = new AtomicBoolean(false);
+ Filter filter = record -> {
+ String msg = record.getMessage();
+ if (msg.contains("JavaLogger") && msg.contains("config=") &&
msg.contains(JavaLogger.DFLT_CONFIG_PATH))
+ hasLog.set(true);
+ return true;
+ };
+
+ Logger rootLog = LogManager.getLogManager().getLogger("");
+ for (Handler hnd : rootLog.getHandlers()) {
+ if (hnd instanceof JavaLoggerFileHandler) {
+ hnd.setFilter(filter);
+ break;
+ }
+ }
+
+ IgniteConfiguration cfg =
getConfiguration(getTestIgniteInstanceName());
+ cfg.setGridLogger(log);
+ startGrid(cfg);
+
+ doSleep(2_000);
+ assertTrue(hasLog.get());
+ stopAllGrids();
+ }
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testGridLoggingWithCustomLogger() throws Exception {
+ AtomicBoolean hasLog = new AtomicBoolean(false);
+ Filter filter = record -> {
+ if (record.getMessage().contains("JavaLogger") &&
record.getMessage().contains("config=null"))
+ hasLog.set(true);
+ return true;
+ };
+ ConsoleHandler hnd = new ConsoleHandler();
+ hnd.setFilter(filter);
+ hnd.setLevel(Level.INFO);
+
+ Logger rootLog = LogManager.getLogManager().getLogger("");
+ rootLog.addHandler(hnd);
+ JavaLogger log = new JavaLogger(rootLog);
+
+ IgniteConfiguration cfg =
getConfiguration(getTestIgniteInstanceName());
+ cfg.setGridLogger(log);
+ startGrid(cfg);
+
+ doSleep(2_000);
Review Comment:
Please don't use sleep for waiting any events. Use instead
GridTestUtils#waitForCondition.
##########
modules/core/src/main/java/org/apache/ignite/logger/java/JavaLogger.java:
##########
@@ -194,7 +194,7 @@ public JavaLogger(boolean init) {
* @param impl Java Logging implementation to use.
*/
public JavaLogger(final Logger impl) {
Review Comment:
Do we really need this 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
