Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix flakey test. #1123

Merged
merged 1 commit into from
Mar 16, 2024
Merged

Fix flakey test. #1123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions glide-core/tests/test_socket_listener.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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())
Expand All @@ -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);
}
Expand Down
22 changes: 0 additions & 22 deletions java/integTest/src/test/java/glide/TestUtilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"));
}
}
23 changes: 13 additions & 10 deletions java/integTest/src/test/java/glide/cluster/CommandTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand All @@ -402,7 +405,7 @@ public void cluster_route_by_address_reaches_correct_node() {

String[] splitHost = host.split(":");
String specifiedClusterNode2 =
removeTimeStampsFromClusterNodesOutput(
cleanResult(
(String)
clusterClient
.customCommand(
Expand Down
45 changes: 25 additions & 20 deletions node/tests/RedisClusterClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
27 changes: 16 additions & 11 deletions python/python/tests/test_async_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading