From ebc3cd8b8249a68360e16b1d9a6acdb5e0018b57 Mon Sep 17 00:00:00 2001 From: Harkrishn Patro Date: Wed, 29 Jan 2025 23:14:32 +0000 Subject: [PATCH] Address feedback for tests Signed-off-by: Harkrishn Patro --- tests/modules/cluster.c | 33 ++----- .../unit/moduleapi/cross-version-cluster.tcl | 87 +++++++++---------- 2 files changed, 48 insertions(+), 72 deletions(-) diff --git a/tests/modules/cluster.c b/tests/modules/cluster.c index 3b58af7abb9..fe4509617e3 100644 --- a/tests/modules/cluster.c +++ b/tests/modules/cluster.c @@ -34,41 +34,18 @@ int test_cluster_shards(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int arg return VALKEYMODULE_OK; } -#define MSGTYPE_PING 1 -#define MSGTYPE_PONG 2 +#define MSGTYPE_DING 1 +#define MSGTYPE_DONG 2 /* test.pingall */ -int PingallCommand_ValkeyCommand(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) { +int PingallCommand(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) { VALKEYMODULE_NOT_USED(argv); VALKEYMODULE_NOT_USED(argc); - ValkeyModule_SendClusterMessage(ctx, NULL, MSGTYPE_PING, "Hey", 3); + ValkeyModule_SendClusterMessage(ctx, NULL, MSGTYPE_DING, "Hey", 3); return ValkeyModule_ReplyWithSimpleString(ctx, "OK"); } -/* Callback for message MSGTYPE_PING */ -void PingReceiver(ValkeyModuleCtx *ctx, - const char *sender_id, - uint8_t type, - const unsigned char *payload, - uint32_t len) { - ValkeyModule_Log(ctx, "notice", "PING (type %d) RECEIVED from %.*s: '%.*s'", type, VALKEYMODULE_NODE_ID_LEN, - sender_id, (int)len, payload); - ValkeyModule_SendClusterMessage(ctx, NULL, MSGTYPE_PONG, "Ohi!", 4); - ValkeyModuleCallReply *reply = ValkeyModule_Call(ctx, "INCR", "c", "pings_received"); - ValkeyModule_FreeCallReply(reply); -} - -/* Callback for message MSGTYPE_PONG */ -void PongReceiver(ValkeyModuleCtx *ctx, - const char *sender_id, - uint8_t type, - const unsigned char *payload, - uint32_t len) { - ValkeyModule_Log(ctx, "notice", "PONG (type %d) RECEIVED from %.*s: '%.*s'", type, VALKEYMODULE_NODE_ID_LEN, - sender_id, (int)len, payload); -} - int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) { VALKEYMODULE_NOT_USED(argv); VALKEYMODULE_NOT_USED(argc); @@ -76,7 +53,7 @@ int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int arg if (ValkeyModule_Init(ctx, "cluster", 1, VALKEYMODULE_APIVER_1)== VALKEYMODULE_ERR) return VALKEYMODULE_ERR; - if (ValkeyModule_CreateCommand(ctx, "test.pingall", PingallCommand_ValkeyCommand, "readonly", 0, 0, 0) == + if (ValkeyModule_CreateCommand(ctx, "test.pingall", PingallCommand, "readonly", 0, 0, 0) == VALKEYMODULE_ERR) return VALKEYMODULE_ERR; diff --git a/tests/unit/moduleapi/cross-version-cluster.tcl b/tests/unit/moduleapi/cross-version-cluster.tcl index a1a36aeb7f8..22df4f5e7b5 100644 --- a/tests/unit/moduleapi/cross-version-cluster.tcl +++ b/tests/unit/moduleapi/cross-version-cluster.tcl @@ -7,58 +7,57 @@ set old_singledb $::singledb set ::singledb 1 tags {external:skip needs:other-server cluster modules} { - -# To run this test use the `--other-server-path` parameter and pass in a compatible server path supporting -# SendClusterMessage module API. -# -# ./runtest-moduleapi --single unit/moduleapi/cross-version-cluster --other-server-path tests/tmp/valkey-8-0/valkey-server -# -# Test cross version compatibility of cluster module send message API. -start_server {config "minimal-cluster.conf" start-other-server 1} { - set testmodule [file normalize tests/modules/cluster.so] - r MODULE LOAD $testmodule - set other_node_name [r CLUSTER MYID] - - start_server {config "minimal-cluster.conf"} { + # To run this test use the `--other-server-path` parameter and pass in a compatible server path supporting + # SendClusterMessage module API. + # + # ./runtest-moduleapi --single unit/moduleapi/cross-version-cluster --other-server-path tests/tmp/valkey-8-0/valkey-server + # + # Test cross version compatibility of cluster module send message API. + start_server {config "minimal-cluster.conf" start-other-server 1} { + set testmodule [file normalize tests/modules/cluster.so] r MODULE LOAD $testmodule - - test "set up cluster" { - r CLUSTER MEET [srv -1 host] [srv -1 port] - - # Link establishment requires few PING-PONG between two nodes - wait_for_condition 50 100 { - [string match {*handshake*} [r CLUSTER NODES]] eq 0 && - [string match {*handshake*} [r -1 CLUSTER NODES]] eq 0 - } else { - puts [r CLUSTER NODES] - puts [r -1 CLUSTER NODES] - fail "Cluster meet stuck in handshake state" + set other_node_name [r CLUSTER MYID] + + start_server {config "minimal-cluster.conf"} { + r MODULE LOAD $testmodule + + test "set up cluster" { + r CLUSTER MEET [srv -1 host] [srv -1 port] + + # Link establishment requires few PING-PONG between two nodes + wait_for_condition 50 100 { + [string match {*handshake*} [r CLUSTER NODES]] eq 0 && + [string match {*handshake*} [r -1 CLUSTER NODES]] eq 0 + } else { + puts [r CLUSTER NODES] + puts [r -1 CLUSTER NODES] + fail "Cluster meet stuck in handshake state" + } } - } - test "Send cluster message via module from latest to other server" { - assert_equal OK [r test.pingall] - assert_match "*cluster_stats_messages_module_sent:1*" [r CLUSTER INFO] - wait_for_condition 50 100 { - [string match {*cluster_stats_messages_module_received:1*} [r -1 CLUSTER INFO]] - } else { - fail "Node didn't receive the message" + test "Send cluster message via module from latest to other server" { + assert_equal OK [r test.pingall] + assert_match "*cluster_stats_messages_module_sent:1*" [r CLUSTER INFO] + wait_for_condition 50 100 { + [string match {*cluster_stats_messages_module_received:1*} [r -1 CLUSTER INFO]] + } else { + fail "Node didn't receive the message" + } } - } - test "Send cluster message via module from other server to latest" { - r CONFIG resetstat - r -1 CONFIG resetstat - assert_equal OK [r -1 test.pingall] - assert_match "*cluster_stats_messages_module_sent:1*" [r -1 CLUSTER INFO] - wait_for_condition 50 100 { - [string match {*cluster_stats_messages_module_received:1*} [r CLUSTER INFO]] - } else { - fail "Node didn't receive the message" + test "Send cluster message via module from other server to latest" { + r CONFIG resetstat + r -1 CONFIG resetstat + assert_equal OK [r -1 test.pingall] + assert_match "*cluster_stats_messages_module_sent:1*" [r -1 CLUSTER INFO] + wait_for_condition 50 100 { + [string match {*cluster_stats_messages_module_received:1*} [r CLUSTER INFO]] + } else { + fail "Node didn't receive the message" + } } } } } -} set ::singledb $old_singledb \ No newline at end of file