This is an automated email from the ASF dual-hosted git repository.

ptupitsyn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git


The following commit(s) were added to refs/heads/main by this push:
     new 4d37da0d13 IGNITE-19058 Fix ClientLoggingTest.testBasicLogging 
flakiness (#1806)
4d37da0d13 is described below

commit 4d37da0d139056f071d1468ba865c45ae3d9f407
Author: Pavel Tupitsyn <[email protected]>
AuthorDate: Fri Mar 17 17:48:43 2023 +0300

    IGNITE-19058 Fix ClientLoggingTest.testBasicLogging flakiness (#1806)
    
    * Fix flakiness: introduce `waitForLogContains`, do not rely on log message 
count. Speeds up the test as well (wait only for what we need).
    * Synchronize access to entries in `TestLoggerFactory`.
    * Improve assertion messages.
---
 .../apache/ignite/client/ClientLoggingTest.java    | 10 +++-------
 .../apache/ignite/client/TestLoggerFactory.java    | 23 ++++++++++++++++++----
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git 
a/modules/client/src/test/java/org/apache/ignite/client/ClientLoggingTest.java 
b/modules/client/src/test/java/org/apache/ignite/client/ClientLoggingTest.java
index 41635df9b8..f24607a92e 100644
--- 
a/modules/client/src/test/java/org/apache/ignite/client/ClientLoggingTest.java
+++ 
b/modules/client/src/test/java/org/apache/ignite/client/ClientLoggingTest.java
@@ -22,11 +22,9 @@ import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.startsWith;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import org.apache.ignite.client.fakes.FakeIgnite;
 import org.apache.ignite.client.fakes.FakeIgniteTables;
-import org.apache.ignite.internal.testframework.IgniteTestUtils;
 import org.apache.ignite.internal.util.IgniteUtils;
 import org.apache.ignite.lang.LoggerFactory;
 import org.junit.jupiter.api.AfterEach;
@@ -94,11 +92,9 @@ public class ClientLoggingTest {
             client.tables().tables();
             client.tables().table("t");
 
-            assertTrue(IgniteTestUtils.waitForCondition(() -> 
loggerFactory.logger.entries().size() > 10, 5_000));
-
-            loggerFactory.assertLogContains("Connection established");
-            loggerFactory.assertLogContains("c:Sending request [opCode=3, 
remoteAddress=127.0.0.1:1095");
-            loggerFactory.assertLogContains("c:Failed to establish connection 
to 127.0.0.1:1095");
+            loggerFactory.waitForLogContains("Connection established", 5000);
+            loggerFactory.waitForLogContains("c:Sending request [opCode=3, 
remoteAddress=127.0.0.1:1095", 5000);
+            loggerFactory.waitForLogContains("c:Failed to establish connection 
to 127.0.0.1:1095", 5000);
         }
     }
 
diff --git 
a/modules/client/src/test/java/org/apache/ignite/client/TestLoggerFactory.java 
b/modules/client/src/test/java/org/apache/ignite/client/TestLoggerFactory.java
index dfe39118fb..468aa75562 100644
--- 
a/modules/client/src/test/java/org/apache/ignite/client/TestLoggerFactory.java
+++ 
b/modules/client/src/test/java/org/apache/ignite/client/TestLoggerFactory.java
@@ -24,6 +24,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.ResourceBundle;
 import java.util.function.Supplier;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
 import org.apache.ignite.lang.LoggerFactory;
 
 /**
@@ -42,7 +43,21 @@ public class TestLoggerFactory implements LoggerFactory {
     }
 
     void assertLogContains(String msg) {
-        assertTrue(logger.entries().stream().anyMatch(x -> x.contains(msg)));
+        assertTrue(logContains(msg), this::log);
+    }
+
+    void waitForLogContains(String msg, long timeoutMillis) throws 
InterruptedException {
+        assertTrue(
+                IgniteTestUtils.waitForCondition(() -> logContains(msg), 
timeoutMillis),
+                () -> "Log does not contain expected message '" + msg + "': " 
+  log());
+    }
+
+    private boolean logContains(String msg) {
+        return logger.entries().stream().anyMatch(x -> x.contains(msg));
+    }
+
+    private String log() {
+        return String.join("\n", logger.entries());
     }
 
     /** Logger that stores all messages in a list. */
@@ -105,11 +120,11 @@ public class TestLoggerFactory implements LoggerFactory {
             throw new AssertionError("Should not be called");
         }
 
-        public List<String> entries() {
-            return logEntries;
+        public synchronized List<String> entries() {
+            return new ArrayList<>(logEntries);
         }
 
-        private void captureLog(String msg) {
+        private synchronized void captureLog(String msg) {
             logEntries.add(name + ":" + msg);
         }
     }

Reply via email to