Skip to content

Commit 3fd0942

Browse files
zhijun42zhijun42
authored
Refactor TCL reference to support running tests with CMake (#2816)
Historically, Valkey’s TCL test suite expected all binaries (src/valkey-server, src/valkey-cli, src/valkey-benchmark, etc.) to exist under the src/ directory. This PR enables Valkey TCL tests to run seamlessly after a CMake build — no manual symlinks or make build required. The test framework accepts a new environment variable `VALKEY_BIN_DIR` to look for the binaries. CMake will copy all TCL test entrypoints (runtest, runtest-cluster, etc.) into the CMake build dir (e.g. `cmake-build-debug`) and insert `VALKEY_BIN_DIR` into these. Now we can either do ./cmake-build-debug/runtest at the project root or ./runtest at the Cmake dir to run all tests. A new CMake post-build target prints a friendly reminder after successful builds, guiding developers on how to run tests with their CMake binaries: ``` Hint: It is a good idea to run tests with your CMake-built binaries ;) ./cmake-build-debug/runtest Build finished ``` A helper TCL script `tests/support/set_executable_path.tcl` is added to support this change, which gets called by all test entrypoints: `runtest`, `runtest-cluster`, `runtest-sentinel`. --------- Signed-off-by: Zhijun <dszhijun@gmail.com>
1 parent 825d19f commit 3fd0942

27 files changed

+179
-105
lines changed

‎CMakeLists.txt‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,34 @@ unset(BUILD_TEST_MODULES CACHE)
4343
unset(BUILD_EXAMPLE_MODULES CACHE)
4444
unset(USE_TLS CACHE)
4545
unset(DEBUG_FORCE_DEFRAG CACHE)
46+
47+
# Helper to copy runtest scripts to allow running tests with CMake built binaries
48+
function(copy_runtest_script script_name)
49+
set(src "${CMAKE_SOURCE_DIR}/${script_name}")
50+
set(dst "${CMAKE_BINARY_DIR}/${script_name}")
51+
file(READ "${src}" contents)
52+
53+
# Split at the first newline (after shebang #!/bin/sh)
54+
string(FIND "${contents}" "\n" index_of_first_newline_char)
55+
math(EXPR first_index_script_body "${index_of_first_newline_char} + 1")
56+
57+
string(SUBSTRING "${contents}" 0 ${first_index_script_body} shebang_line)
58+
string(SUBSTRING "${contents}" ${first_index_script_body} -1 script_body)
59+
60+
# Insert our environment variable lines
61+
set(insert_content
62+
"# Most tests assume running from the project root
63+
cd ${CMAKE_SOURCE_DIR}
64+
export VALKEY_BIN_DIR=\"${CMAKE_BINARY_DIR}/bin\"
65+
")
66+
# Reconstruct the full script
67+
set(new_contents "${shebang_line}${insert_content}${script_body}")
68+
file(WRITE "${dst}" "${new_contents}")
69+
file(CHMOD "${dst}" PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE)
70+
endfunction()
71+
72+
copy_runtest_script("runtest")
73+
copy_runtest_script("runtest-cluster")
74+
copy_runtest_script("runtest-moduleapi")
75+
copy_runtest_script("runtest-rdma")
76+
copy_runtest_script("runtest-sentinel")

‎cmake/Modules/ValkeySetup.cmake‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,13 @@ endif ()
5050

5151
# Helper function for creating symbolic link so that: link -> source
5252
macro (valkey_create_symlink source link)
53-
install(
54-
CODE "execute_process( \
55-
COMMAND /bin/bash ${CMAKE_BINARY_DIR}/CreateSymlink.sh \
56-
${source} \
57-
${link} \
58-
)"
59-
COMPONENT "valkey")
53+
add_custom_command(
54+
TARGET ${source} POST_BUILD
55+
COMMAND ${CMAKE_COMMAND} -E create_symlink
56+
"$<TARGET_FILE_NAME:${source}>"
57+
"$<TARGET_FILE_DIR:${source}>/${link}"
58+
VERBATIM
59+
)
6060
endmacro ()
6161

6262
# Install a binary

‎src/CMakeLists.txt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,13 @@ endif ()
9393
if (BUILD_UNIT_TESTS)
9494
add_subdirectory(unit)
9595
endif ()
96+
97+
# Friendly hint like the Makefile one
98+
file(RELATIVE_PATH _CMAKE_DIR_RELATIVE_PATH "${CMAKE_SOURCE_DIR}" "${CMAKE_BINARY_DIR}")
99+
add_custom_target(hint ALL
100+
DEPENDS valkey-server valkey-cli valkey-benchmark
101+
COMMAND ${CMAKE_COMMAND} -E echo ""
102+
COMMAND ${CMAKE_COMMAND} -E echo "Hint: It is a good idea to run tests with your CMake-built binaries \\;\\)"
103+
COMMAND ${CMAKE_COMMAND} -E echo " ./${_CMAKE_DIR_RELATIVE_PATH}/runtest"
104+
COMMAND ${CMAKE_COMMAND} -E echo ""
105+
)

‎tests/cluster/run.tcl‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
# This software is released under the BSD License. See the COPYING file for
33
# more information.
44

5+
# Set the executable paths at project root
6+
source tests/support/set_executable_path.tcl
7+
58
cd tests/cluster
69
source cluster.tcl
710
source ../instances.tcl

‎tests/cluster/tests/04-resharding.tcl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ test "Cluster consistency during live resharding" {
8484
flush stdout
8585
set target [dict get [get_myself [randomInt 5]] id]
8686
set tribpid [lindex [exec \
87-
../../../src/valkey-cli --cluster reshard \
87+
$::VALKEY_CLI_BIN --cluster reshard \
8888
127.0.0.1:[get_instance_attrib valkey 0 port] \
8989
--cluster-from all \
9090
--cluster-to $target \

‎tests/cluster/tests/12-replica-migration-2.tcl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ test "Set allow-replica-migration yes" {
3838
set master0_id [dict get [get_myself 0] id]
3939
test "Resharding all the master #0 slots away from it" {
4040
set output [exec \
41-
../../../src/valkey-cli --cluster rebalance \
41+
$::VALKEY_CLI_BIN --cluster rebalance \
4242
127.0.0.1:[get_instance_attrib valkey 0 port] \
4343
{*}[valkeycli_tls_config "../../../tests"] \
4444
--cluster-weight ${master0_id}=0 >@ stdout ]
@@ -59,7 +59,7 @@ test "Resharding back some slot to master #0" {
5959
# new resharding.
6060
after 10000
6161
set output [exec \
62-
../../../src/valkey-cli --cluster rebalance \
62+
$::VALKEY_CLI_BIN --cluster rebalance \
6363
127.0.0.1:[get_instance_attrib valkey 0 port] \
6464
{*}[valkeycli_tls_config "../../../tests"] \
6565
--cluster-weight ${master0_id}=.01 \

‎tests/cluster/tests/12.1-replica-migration-3.tcl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ test "Set allow-replica-migration no" {
3737
set master0_id [dict get [get_myself 0] id]
3838
test "Resharding all the master #0 slots away from it" {
3939
set output [exec \
40-
../../../src/valkey-cli --cluster rebalance \
40+
$::VALKEY_CLI_BIN --cluster rebalance \
4141
127.0.0.1:[get_instance_attrib valkey 0 port] \
4242
{*}[valkeycli_tls_config "../../../tests"] \
4343
--cluster-weight ${master0_id}=0 >@ stdout ]

‎tests/cluster/tests/includes/utils.tcl‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ proc config_set_all_nodes {keyword value} {
88

99
proc fix_cluster {addr} {
1010
set code [catch {
11-
exec ../../../src/valkey-cli {*}[valkeycli_tls_config "../../../tests"] --cluster fix $addr << yes
11+
exec $::VALKEY_CLI_BIN {*}[valkeycli_tls_config "../../../tests"] --cluster fix $addr << yes
1212
} result]
1313
if {$code != 0} {
1414
puts "valkey-cli --cluster fix returns non-zero exit code, output below:\n$result"
@@ -17,7 +17,7 @@ proc fix_cluster {addr} {
1717
# but we can ignore that and rely on the check below.
1818
assert_cluster_state ok
1919
wait_for_condition 100 100 {
20-
[catch {exec ../../../src/valkey-cli {*}[valkeycli_tls_config "../../../tests"] --cluster check $addr} result] == 0
20+
[catch {exec $::VALKEY_CLI_BIN {*}[valkeycli_tls_config "../../../tests"] --cluster check $addr} result] == 0
2121
} else {
2222
puts "valkey-cli --cluster check returns non-zero exit code, output below:\n$result"
2323
fail "Cluster could not settle with configuration"
@@ -26,7 +26,7 @@ proc fix_cluster {addr} {
2626

2727
proc wait_cluster_stable {} {
2828
wait_for_condition 1000 50 {
29-
[catch {exec ../../../src/valkey-cli --cluster \
29+
[catch {exec $::VALKEY_CLI_BIN --cluster \
3030
check 127.0.0.1:[get_instance_attrib valkey 0 port] \
3131
{*}[valkeycli_tls_config "../../../tests"] \
3232
}] == 0

‎tests/instances.tcl‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,18 @@ if {[catch {cd tmp}]} {
4848
# the provided configuration file. Returns the PID of the process.
4949
proc exec_instance {type dirname cfgfile} {
5050
if {$type eq "valkey"} {
51-
set prgname valkey-server
51+
set program_path $::VALKEY_SERVER_BIN
5252
} elseif {$type eq "sentinel"} {
53-
set prgname valkey-sentinel
53+
set program_path $::VALKEY_SENTINEL_BIN
5454
} else {
5555
error "Unknown instance type."
5656
}
5757

5858
set errfile [file join $dirname err.txt]
5959
if {$::valgrind} {
60-
set pid [exec valgrind --track-origins=yes --suppressions=../../../src/valgrind.sup --show-reachable=no --show-possibly-lost=no --leak-check=full ../../../src/${prgname} $cfgfile 2>> $errfile &]
60+
set pid [exec valgrind --track-origins=yes --suppressions=../../../src/valgrind.sup --show-reachable=no --show-possibly-lost=no --leak-check=full ${program_path} $cfgfile 2>> $errfile &]
6161
} else {
62-
set pid [exec ../../../src/${prgname} $cfgfile 2>> $errfile &]
62+
set pid [exec ${program_path} $cfgfile 2>> $errfile &]
6363
}
6464
return $pid
6565
}

‎tests/integration/aof-race.tcl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ tags {"aof external:skip"} {
88
# was subsequently appended to the new AOF, resulting in duplicate commands.
99
start_server_aof [list dir $server_path] {
1010
set client [valkey [srv host] [srv port] 0 $::tls]
11-
set bench [open "|src/valkey-benchmark -q -s [srv unixsocket] -c 20 -n 20000 incr foo" "r+"]
11+
set bench [open "|$::VALKEY_BENCHMARK_BIN -q -s [srv unixsocket] -c 20 -n 20000 incr foo" "r+"]
1212

1313
wait_for_condition 100 1 {
1414
[$client get foo] > 0

0 commit comments

Comments
 (0)