From 03a562a3ac273f0e919d85aedd7acb171b768798 Mon Sep 17 00:00:00 2001 From: Shachar Langbeheim Date: Sat, 16 Mar 2024 18:38:52 +0200 Subject: [PATCH] Fix flakey test. (#1123) --- glide-core/tests/test_socket_listener.rs | 22 ++++++--- .../src/test/java/glide/TestUtilities.java | 22 --------- .../test/java/glide/cluster/CommandTests.java | 23 +++++----- node/tests/RedisClusterClient.test.ts | 45 ++++++++++--------- python/python/tests/test_async_client.py | 27 ++++++----- 5 files changed, 70 insertions(+), 69 deletions(-) diff --git a/glide-core/tests/test_socket_listener.rs b/glide-core/tests/test_socket_listener.rs index 11503c449c..39c5fb2447 100644 --- 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::().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 index 51af70e326..8c18e8b98f 100644 --- 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 getFirstEntryFromMultiValue(ClusterValue 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: - // ... - 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 index ce54e474ef..7bc59d60f8 100644 --- 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 index fd5528e698..f2c5cdcfd7 100644 --- 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 index 66210eeec1..4256cdaa3a 100644 --- a/python/python/tests/test_async_client.py +++ b/python/python/tests/test_async_client.py @@ -1671,18 +1671,21 @@ 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 @@ -1690,8 +1693,10 @@ async def test_cluster_route_by_address_reaches_correct_node( 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