Introduce GoogleTest framework for Valkey unit testing #2956

harrylin98 · Dec 19, 2025

This PR adds GoogleTest (gtest) support to Valkey to enable writing modern unit tests.

Motivation:
GoogleTest provides richer assertions, test fixtures, mocking support, and improved diagnostics, helping improve test coverage and maintainability over time.

Summary:
The change is limited to test infrastructure, and existing C unit tests remain unchanged.

This PR focuses only on integrating the framework and includes a small set of example tests to demonstrate usage.

For more details, see src/gtest/README.md.

Signed-off-by: Harry Lin <harrylhl@amazon.com>

Co-authored-by: Jim Brunner <brunnerj@amazon.com> Signed-off-by: Harry Lin <harrylhl@amazon.com>

Signed-off-by: Harry Lin <harrylhl@amazon.com>

codecov · Dec 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.92%. Comparing base (328b3ce) to head (1224f3e).
⚠️ Report is 1 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #2956      +/-   ##
============================================
+ Coverage     73.69%   73.92%   +0.23%     
============================================
  Files           125      125              
  Lines         69138    69345     +207     
============================================
+ Hits          50949    51266     +317     
+ Misses        18189    18079     -110

Files with missing lines	Coverage Δ
src/networking.c	`88.34% <100.00%> (ø)`
src/sds.c	`88.71% <100.00%> (+0.34%)`	⬆️
src/sds.h	`82.66% <ø> (ø)`
src/server.c	`88.71% <100.00%> (-0.03%)`	⬇️
src/server.h	`100.00% <ø> (ø)`

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JimB123

I like the documentation and the integration with the current unit tests. We can see the example tests already running as the test builds are executed. Nice work pulling this all together and ensuring support across all of the platforms.

This provides the capability for much richer unit testing.

I look forward to using this during the development of the forkless capabilities: #2878

madolson · Dec 19, 2025

Reposting my top level comment:s

I don't think we should have two unit testing frameworks. I feel like we need to commit to incorporating the google testing framework. I'm not saying that is a blocker to start, but we should agree to move what we have.
We typically deep copy dependencies to make vending easier, maybe we don't care in this case because it is optional and not required? Not a particularly strong opinion.

madolson

I think there is a bit more cleanup to do in make files, but I think they're close as of right now.

Left some other comments.

src/gtest/README.md

madolson · Dec 19, 2025

src/gtest/example_tests.cpp

