From 30f277a86d35e3f70314f50ca7b59562838bd429 Mon Sep 17 00:00:00 2001
From: Ping Xie <pingxie@google.com>
Date: Sun, 2 Jun 2024 13:15:08 -0700
Subject: [PATCH] Enable debug asserts for cluster and sentinel tests (#588)

Also make `enable-debug-assert` an immutable config

Address review comments in #584

---------

Signed-off-by: Ping Xie <pingxie@google.com>
---
 .gitignore          | 1 +
 src/config.c        | 2 +-
 src/object.c        | 5 +----
 src/server.h        | 3 ++-
 tests/instances.tcl | 2 ++
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/.gitignore b/.gitignore
index e745f76a04..c660bc3600 100644
--- a/.gitignore
+++ b/.gitignore
@@ -45,3 +45,4 @@ redis.code-workspace
 .cache
 .cscope*
 .swp
+tests/cluster/tmp/*
diff --git a/src/config.c b/src/config.c
index 7dc86fbf39..3cd0cc5def 100644
--- a/src/config.c
+++ b/src/config.c
@@ -3054,6 +3054,7 @@ standardConfig static_configs[] = {
     createBoolConfig("aof-disable-auto-gc", NULL, MODIFIABLE_CONFIG | HIDDEN_CONFIG, server.aof_disable_auto_gc, 0, NULL, updateAofAutoGCEnabled),
     createBoolConfig("replica-ignore-disk-write-errors", NULL, MODIFIABLE_CONFIG, server.repl_ignore_disk_write_error, 0, NULL, NULL),
     createBoolConfig("extended-redis-compatibility", NULL, MODIFIABLE_CONFIG, server.extended_redis_compat, 0, NULL, updateExtendedRedisCompat),
+    createBoolConfig("enable-debug-assert", NULL, IMMUTABLE_CONFIG | HIDDEN_CONFIG, server.enable_debug_assert, 0, NULL, NULL),
 
     /* String Configs */
     createStringConfig("aclfile", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.acl_filename, "", NULL, NULL),
@@ -3191,7 +3192,6 @@ standardConfig static_configs[] = {
     createTimeTConfig("repl-backlog-ttl", NULL, MODIFIABLE_CONFIG, 0, LONG_MAX, server.repl_backlog_time_limit, 60 * 60, INTEGER_CONFIG, NULL, NULL), /* Default: 1 hour */
     createOffTConfig("auto-aof-rewrite-min-size", NULL, MODIFIABLE_CONFIG, 0, LLONG_MAX, server.aof_rewrite_min_size, 64 * 1024 * 1024, MEMORY_CONFIG, NULL, NULL),
     createOffTConfig("loading-process-events-interval-bytes", NULL, MODIFIABLE_CONFIG | HIDDEN_CONFIG, 1024, INT_MAX, server.loading_process_events_interval_bytes, 1024 * 1024 * 2, INTEGER_CONFIG, NULL, NULL),
-    createBoolConfig("enable-debug-assert", NULL, MODIFIABLE_CONFIG | HIDDEN_CONFIG, server.enable_debug_assert, 0, NULL, NULL),
 
     /* Tls configs */
     createIntConfig("tls-port", NULL, MODIFIABLE_CONFIG, 0, 65535, server.tls_port, 0, INTEGER_CONFIG, NULL, applyTLSPort), /* TCP port. */
diff --git a/src/object.c b/src/object.c
index ea3538b9ee..814b8de3a9 100644
--- a/src/object.c
+++ b/src/object.c
@@ -647,10 +647,7 @@ robj *tryObjectEncodingEx(robj *o, int try_trim) {
          * Note that we avoid using shared integers when maxmemory is used
          * because every object needs to have a private LRU field for the LRU
          * algorithm to work well. */
-        if (canUseSharedObject() &&
-            value >= 0 &&
-            value < OBJ_SHARED_INTEGERS)
-        {
+        if (canUseSharedObject() && value >= 0 && value < OBJ_SHARED_INTEGERS) {
             decrRefCount(o);
             return shared.integers[value];
         } else {
diff --git a/src/server.h b/src/server.h
index 8f273db1e8..66f2c81c4d 100644
--- a/src/server.h
+++ b/src/server.h
@@ -701,10 +701,11 @@ typedef enum {
 #define serverAssert(_e) (likely(_e) ? (void)0 : (_serverAssert(#_e, __FILE__, __LINE__), valkey_unreachable()))
 #define serverPanic(...) _serverPanic(__FILE__, __LINE__, __VA_ARGS__), valkey_unreachable()
 
-/* The following macro provides a conditional assertion that is only executed
+/* The following macros provide a conditional assertion that is only executed
  * when the server config 'enable-debug-assert' is true. This is useful for adding
  * assertions that are too computationally expensive or risky to run in normal
  * operation, but are valuable for debugging or testing. */
+#define debugServerAssert(...) (server.enable_debug_assert ? serverAssert(__VA_ARGS__) : (void)0)
 #define debugServerAssertWithInfo(...) (server.enable_debug_assert ? serverAssertWithInfo(__VA_ARGS__) : (void)0)
 
 /* latency histogram per command init settings */
diff --git a/tests/instances.tcl b/tests/instances.tcl
index 3b487423f8..782804ddae 100644
--- a/tests/instances.tcl
+++ b/tests/instances.tcl
@@ -118,6 +118,8 @@ proc spawn_instance {type base_port count {conf {}} {base_conf_file ""}} {
         puts $cfg "repl-diskless-sync-delay 0"
         puts $cfg "dir ./$dirname"
         puts $cfg "logfile log.txt"
+        puts $cfg "enable-debug-assert yes"
+
         # Add additional config files
         foreach directive $conf {
             puts $cfg $directive