rpuch commented on code in PR #2429:
URL: https://github.com/apache/ignite-3/pull/2429#discussion_r1298364464
##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/ItClusterCommandTest.java:
##########
@@ -70,42 +67,34 @@ class ItClusterCommandTest extends AbstractCliTest {
private static final String NL = System.lineSeparator();
- private static final Logger topologyLogger =
Logger.getLogger("org.apache.ignite.network.scalecube.ScaleCubeTopologyService");
-
@BeforeEach
void setup(@WorkDirectory Path workDir, TestInfo testInfo) throws
Exception {
CountDownLatch allNodesAreInPhysicalTopology = new CountDownLatch(1);
- Handler physicalTopologyWaiter =
physicalTopologyWaiter(allNodesAreInPhysicalTopology);
- topologyLogger.addHandler(physicalTopologyWaiter);
+ TestLogChecker topologyLogger = new TestLogChecker(
Review Comment:
This is not a logger, this is a checker. I suggest to rename this accordingly
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/TestLogChecker.java:
##########
@@ -0,0 +1,324 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.testframework.log4j2;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Predicate;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+import org.apache.logging.log4j.core.config.Property;
+
+/**
+ * Test log checker.
Review Comment:
Let's expand the documentation with an explanation of what this checker
does, what checks it performs, how it can be used
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/raftsnapshot/ItTableRaftSnapshotsTest.java:
##########
@@ -130,15 +128,15 @@ class ItTableRaftSnapshotsTest extends
IgniteIntegrationTest {
private Cluster cluster;
- private Logger replicatorLogger;
+ private TestLogChecker replicatorLogger;
Review Comment:
This is not a logger anymore, the field probably needs to be renamed
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/TestLogChecker.java:
##########
@@ -0,0 +1,324 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.testframework.log4j2;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Predicate;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+import org.apache.logging.log4j.core.config.Property;
+
+/**
+ * Test log checker.
+ */
+public class TestLogChecker {
+ /** Logger name. */
+ private final String loggerName;
+
+ /** List of handlers. */
+ private final List<Handler> handlers = new ArrayList<>(1);
+
+ /** Lock that is used to synchronize the collection of handlers. */
+ private final ReadWriteLock lock = new ReentrantReadWriteLock();
+
+ /** Test log appender that is used to check log events/messages. */
+ private final TestLogAppender appender;
+
+ /** Logger configuration. */
+ private Configuration config;
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
loggerName}.
+ *
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(String loggerName) {
+ return create(loggerName, false);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
loggerName}.
+ *
+ * @param started Whether the checker should be started.
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(String loggerName, boolean started) {
+ TestLogChecker checker = new TestLogChecker(loggerName);
+
+ if (started) {
+ checker.start();
+ }
+
+ return checker;
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
clazz}.
+ *
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(Class<?> clazz) {
+ return create(clazz, false);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
clazz}.
+ *
+ * @param started Whether the checker should be started.
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(Class<?> clazz, boolean started) {
+ return create(clazz.getName(), started);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker}.
+ *
+ * @param loggerName Logger name.
+ */
+ public TestLogChecker(String loggerName) {
+ Objects.requireNonNull(loggerName);
+
+ this.loggerName = loggerName;
+ this.appender = new TestLogAppender(loggerName);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} with the given {@code
predicate}.
+ *
+ * @param loggerName Logger name.
+ * @param predicate Predicate to check log messages.
+ */
+ public TestLogChecker(String loggerName, Predicate<String> predicate) {
Review Comment:
Also, in some cases it is required to verify other message attributes, not
only its text (for instance, we might want to verify that a message is logged
as a WARN). Let's make the predicate accept the whole message, not just its
text.
##########
modules/network/src/integrationTest/java/org/apache/ignite/network/scalecube/ItScaleCubeNetworkMessagingTest.java:
##########
@@ -87,10 +86,15 @@ class ItScaleCubeNetworkMessagingTest {
/** Message factory. */
private final TestMessagesFactory messageFactory = new
TestMessagesFactory();
+ /** List of test loggers. */
+ private final List<TestLogChecker> loggers = new ArrayList<>();
Review Comment:
Those are not loggers, they are checkers
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/raftsnapshot/ItTableRaftSnapshotsTest.java:
##########
@@ -734,18 +728,14 @@ private BiPredicate<String, NetworkMessage>
dropSnapshotMetaResponse(Completable
void
snapshotInstallTimeoutDoesNotBreakSubsequentInstallsWhenSecondAttemptIsIdenticalToFirst()
throws Exception {
AtomicBoolean snapshotInstallFailedDueToIdenticalRetry = new
AtomicBoolean(false);
- Logger snapshotExecutorLogger =
Logger.getLogger(SnapshotExecutorImpl.class.getName());
+ TestLogChecker snapshotExecutorLogger =
TestLogChecker.create(SnapshotExecutorImpl.class);
Review Comment:
This is not a logger
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/TestLogChecker.java:
##########
@@ -0,0 +1,324 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.testframework.log4j2;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Predicate;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+import org.apache.logging.log4j.core.config.Property;
+
+/**
+ * Test log checker.
+ */
+public class TestLogChecker {
+ /** Logger name. */
+ private final String loggerName;
+
+ /** List of handlers. */
+ private final List<Handler> handlers = new ArrayList<>(1);
+
+ /** Lock that is used to synchronize the collection of handlers. */
+ private final ReadWriteLock lock = new ReentrantReadWriteLock();
+
+ /** Test log appender that is used to check log events/messages. */
+ private final TestLogAppender appender;
+
+ /** Logger configuration. */
+ private Configuration config;
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
loggerName}.
+ *
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(String loggerName) {
+ return create(loggerName, false);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
loggerName}.
+ *
+ * @param started Whether the checker should be started.
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(String loggerName, boolean started) {
+ TestLogChecker checker = new TestLogChecker(loggerName);
+
+ if (started) {
+ checker.start();
+ }
+
+ return checker;
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
clazz}.
+ *
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(Class<?> clazz) {
+ return create(clazz, false);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
clazz}.
+ *
+ * @param started Whether the checker should be started.
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(Class<?> clazz, boolean started) {
+ return create(clazz.getName(), started);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker}.
+ *
+ * @param loggerName Logger name.
+ */
+ public TestLogChecker(String loggerName) {
+ Objects.requireNonNull(loggerName);
+
+ this.loggerName = loggerName;
+ this.appender = new TestLogAppender(loggerName);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} with the given {@code
predicate}.
+ *
+ * @param loggerName Logger name.
+ * @param predicate Predicate to check log messages.
+ */
+ public TestLogChecker(String loggerName, Predicate<String> predicate) {
+ this(loggerName, predicate, () -> {});
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} with the given {@code
predicate} and {@code action}.
+ *
+ * @param loggerName Logger name.
+ * @param predicate Predicate to check log messages.
+ * @param action Action to be executed when the {@code predicate} is
matched.
+ */
+ public TestLogChecker(String loggerName, Predicate<String> predicate,
Runnable action) {
+ this(loggerName, new Handler(predicate, action));
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} with the given {@code
predicate} and {@code action}.
+ *
+ * @param loggerName Logger name.
+ * @param handlers List of handlers to be added.
+ */
+ public TestLogChecker(String loggerName, Handler... handlers) {
+ this(loggerName);
+
+ lock.writeLock().lock();
+ try {
+ Collections.addAll(this.handlers, handlers);
+ } finally {
+ lock.writeLock().unlock();
+ }
+ }
+
+ /**
+ * Adds a new handler with the given {@code predicate} and {@code action}.
+ *
+ * @param predicate Predicate to check log messages.
+ * @param action Action to be executed when the {@code predicate} is
matched.
+ * @return New instance of {@link Handler}.
+ */
+ public Handler addHandler(Predicate<String> predicate, Runnable action) {
+ Handler handler = new Handler(predicate, action);
+
+ lock.writeLock().lock();
Review Comment:
It seems we can just invoke `addHandler(handler)` here
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/TestLogChecker.java:
##########
@@ -0,0 +1,324 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.testframework.log4j2;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Predicate;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+import org.apache.logging.log4j.core.config.Property;
+
+/**
+ * Test log checker.
+ */
+public class TestLogChecker {
+ /** Logger name. */
+ private final String loggerName;
+
+ /** List of handlers. */
+ private final List<Handler> handlers = new ArrayList<>(1);
+
+ /** Lock that is used to synchronize the collection of handlers. */
+ private final ReadWriteLock lock = new ReentrantReadWriteLock();
+
+ /** Test log appender that is used to check log events/messages. */
+ private final TestLogAppender appender;
+
+ /** Logger configuration. */
+ private Configuration config;
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
loggerName}.
+ *
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(String loggerName) {
+ return create(loggerName, false);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
loggerName}.
+ *
+ * @param started Whether the checker should be started.
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(String loggerName, boolean started) {
+ TestLogChecker checker = new TestLogChecker(loggerName);
+
+ if (started) {
+ checker.start();
+ }
+
+ return checker;
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
clazz}.
+ *
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(Class<?> clazz) {
+ return create(clazz, false);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
clazz}.
+ *
+ * @param started Whether the checker should be started.
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(Class<?> clazz, boolean started) {
+ return create(clazz.getName(), started);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker}.
+ *
+ * @param loggerName Logger name.
+ */
+ public TestLogChecker(String loggerName) {
+ Objects.requireNonNull(loggerName);
+
+ this.loggerName = loggerName;
+ this.appender = new TestLogAppender(loggerName);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} with the given {@code
predicate}.
+ *
+ * @param loggerName Logger name.
+ * @param predicate Predicate to check log messages.
+ */
+ public TestLogChecker(String loggerName, Predicate<String> predicate) {
+ this(loggerName, predicate, () -> {});
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} with the given {@code
predicate} and {@code action}.
+ *
+ * @param loggerName Logger name.
+ * @param predicate Predicate to check log messages.
+ * @param action Action to be executed when the {@code predicate} is
matched.
+ */
+ public TestLogChecker(String loggerName, Predicate<String> predicate,
Runnable action) {
+ this(loggerName, new Handler(predicate, action));
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} with the given {@code
predicate} and {@code action}.
Review Comment:
There is no predicate among parameters, probably this needs to be adjusted
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/log4j2/TestLogChecker.java:
##########
@@ -0,0 +1,324 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.testframework.log4j2;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Predicate;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+import org.apache.logging.log4j.core.config.Property;
+
+/**
+ * Test log checker.
+ */
+public class TestLogChecker {
+ /** Logger name. */
+ private final String loggerName;
+
+ /** List of handlers. */
+ private final List<Handler> handlers = new ArrayList<>(1);
+
+ /** Lock that is used to synchronize the collection of handlers. */
+ private final ReadWriteLock lock = new ReentrantReadWriteLock();
+
+ /** Test log appender that is used to check log events/messages. */
+ private final TestLogAppender appender;
+
+ /** Logger configuration. */
+ private Configuration config;
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
loggerName}.
+ *
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(String loggerName) {
+ return create(loggerName, false);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
loggerName}.
+ *
+ * @param started Whether the checker should be started.
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(String loggerName, boolean started) {
+ TestLogChecker checker = new TestLogChecker(loggerName);
+
+ if (started) {
+ checker.start();
+ }
+
+ return checker;
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
clazz}.
+ *
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(Class<?> clazz) {
+ return create(clazz, false);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} for the given {@code
clazz}.
+ *
+ * @param started Whether the checker should be started.
+ * @return New instance of {@link TestLogChecker}.
+ **/
+ public static TestLogChecker create(Class<?> clazz, boolean started) {
+ return create(clazz.getName(), started);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker}.
+ *
+ * @param loggerName Logger name.
+ */
+ public TestLogChecker(String loggerName) {
+ Objects.requireNonNull(loggerName);
+
+ this.loggerName = loggerName;
+ this.appender = new TestLogAppender(loggerName);
+ }
+
+ /**
+ * Creates a new instance of {@link TestLogChecker} with the given {@code
predicate}.
+ *
+ * @param loggerName Logger name.
+ * @param predicate Predicate to check log messages.
+ */
+ public TestLogChecker(String loggerName, Predicate<String> predicate) {
Review Comment:
What happens if the predicate returns true or 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
