An initial simple unit test framework (#344)

The core idea was to take a lot of the stuff from the C unity framework
and adapt it a bit here. Each file in the `unit` directory that starts
with `test_` is automatically assumed to be a test suite. Within each
file, all functions that start with `test_` are assumed to be a test.

See unit/README.md for details about the implementation.

Instead of compiling basically a net new binary, the way the tests are
compiled is that the main valkey server is compiled as a static archive,
which we then compile the individual test files against to create a new
test executable. This is not all that important now, other than it makes
the compilation simpler, but what it will allow us to do is overwrite
functions in the archive to enable mocking for cross compilation unit
functions. There are also ways to enable mocking from within the same
compilation unit, but I don't know how important this is.

Tests are also written in one of two styles:
1. Including the header file and directly calling functions from the
archive.
2. Importing the original file, and then calling the functions. This
second approach is cool because we can call static functions. It won't
mess up the archive either.

---------

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>

.github/workflows/ci.yml

@@ -14,7 +14,7 @@ jobs:
     - name: make
       # Fail build if there are warnings
       # build with TLS just for compilation coverage
-      run: make SERVER_CFLAGS='-Werror' BUILD_TLS=yes
+      run: make all-with-unit-tests SERVER_CFLAGS='-Werror' BUILD_TLS=yes
     - name: test
       run: |
         sudo apt-get install tcl8.6 tclx
@@ -27,6 +27,9 @@ jobs:
         make commands.def
         dirty=$(git diff)
         if [[ ! -z  $dirty ]]; then echo $dirty; exit 1; fi
+    - name: unit tests
+      run: |
+        ./src/valkey-unit-tests
 
   test-sanitizer-address:
     runs-on: ubuntu-latest

.github/workflows/daily.yml

@@ -54,7 +54,7 @@ jobs:
         repository: ${{ env.GITHUB_REPOSITORY }}
         ref: ${{ env.GITHUB_HEAD_REF }}
     - name: make
-      run: make SERVER_CFLAGS='-Werror -DSERVER_TEST'
+      run: make all-with-unit-tests SERVER_CFLAGS='-Werror -DSERVER_TEST'
     - name: testprep
       run: sudo apt-get install tcl8.6 tclx
     - name: test
@@ -69,9 +69,12 @@ jobs:
     - name: cluster tests
       if: true && !contains(github.event.inputs.skiptests, 'cluster')
       run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}}
-    - name: unittest
+    - name: legacy unit tests
       if: true && !contains(github.event.inputs.skiptests, 'unittest')
       run: ./src/valkey-server test all --accurate
+    - name: new unit tests
+      if: true && !contains(github.event.inputs.skiptests, 'unittest')
+      run: ./src/valkey-unit-tests --accurate
 
   test-ubuntu-jemalloc-fortify:
     runs-on: ubuntu-latest
@@ -113,9 +116,12 @@ jobs:
     - name: cluster tests
       if: true && !contains(github.event.inputs.skiptests, 'cluster')
       run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}}
-    - name: unittest
+    - name: legacy unit tests
       if: true && !contains(github.event.inputs.skiptests, 'unittest')
       run: ./src/valkey-server test all --accurate
+    - name: new unit tests
+      if: true && !contains(github.event.inputs.skiptests, 'unittest')
+      run: make valkey-unit-tests && ./src/valkey-unit-tests --accurate
 
   test-ubuntu-libc-malloc:
     runs-on: ubuntu-latest
@@ -231,9 +237,12 @@ jobs:
     - name: cluster tests
       if: true && !contains(github.event.inputs.skiptests, 'cluster')
       run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}}
-    - name: unittest
+    - name: legacy unit tests
       if: true && !contains(github.event.inputs.skiptests, 'unittest')
       run: ./src/valkey-server test all --accurate
+    - name: new unit tests
+      if: true && !contains(github.event.inputs.skiptests, 'unittest')
+      run: ./src/valkey-unit-tests --accurate
 
   test-ubuntu-tls:
     runs-on: ubuntu-latest
