Fix flakey test. (valkey-io#1123)

avifenesh · Mar 19, 2024 · 6b25812 · 6b25812
1 parent 3d2cadc
commit 6b25812
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 69 deletions.
diff --git a/glide-core/tests/test_socket_listener.rs b/glide-core/tests/test_socket_listener.rs
@@ -700,7 +700,18 @@ mod socket_listener {
 
     #[rstest]
     #[timeout(SHORT_CLUSTER_TEST_TIMEOUT)]
-    fn test_socket_pass_manual_route_by_address() {
+    fn test_socket_cluster_route_by_address_reaches_correct_node() {
+        // returns the line that contains the word "myself", up to that point. This is done because the values after it might change with time.
+        let clean_result = |value: String| {
+            value
+                .split('\n')
+                .find(|line| line.contains("myself"))
+                .and_then(|line| line.split_once("myself"))
+                .unwrap()
+                .0
+                .to_string()
+        };
+
         // We send a request to a random node, get that node's hostname & port, and then
         // route the same request to the hostname & port, and verify that we've received the same value.
         let mut test_basics = setup_cluster_test_basics(Tls::NoTls, TestServer::Shared);
@@ -726,11 +737,10 @@ mod socket_listener {
             panic!("Unexpected response {:?}", response.value);
         };
         let received_value = pointer_to_value(pointer);
-        let first_value = String::from_redis_value(&received_value).unwrap();
+        let first_value = clean_result(String::from_redis_value(&received_value).unwrap());
+
         let (host, port) = first_value
-            .split('\n')
-            .find(|line| line.contains("myself"))
-            .and_then(|line| line.split_once(' '))
+            .split_once(' ')
             .and_then(|(_, second)| second.split_once('@'))
             .and_then(|(first, _)| first.split_once(':'))
             .and_then(|(host, port)| port.parse::<i32>().map(|port| (host, port)).ok())
@@ -754,7 +764,7 @@ mod socket_listener {
             panic!("Unexpected response {:?}", response.value);
         };
         let received_value = pointer_to_value(pointer);
-        let second_value = String::from_redis_value(&received_value).unwrap();
+        let second_value = clean_result(String::from_redis_value(&received_value).unwrap());
 
         assert_eq!(first_value, second_value);
     }

diff --git a/java/integTest/src/test/java/glide/TestUtilities.java b/java/integTest/src/test/java/glide/TestUtilities.java
@@ -4,8 +4,6 @@
 import static org.junit.jupiter.api.Assertions.fail;
 
 import glide.api.models.ClusterValue;
-import java.util.Arrays;
-import java.util.stream.Collectors;
 import lombok.experimental.UtilityClass;
 
 @UtilityClass
@@ -25,24 +23,4 @@ public static int getValueFromInfo(String data, String value) {
     public static <T> T getFirstEntryFromMultiValue(ClusterValue<T> data) {
         return data.getMultiValue().get(data.getMultiValue().keySet().toArray(String[]::new)[0]);
     }
-
-    /**
-     * Replaces 'ping-sent' and 'pong-recv' timestamps in Redis Cluster Nodes output with
-     * placeholders.
-     */
-    public static String removeTimeStampsFromClusterNodesOutput(String rawOutput) {
-        return Arrays.stream(rawOutput.split("\n"))
-                .map(
-                        line -> {
-                            String[] parts = line.split(" ");
-                            // Format for CLUSTER NODES COMMAND: <id> <ip:port@cport[,hostname]> <flags> <master>
-                            // <ping-sent> <pong-recv> <config-epoch> <link-state> <slot> <slot> ... <slot>
-                            if (parts.length > 6) {
-                                parts[4] = "ping-sent";
-                                parts[5] = "pong-recv";
-                            }
-                            return String.join(" ", parts);
-                        })
-                .collect(Collectors.joining("\n"));
-    }
 }
diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java
@@ -5,7 +5,6 @@
 import static glide.TestConfiguration.REDIS_VERSION;
 import static glide.TestUtilities.getFirstEntryFromMultiValue;
 import static glide.TestUtilities.getValueFromInfo;
-import static glide.TestUtilities.removeTimeStampsFromClusterNodesOutput;
 import static glide.api.BaseClient.OK;
 import static glide.api.models.commands.InfoOptions.Section.CLIENTS;
 import static glide.api.models.commands.InfoOptions.Section.CLUSTER;
@@ -371,28 +370,32 @@ public void config_rewrite_non_existent_config_file() {
         assertTrue(executionException.getCause() instanceof RequestException);
     }
 
+    // returns the line that contains the word "myself", up to that point. This is done because the
+    // values after it might change with time.
+    private String cleanResult(String value) {
+        return Arrays.stream(value.split("\n"))
+                .filter(line -> line.contains("myself"))
+                .findFirst()
+                .orElse(null);
+    }
+
     @Test
     @SneakyThrows
     public void cluster_route_by_address_reaches_correct_node() {
         // Masks timestamps in the cluster nodes output to avoid flakiness due to dynamic values.
         String initialNode =
-                removeTimeStampsFromClusterNodesOutput(
+                cleanResult(
                         (String)
                                 clusterClient
                                         .customCommand(new String[] {"cluster", "nodes"}, RANDOM)
                                         .get()
                                         .getSingleValue());
 
-        String host =
-                Arrays.stream(initialNode.split("\n"))
-                        .filter(line -> line.contains("myself"))
-                        .findFirst()
-                        .map(line -> line.split(" ")[1].split("@")[0])
-                        .orElse(null);
+        String host = initialNode.split(" ")[1].split("@")[0];
         assertNotNull(host);
 
         String specifiedClusterNode1 =
-                removeTimeStampsFromClusterNodesOutput(
+                cleanResult(
                         (String)
                                 clusterClient
                                         .customCommand(new String[] {"cluster", "nodes"}, new ByAddressRoute(host))
@@ -402,7 +405,7 @@ public void cluster_route_by_address_reaches_correct_node() {
 
         String[] splitHost = host.split(":");
         String specifiedClusterNode2 =
-                removeTimeStampsFromClusterNodesOutput(
+                cleanResult(
                         (String)
                                 clusterClient
                                         .customCommand(

diff --git a/node/tests/RedisClusterClient.test.ts b/node/tests/RedisClusterClient.test.ts
@@ -138,47 +138,52 @@ describe("RedisClusterClient", () => {
     it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])(
         `route by address reaches correct node_%p`,
         async (protocol) => {
+            // returns the line that contains the word "myself", up to that point. This is done because the values after it might change with time.
+            const cleanResult = (value: string) => {
+                return (
+                    value
+                        .split("\n")
+                        .find((line: string) => line.includes("myself"))
+                        ?.split("myself")[0] ?? ""
+                );
+            };
+
             const client = await RedisClusterClient.createClient(
                 getOptions(cluster.ports(), protocol),
             );
-            const result = (await client.customCommand(
-                ["cluster", "nodes"],
-                "randomNode",
-            )) as string;
+            const result = cleanResult(
+                (await client.customCommand(
+                    ["cluster", "nodes"],
+                    "randomNode",
+                )) as string,
+            );
 
             // check that routing without explicit port works
-            const host =
-                result
-                    .split("\n")
-                    .find((line) => line.includes("myself"))
-                    ?.split(" ")[1]
-                    .split("@")[0] ?? "";
+            const host = result.split(" ")[1].split("@")[0] ?? "";
 
             if (!host) {
                 throw new Error("No host could be parsed");
             }
 
-            const secondResult = (await client.customCommand(
-                ["cluster", "nodes"],
-                {
+            const secondResult = cleanResult(
+                (await client.customCommand(["cluster", "nodes"], {
                     type: "routeByAddress",
                     host,
-                },
-            )) as string;
+                })) as string,
+            );
 
             expect(result).toEqual(secondResult);
 
             const [host2, port] = host.split(":");
 
             // check that routing with explicit port works
-            const thirdResult = (await client.customCommand(
-                ["cluster", "nodes"],
-                {
+            const thirdResult = cleanResult(
+                (await client.customCommand(["cluster", "nodes"], {
                     type: "routeByAddress",
                     host: host2,
                     port: Number(port),
-                },
-            )) as string;
+                })) as string,
+            );
 
             expect(result).toEqual(thirdResult);
 

diff --git a/python/python/tests/test_async_client.py b/python/python/tests/test_async_client.py
@@ -1671,27 +1671,32 @@ async def test_info_random_route(self, redis_client: RedisClusterClient):
     async def test_cluster_route_by_address_reaches_correct_node(
         self, redis_client: RedisClusterClient
     ):
-        cluster_nodes = await redis_client.custom_command(
-            ["cluster", "nodes"], RandomNode()
+        # returns the line that contains the word "myself", up to that point. This is done because the values after it might change with time.
+        clean_result = lambda value: [
+            line for line in value.split("\n") if "myself" in line
+        ][0]
+
+        cluster_nodes = clean_result(
+            await redis_client.custom_command(["cluster", "nodes"], RandomNode())
         )
         assert isinstance(cluster_nodes, str)
-        host = (
-            [line for line in cluster_nodes.split("\n") if "myself" in line][0]
-            .split(" ")[1]
-            .split("@")[0]
-        )
+        host = cluster_nodes.split(" ")[1].split("@")[0]
 
-        second_result = await redis_client.custom_command(
-            ["cluster", "nodes"], ByAddressRoute(host)
+        second_result = clean_result(
+            await redis_client.custom_command(
+                ["cluster", "nodes"], ByAddressRoute(host)
+            )
         )
 
         assert cluster_nodes == second_result
 
         host, port = host.split(":")
         port_as_int = int(port)
 
-        third_result = await redis_client.custom_command(
-            ["cluster", "nodes"], ByAddressRoute(host, port_as_int)
+        third_result = clean_result(
+            await redis_client.custom_command(
+                ["cluster", "nodes"], ByAddressRoute(host, port_as_int)
+            )
         )
 
         assert cluster_nodes == third_result