+}
+
+// Verify mocking works via zfree
+TEST_F(ExampleTest, TestMocking) {


I think this is the type of mocking that got ElastiCache into trouble. This is trying to make sure zfree is called, but there are better systemic ways like deprecating not using free (See

valkey/src/server.h

Line 4077 in 0a9cb98

void free(void *ptr) __attribute__((deprecated));

)

madolson · Dec 19, 2025

src/gtest/wrappers.h

+#include "ae.h"
+#include "dict.h"
+#include "server.h"
+#include "adlist.h"
+#include "zmalloc.h"


Do we need server.h here? I feel like a bunch of these are not useful.

server.h pulls in core headers and ensures consistent C/C++ compatibility, so I’d suggest keeping it included.

The main Valkey project doesn't guarantee C/C++ compatibility, it's a C project. I don't want to enforce that as a constraint.

madolson · Dec 19, 2025

src/gtest/wrappers.h

+
+#ifndef __WRAPPERS_H
+#define __WRAPPERS_H
+#define _Atomic /* nothing */


What does the nothing comment mean?

It means _Atomic is being defined as an empty macro.

This strips the _Atomic keyword so the code can compile as C++ (where _Atomic is not a valid keyword). This is a compatibility trick when sharing headers between C and C++.

Can you document that instead of a "trick".

madolson · Dec 19, 2025

src/gtest/wrappers.h

+ *       See: https://github.com/google/googletest/blob/master/googlemock/docs/gmock_faq.md#can-i-mock-a-variadic-function
+ *       Example: serverLog(int level, const char *fmt, ...) should NOT be mocked.
+ */
+long long __wrap_aeCreateTimeEvent(aeEventLoop *eventLoop, long long milliseconds, aeTimeProc *proc, void *clientData, aeEventFinalizerProc *finalizerProc);


I would like to just keep this one, and remove the other wraps. I don't think you should be mocking creation of basic objects. I think you want to be mocking things you don't get deterministic results from (like timers, network, disk etc.

madolson · Dec 19, 2025

src/gtest/Makefile

+	@echo '$(TEST_CFLAGS)' | cmp -s - $@ || echo '$(TEST_CFLAGS)' > $@
+
+# OSS Valkey library dependencies
+VALKEY_LIB := ../libvalkey.a


The Valkey client is called libvalkey. We build this as valkey.a in the top level directory. I don't know if we need to rebuild it here, and if we do, let's call it something else.

src/gtest/wrappers.h

madolson · Dec 19, 2025

src/gtest/Makefile

+-include ../.make-settings
+OPT=-O3 -DNDEBUG
+
+ifeq ($(DEBUG_BUILD),1)


What is this DEBUG_BUILD option? EDIT: I think this might be an AWS thing.

madolson · Dec 19, 2025

src/gtest/example_tests.cpp

+    #include "dict.h"
+}
+
+// Use a class name descriptive of your test unit


I think it is required to end with TEST no? Would be useful to document any other requirements since this is the example.

madolson · Dec 19, 2025

src/gtest/example_tests.cpp

We should add a style checker for CPP tests.

.github/workflows/ci.yml

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

madolson · Dec 20, 2025

@harrylhl Also check the 32 bit build, which seems to be failing now.

zuiderkwast · Dec 21, 2025

We typically deep copy dependencies to make vending easier, maybe we don't care in this case because it is optional and not required? Not a particularly strong opinion.

I prefer that we don't vendor it, since it's optional and only for developers, not useful for end users. I think we should handle it like we handle the optional OpenSSL, i.e. the user has to install it locally using apt install or similar.

madolson · Dec 21, 2025

I prefer that we don't vendor it

That's a good point. It seems to available on hombrew and most most package managers. Let's do that!

Co-authored-by: Madelyn Olson <madelyneolson@gmail.com> Signed-off-by: Harry Lin <49881386+harrylin98@users.noreply.github.com>

Signed-off-by: Harry Lin <harrylhl@amazon.com>

harrylhl and others added 8 commits December 16, 2025 20:41

Add gtest support for unit testing

c11fc88

Signed-off-by: Harry Lin <harrylhl@amazon.com>

gtest rev2

35b1e10

Signed-off-by: Harry Lin <harrylhl@amazon.com>

Apply suggestions from code review

bc0e5fa

Co-authored-by: Jim Brunner <brunnerj@amazon.com> Signed-off-by: Harry Lin <harrylhl@amazon.com>

Apply suggestions from code review

81059f2

Co-authored-by: Jim Brunner <brunnerj@amazon.com> Signed-off-by: Harry Lin <harrylhl@amazon.com>

Cleanup googletest and gtest-parallel after tests in ci.yml

9992fb3

Signed-off-by: Harry Lin <harrylhl@amazon.com>

Correct wrappers.h format

e8b4468

Signed-off-by: Harry Lin <harrylhl@amazon.com>

CI job failure fixes

2bc7e8d

Signed-off-by: Harry Lin <harrylhl@amazon.com>

Merge branch 'unstable' into unstable_gtest_PR

dff09f0

Signed-off-by: Harry Lin <harrylhl@amazon.com>

github-actions bot assigned harrylin98 Dec 19, 2025

harrylin98 marked this pull request as ready for review December 19, 2025 23:12

JimB123 approved these changes Dec 19, 2025

View reviewed changes

madolson reviewed Dec 19, 2025

View reviewed changes

madolson added the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Dec 20, 2025

madolson reviewed Dec 20, 2025

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

Remove unnecessary command for cmake

3b06dbe

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

harrylin98 and others added 2 commits December 21, 2025 23:17

Update src/gtest/README.md

e8b192c

Co-authored-by: Madelyn Olson <madelyneolson@gmail.com> Signed-off-by: Harry Lin <49881386+harrylin98@users.noreply.github.com>

Merge branch 'valkey-io:unstable' into unstable_gtest_PR

f10255f

harrylin98 force-pushed the unstable_gtest_PR branch 4 times, most recently from 7afc4cf to efbd4b9 Compare December 23, 2025 00:28

Addressing comments and fix CI job issues.

1224f3e

Signed-off-by: Harry Lin <harrylhl@amazon.com>

harrylin98 force-pushed the unstable_gtest_PR branch from efbd4b9 to 1224f3e Compare December 23, 2025 05:39

Search code, repositories, users, issues, pull requests...

Introduce GoogleTest framework for Valkey unit testing #2956

Are you sure you want to change the base?

Introduce GoogleTest framework for Valkey unit testing #2956

Uh oh!

Conversation

harrylin98 commented Dec 19, 2025

This PR adds GoogleTest (gtest) support to Valkey to enable writing modern unit tests.

Uh oh!

codecov bot commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JimB123 left a comment

Choose a reason for hiding this comment

Uh oh!

madolson commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

madolson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

madolson Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madolson Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

madolson Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madolson Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

madolson commented Dec 20, 2025

Uh oh!

zuiderkwast commented Dec 21, 2025

Uh oh!

madolson commented Dec 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Dec 19, 2025 •

edited

Loading

madolson commented Dec 19, 2025 •

edited

Loading

madolson Dec 19, 2025 •

edited

Loading

madolson Dec 19, 2025 •

edited

Loading

madolson Dec 19, 2025 •

edited

Loading

madolson Dec 19, 2025 •

edited

Loading