@@ -589,7 +598,7 @@ jobs:
           repository: ${{ env.GITHUB_REPOSITORY }}
           ref: ${{ env.GITHUB_HEAD_REF }}
       - name: make
-        run: make SANITIZER=address SERVER_CFLAGS='-DSERVER_TEST -Werror -DDEBUG_ASSERTIONS'
+        run: make all-with-unit-tests SANITIZER=address SERVER_CFLAGS='-DSERVER_TEST -Werror -DDEBUG_ASSERTIONS'
       - name: testprep
         # Work around ASAN issue, see https://github.com/google/sanitizers/issues/1716
         run: |
@@ -608,9 +617,12 @@ jobs:
       - name: cluster tests
         if: true && !contains(github.event.inputs.skiptests, 'cluster')
         run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}}
-      - name: unittest
+      - name: legacy unit tests
         if: true && !contains(github.event.inputs.skiptests, 'unittest')
         run: ./src/valkey-server test all
+      - name: new unit tests
+        if: true && !contains(github.event.inputs.skiptests, 'unittest')
+        run: ./src/valkey-unit-tests
 
   test-sanitizer-undefined:
     runs-on: ubuntu-latest
@@ -638,7 +650,7 @@ jobs:
           repository: ${{ env.GITHUB_REPOSITORY }}
           ref: ${{ env.GITHUB_HEAD_REF }}
       - name: make
-        run: make SANITIZER=undefined SERVER_CFLAGS='-DSERVER_TEST -Werror' LUA_DEBUG=yes # we (ab)use this flow to also check Lua C API violations
+        run: make all-with-unit-tests SANITIZER=undefined SERVER_CFLAGS='-DSERVER_TEST -Werror' LUA_DEBUG=yes # we (ab)use this flow to also check Lua C API violations
       - name: testprep
         run: |
           sudo apt-get update
@@ -655,9 +667,12 @@ jobs:
       - name: cluster tests
         if: true && !contains(github.event.inputs.skiptests, 'cluster')
         run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}}
-      - name: unittest
+      - name: legacy unit tests
         if: true && !contains(github.event.inputs.skiptests, 'unittest')
         run: ./src/valkey-server test all --accurate
+      - name: new unit tests
+        if: true && !contains(github.event.inputs.skiptests, 'unittest')
+        run: ./src/valkey-unit-tests --accurate
 
   test-centos7-jemalloc:
     runs-on: ubuntu-latest

.gitignore

@@ -1,5 +1,6 @@
 .*.swp
 *.o
+*.a
 *.xo
 *.so
 *.d
@@ -12,6 +13,7 @@ dump.rdb
 *-cli
 *-sentinel
 *-server
+*-unit-tests
 doc-tools
 release
 misc/*

src/Makefile

@@ -98,6 +98,15 @@ ifeq ($(USE_JEMALLOC),no)
 	MALLOC=libc
 endif
 
+# Some unit tests compile files a second time to get access to static functions, the "--allow-multiple-definition" flag
+# allows us to do that without an error, by using the first instance of function. This behavior can also be used
+# to tweak behavior of code just for unit tests. The version of ld on MacOS apparently always does this.
+ifneq ($(uname_S),Darwin)
+	ALLOW_DUPLICATE_FLAG=-Wl,--allow-multiple-definition
+else
+	ALLOW_DUPLICATE_FLAG=
+endif
+
 ifdef SANITIZER
 ifeq ($(SANITIZER),address)
 	MALLOC=libc
@@ -357,6 +366,7 @@ else
 endif
 
 SERVER_CC=$(QUIET_CC)$(CC) $(FINAL_CFLAGS)
+SERVER_AR=$(QUIET_AR)$(AR)
 SERVER_LD=$(QUIET_LINK)$(CC) $(FINAL_LDFLAGS)
 ENGINE_INSTALL=$(QUIET_INSTALL)$(INSTALL)
 
@@ -372,6 +382,7 @@ QUIET_CC = @printf '    %b %b\n' $(CCCOLOR)CC$(ENDCOLOR) $(SRCCOLOR)$@$(ENDCOLOR
 QUIET_GEN = @printf '    %b %b\n' $(CCCOLOR)GEN$(ENDCOLOR) $(SRCCOLOR)$@$(ENDCOLOR) 1>&2;
 QUIET_LINK = @printf '    %b %b\n' $(LINKCOLOR)LINK$(ENDCOLOR) $(BINCOLOR)$@$(ENDCOLOR) 1>&2;
 QUIET_INSTALL = @printf '    %b %b\n' $(LINKCOLOR)INSTALL$(ENDCOLOR) $(BINCOLOR)$@$(ENDCOLOR) 1>&2;
+QUIET_AR = @printf '    %b %b\n' $(CCCOLOR)ARCHIVE$(ENDCOLOR) $(BINCOLOR)$@$(ENDCOLOR) 1>&2;
 endif
 
 ifneq (, $(findstring LOG_REQ_RES, $(SERVER_CFLAGS)))
@@ -392,6 +403,10 @@ ENGINE_BENCHMARK_NAME=$(ENGINE_NAME)-benchmark$(PROG_SUFFIX)
 ENGINE_BENCHMARK_OBJ=ae.o anet.o valkey-benchmark.o adlist.o dict.o zmalloc.o serverassert.o release.o crcspeed.o crccombine.o crc64.o siphash.o crc16.o monotonic.o cli_common.o mt19937-64.o strl.o
 ENGINE_CHECK_RDB_NAME=$(ENGINE_NAME)-check-rdb$(PROG_SUFFIX)
 ENGINE_CHECK_AOF_NAME=$(ENGINE_NAME)-check-aof$(PROG_SUFFIX)
+ENGINE_LIB_NAME=lib$(ENGINE_NAME).a
+ENGINE_TEST_FILES:=$(wildcard unit/*.c)
+ENGINE_TEST_OBJ:=$(sort $(patsubst unit/%.c,unit/%.o,$(ENGINE_TEST_FILES)))
+ENGINE_UNIT_TESTS:=$(ENGINE_NAME)-unit-tests$(PROG_SUFFIX)
 ALL_SOURCES=$(sort $(patsubst %.o,%.c,$(ENGINE_SERVER_OBJ) $(ENGINE_CLI_OBJ) $(ENGINE_BENCHMARK_OBJ)))
 
 all: $(SERVER_NAME) $(ENGINE_SENTINEL_NAME) $(ENGINE_CLI_NAME) $(ENGINE_BENCHMARK_NAME) $(ENGINE_CHECK_RDB_NAME) $(ENGINE_CHECK_AOF_NAME) $(TLS_MODULE)
@@ -408,6 +423,9 @@ endif
 
 .PHONY: all
 
+all-with-unit-tests: all $(ENGINE_UNIT_TESTS)
+.PHONY: all
+
 persist-settings: distclean
 	echo STD=$(STD) >> .make-settings
 	echo WARN=$(WARN) >> .make-settings
@@ -442,6 +460,14 @@ endif
 $(SERVER_NAME): $(ENGINE_SERVER_OBJ)
 	$(SERVER_LD) -o $@ $^ ../deps/hiredis/libhiredis.a ../deps/lua/src/liblua.a ../deps/hdr_histogram/libhdrhistogram.a ../deps/fpconv/libfpconv.a $(FINAL_LIBS)
 
+# Valkey static library, used to compile against for unit testing
+$(ENGINE_LIB_NAME): $(ENGINE_SERVER_OBJ)
+	$(SERVER_AR) rcs $@ $^
+
+# valkey-unit-tests
+$(ENGINE_UNIT_TESTS): $(ENGINE_TEST_OBJ) $(ENGINE_LIB_NAME)
+	$(SERVER_LD) $(ALLOW_DUPLICATE_FLAG) -o $@ $^ ../deps/fpconv/libfpconv.a $(FINAL_LIBS)
+
 # valkey-sentinel
 $(ENGINE_SENTINEL_NAME): $(SERVER_NAME)
 	$(ENGINE_INSTALL) $(SERVER_NAME) $(ENGINE_SENTINEL_NAME)
@@ -475,6 +501,9 @@ DEP = $(ENGINE_SERVER_OBJ:%.o=%.d) $(ENGINE_CLI_OBJ:%.o=%.d) $(ENGINE_BENCHMARK_
 %.o: %.c .make-prerequisites
 	$(SERVER_CC) -MMD -o $@ -c $<
 
+unit/%.o: unit/%.c .make-prerequisites
+	$(SERVER_CC) -MMD -o $@ -c $<
+
 # The following files are checked in and don't normally need to be rebuilt. They
 # are built only if python is available and their prereqs are modified.
 ifneq (,$(PYTHON))
@@ -485,12 +514,17 @@ fmtargs.h: ../utils/generate-fmtargs.py
 	$(QUITE_GEN)sed '/Everything below this line/,$$d' $@ > $@.tmp
 	$(QUITE_GEN)$(PYTHON) ../utils/generate-fmtargs.py >> $@.tmp
 	$(QUITE_GEN)mv $@.tmp $@
+
+unit/test_files.h: unit/*.c ../utils/generate-unit-test-header.py
+	$(QUIET_GEN)$(PYTHON) ../utils/generate-unit-test-header.py
+
+unit/test_main.o: unit/test_files.h
 endif
 
 commands.c: $(COMMANDS_DEF_FILENAME).def
 
 clean:
-	rm -rf $(SERVER_NAME) $(ENGINE_SENTINEL_NAME) $(ENGINE_CLI_NAME) $(ENGINE_BENCHMARK_NAME) $(ENGINE_CHECK_RDB_NAME) $(ENGINE_CHECK_AOF_NAME) *.o *.gcda *.gcno *.gcov valkey.info lcov-html Makefile.dep *.so
+	rm -rf $(SERVER_NAME) $(ENGINE_SENTINEL_NAME) $(ENGINE_CLI_NAME) $(ENGINE_BENCHMARK_NAME) $(ENGINE_CHECK_RDB_NAME) $(ENGINE_CHECK_AOF_NAME) $(ENGINE_UNIT_TESTS) $(ENGINE_LIB_NAME) unit/*.o unit/*.d *.o *.gcda *.gcno *.gcov valkey.info lcov-html Makefile.dep *.so
 	rm -f $(DEP)
 
 .PHONY: clean
@@ -506,6 +540,9 @@ distclean: clean
 test: $(SERVER_NAME) $(ENGINE_CHECK_AOF_NAME) $(ENGINE_CLI_NAME) $(ENGINE_BENCHMARK_NAME)
 	@(cd ..; ./runtest)
 
+test-unit: $(ENGINE_UNIT_TESTS)
+	./$(ENGINE_UNIT_TESTS)
+
 test-modules: $(SERVER_NAME)
 	@(cd ..; ./runtest-moduleapi)
 
@@ -533,7 +570,7 @@ bench: $(ENGINE_BENCHMARK_NAME)
 	@echo ""
 	@echo "WARNING: if it fails under Linux you probably need to install libc6-dev-i386"
 	@echo ""
-	$(MAKE) CFLAGS="-m32" LDFLAGS="-m32"
+	$(MAKE) all-with-unit-tests CFLAGS="-m32" LDFLAGS="-m32"
 
 gcov:
 	$(MAKE) SERVER_CFLAGS="-fprofile-arcs -ftest-coverage -DCOVERAGE_TEST" SERVER_LDFLAGS="-fprofile-arcs -ftest-coverage"