Spike: replace libtap with cmocka

.github/workflows/clang-addresssanitizer.yml

@@ -21,7 +21,7 @@ jobs:
         with:
           submodules: true
           persist-credentials: false
-      - run: sudo apt install clang libipc-run3-perl
+      - run: sudo apt install clang libcmocka-dev libipc-run3-perl
       - run: ./bootstrap
       - run: ./configure
       - run: make

.github/workflows/clang-analyzer.yml

@@ -13,7 +13,7 @@ jobs:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
         with:
           persist-credentials: false
-      - run: sudo apt install clang-tools libipc-run3-perl
+      - run: sudo apt install clang-tools libcmocka-dev libipc-run3-perl
       - run: ./bootstrap
       - run: scan-build ./configure
         env:

.github/workflows/codeql-analysis.yml

@@ -35,7 +35,7 @@ jobs:
     - name: Initialize CodeQL
       uses: github/codeql-action/init@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9
 
-    - run: sudo apt install libipc-run3-perl pandoc
+    - run: sudo apt install libcmocka-dev libipc-run3-perl pandoc
     - run: |
             ./bootstrap
             ./configure

.github/workflows/test.yml

@@ -8,11 +8,9 @@ permissions: {}
 jobs:
   # The -m32 variants build and test as 32-bit. ssize_t and long are 32 bits
   # wide on i686, which exposes signedness and range problems that the 64-bit
-  # builds cannot see. The flag goes in CC rather than CFLAGS so that the
-  # libtap build, which has its own Makefile, and the test scripts that
-  # compile code themselves pick it up too. CFLAGS must stay out of the job
-  # environment: when it is set, even empty, make exports the configured
-  # CFLAGS to libtap, and libtap does not build on macOS with _POSIX_C_SOURCE.
+  # builds cannot see. The flag goes in CC rather than CFLAGS so that the test
+  # scripts that compile code themselves pick it up too. The -m32 jobs build
+  # cmocka from source because Ubuntu does not ship a 32-bit libcmocka-dev.
   test-autoconf:
     strategy:
       matrix:
@@ -36,9 +34,22 @@ jobs:
           persist-credentials: false
       - run: sudo apt install libipc-run3-perl
         if: ${{ matrix.os == 'ubuntu-latest' }}
+      - run: sudo apt install libcmocka-dev
+        if: ${{ matrix.os == 'ubuntu-latest' && matrix.arch == '' }}
       - run: sudo apt-get update && sudo apt-get install -y gcc-multilib g++-multilib
         if: ${{ matrix.os == 'ubuntu-latest' && matrix.arch == '-m32' }}
-      - run: brew install autoconf automake libtool
+      - name: Build 32-bit cmocka
+        if: ${{ matrix.arch == '-m32' }}
+        run: |
+          curl -sSLO https://cmocka.org/files/2.0/cmocka-2.0.1.tar.xz
+          echo '3f3533382ba29ab3abf5c4f4b27b79d165f0df51ea587de749b11b68b4019180  cmocka-2.0.1.tar.xz' | sha256sum -c
+          tar xf cmocka-2.0.1.tar.xz
+          cmake -S cmocka-2.0.1 -B cmocka-build -DCMAKE_C_FLAGS=-m32 -DWITH_EXAMPLES=OFF -DCMAKE_INSTALL_PREFIX="$HOME/cmocka"
+          cmake --build cmocka-build
+          cmake --install cmocka-build
+          echo "PKG_CONFIG_PATH=$HOME/cmocka/lib/pkgconfig" >> "$GITHUB_ENV"
+          echo "LD_LIBRARY_PATH=$HOME/cmocka/lib" >> "$GITHUB_ENV"
+      - run: brew install autoconf automake libtool cmocka pkgconf
         if: ${{ matrix.os == 'macos-latest' }}
       - run: ./bootstrap
       - run: ./configure

.gitmodules

@@ -1,6 +1,3 @@
-[submodule "t/libtap"]
-	path = t/libtap
-	url = https://github.com/zorgnax/libtap.git
 [submodule "t/maxmind-db"]
 	path = t/maxmind-db
 	url = https://github.com/maxmind/MaxMind-DB.git

.precious.toml

@@ -1,7 +1,6 @@
 exclude = [
     ".git",
     "maxmind-db/**",
-    "t/libtap/**",
     "t/maxmind-db/**",
 ]
 

CMakeLists.txt

@@ -1,4 +1,4 @@
-cmake_minimum_required (VERSION 3.9...3.30)
+cmake_minimum_required (VERSION 3.14...3.30)
 
 project(maxminddb
   LANGUAGES C

Changes.md

@@ -1,5 +1,10 @@
 ## next release
 
+- The C tests now use [cmocka](https://cmocka.org/) instead of the bundled
+  `libtap` submodule. Building the tests requires cmocka to be installed. The
+  autotools build finds it with `pkg-config`. The CMake build finds it with
+  `find_package` and downloads it when it is not installed. The CMake build now
+  requires CMake 3.14 or later.
 - Fixed a `-Wsign-compare` warning in `MMDB_open()` that broke builds with
   `-Werror` on platforms where `ssize_t` is 32 bits, such as i686. Pull request
   by Robert Scheck. GitHub #487.

README.dev.md

@@ -51,7 +51,7 @@ version updated promptly for some reason.
 
 - Required packages (Ubuntu 25.10): vim git-core dput build-essential autoconf
   automake libtool git-buildpackage lowdown dirmngr debhelper dh-autoreconf
-  libipc-run3-perl libtest-output-perl devscripts
+  libcmocka-dev libipc-run3-perl libtest-output-perl devscripts
 - Install [gh](https://github.com/cli/cli/releases).
 - GitHub ssh key (e.g. in `~/.ssh/id_rsa`)
 - Git config (e.g. `~/.gitconfig`)

README.md

@@ -39,7 +39,9 @@ sudo ldconfig
 ```
 
 You can skip the `make check` step but it's always good to know that tests are
-passing on your platform.
+passing on your platform. The tests need [cmocka](https://cmocka.org/), for
+example the `libcmocka-dev` package on Debian and Ubuntu or the `cmocka`
+formula on Homebrew.
 
 The `configure` script takes the standard options to set where files are
 installed such as `--prefix`, etc. See `./configure --help` for details.

configure.ac

@@ -130,6 +130,7 @@ AC_ARG_ENABLE([tests],
         [enable_tests=${enableval}],
         [enable_tests=yes])
 AM_CONDITIONAL([TESTS], [test "${enable_tests}" = "yes"])
+AS_IF([test "${enable_tests}" = "yes"], [PKG_CHECK_MODULES([CMOCKA], [cmocka])])
 
 AC_CONFIG_FILES([Makefile
                  src/Makefile

src/Makefile.am

@@ -18,12 +18,9 @@ TESTS = test-data-pool
 check_PROGRAMS = test-data-pool
 
 test_data_pool_SOURCES = data-pool.c data-pool.h
-test_data_pool_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/t -DTEST_DATA_POOL
-test_data_pool_LDADD = $(top_srcdir)/t/libmmdbtest.la \
-	$(top_srcdir)/t/libtap/libtap.a
+test_data_pool_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/t -DTEST_DATA_POOL \
+	$(CMOCKA_CFLAGS)
+test_data_pool_LDADD = $(top_srcdir)/t/libmmdbtest.la $(CMOCKA_LIBS)
 
 $(top_srcdir)/t/libmmdbtest.la:
 	$(MAKE) -C $(top_srcdir)/t libmmdbtest.la
-
-$(top_srcdir)/t/libtap/libtap.a:
-	$(MAKE) -C $(top_srcdir)/t/libtap libtap.a

src/data-pool.c

@@ -162,30 +162,22 @@ MMDB_entry_data_list_s *data_pool_to_list(MMDB_data_pool_s *const pool) {
 
 #ifdef TEST_DATA_POOL
 
-    #include <libtap/tap.h>
     #include <maxminddb_test_helper.h>
 
-static void test_can_multiply(void);
-
-int main(void) {
-    plan(NO_PLAN);
-    test_can_multiply();
-    done_testing();
+static void test_can_multiply(void **UNUSED(state)) {
+    assert_true_desc(can_multiply(SIZE_MAX, 1, SIZE_MAX), "1*SIZE_MAX is ok");
+    assert_true_desc(!can_multiply(SIZE_MAX, 2, SIZE_MAX),
+                     "2*SIZE_MAX is not ok");
+    assert_true_desc(
+        can_multiply(SIZE_MAX, 10240, sizeof(MMDB_entry_data_list_s)),
+        "1024 entry_data_list_s's are okay");
 }
 
-static void test_can_multiply(void) {
-    {
-        ok(can_multiply(SIZE_MAX, 1, SIZE_MAX), "1*SIZE_MAX is ok");
-    }
-
-    {
-        ok(!can_multiply(SIZE_MAX, 2, SIZE_MAX), "2*SIZE_MAX is not ok");
-    }
-
-    {
-        ok(can_multiply(SIZE_MAX, 10240, sizeof(MMDB_entry_data_list_s)),
-           "1024 entry_data_list_s's are okay");
-    }
+int main(void) {
+    const struct CMUnitTest tests[] = {
+        cmocka_unit_test(test_can_multiply),
+    };
+    return cmocka_run_group_tests(tests, NULL, NULL);
 }
 
 #endif

t/CMakeLists.txt

@@ -1,6 +1,33 @@
-add_library(tap
-  libtap/tap.c
-)
+find_package(cmocka CONFIG QUIET)
+if(NOT cmocka_FOUND)
+  message(STATUS "cmocka not found, building it from source")
+  include(FetchContent)
+  # These only apply to the cmocka subdirectory.
+  set(BUILD_SHARED_LIBS OFF)
+  set(WITH_EXAMPLES OFF)
+  set(UNIT_TESTING OFF)
+  set(CMOCKA_URL https://cmocka.org/files/2.0/cmocka-2.0.1.tar.xz)
+  set(CMOCKA_SHA256 3f3533382ba29ab3abf5c4f4b27b79d165f0df51ea587de749b11b68b4019180)
+  # EXCLUDE_FROM_ALL keeps cmocka out of the install tree.
+  if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.28)
+    FetchContent_Declare(cmocka
+      URL ${CMOCKA_URL}
+      URL_HASH SHA256=${CMOCKA_SHA256}
+      EXCLUDE_FROM_ALL
+    )
+    FetchContent_MakeAvailable(cmocka)
+  else()
+    FetchContent_Declare(cmocka
+      URL ${CMOCKA_URL}
+      URL_HASH SHA256=${CMOCKA_SHA256}
+    )
+    FetchContent_GetProperties(cmocka)
+    if(NOT cmocka_POPULATED)
+      FetchContent_Populate(cmocka)
+      add_subdirectory(${cmocka_SOURCE_DIR} ${cmocka_BINARY_DIR} EXCLUDE_FROM_ALL)
+    endif()
+  endif()
+endif()
 
 # test programs
 set(TEST_TARGET_NAMES
@@ -41,17 +68,10 @@ if(UNIX)  # or if (NOT WIN32)
   find_package(Threads)
 endif()
 
-if(WIN32)
-  # 4244, 4267 - libtap causes a significant number of conversion warning in
-  #              our tests on Windows.
-  # 4996 - vsprintf used by libtap is unsafe.
-  add_definitions("/wd4244 /wd4267 /wd4996")
-endif(WIN32)
-
 foreach(TEST_TARGET_NAME ${TEST_TARGET_NAMES})
   add_executable(${TEST_TARGET_NAME} ${TEST_TARGET_NAME}.c maxminddb_test_helper.c)
   target_include_directories(${TEST_TARGET_NAME} PRIVATE ../src)
-  target_link_libraries(${TEST_TARGET_NAME} maxminddb tap)
+  target_link_libraries(${TEST_TARGET_NAME} maxminddb cmocka::cmocka)
   target_compile_definitions(${TEST_TARGET_NAME} PRIVATE PACKAGE_VERSION="${PROJECT_VERSION}")
 
   if(UNIX)

t/Makefile.am

@@ -1,20 +1,13 @@
 include $(top_srcdir)/common.mk
 
-all-local:
-		cd libtap && $(MAKE) $(AM_MAKEFLAGS) all
-clean-local:
-		cd libtap && $(MAKE) $(AM_MAKEFLAGS) clean
-
 AM_LDFLAGS = $(top_builddir)/src/libmaxminddb.la
-CFLAGS += -I$(top_srcdir)/src
+CFLAGS += -I$(top_srcdir)/src $(CMOCKA_CFLAGS)
 
 noinst_LTLIBRARIES = libmmdbtest.la
 libmmdbtest_la_SOURCES = maxminddb_test_helper.c maxminddb_test_helper.h
 
 EXTRA_DIST = compile_c++_t.pl decoder_limits_t.pl external_symbols_t.pl \
-	     mmdblookup_t.pl \
-	     libtap/COPYING libtap/INSTALL libtap/Makefile libtap/README.md \
-	     libtap/tap.c libtap/tap.h maxmind-db
+	     mmdblookup_t.pl maxmind-db
 
 check_PROGRAMS = \
 	bad_pointers_t bad_databases_t bad_data_size_t bad_epoch_t bad_indent_t \
@@ -36,4 +29,4 @@ threads_t_CFLAGS = $(CFLAGS) -pthread
 TESTS = $(check_PROGRAMS) compile_c++_t.pl decoder_limits_t.pl \
 	external_symbols_t.pl mmdblookup_t.pl
 
-LDADD = libmmdbtest.la libtap/libtap.a
+LDADD = libmmdbtest.la $(CMOCKA_LIBS)

t/bad_data_size_t.c

@@ -1,68 +1,56 @@
 #include "maxminddb_test_helper.h"
 
-void test_bad_data_size_rejected(void) {
+void test_bad_data_size_rejected(void **UNUSED(state)) {
     char *db_file = bad_database_path("libmaxminddb-oversized-array.mmdb");
 
     MMDB_s mmdb;
     int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb);
-    cmp_ok(status, "==", MMDB_SUCCESS, "opened bad-data-size MMDB");
-
-    if (status != MMDB_SUCCESS) {
-        diag("MMDB_open failed: %s", MMDB_strerror(status));
-        free(db_file);
-        return;
-    }
+    assert_int_equal_desc(status, MMDB_SUCCESS, "opened bad-data-size MMDB");
 
     int gai_error, mmdb_error;
     MMDB_lookup_result_s result =
         MMDB_lookup_string(&mmdb, "1.2.3.4", &gai_error, &mmdb_error);
 
-    cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "lookup succeeded");
-    ok(result.found_entry, "entry found");
+    assert_int_equal_desc(mmdb_error, MMDB_SUCCESS, "lookup succeeded");
+    assert_true_desc(result.found_entry, "entry found");
 
     if (result.found_entry) {
         MMDB_entry_data_list_s *entry_data_list = NULL;
         status = MMDB_get_entry_data_list(&result.entry, &entry_data_list);
-        cmp_ok(status,
-               "==",
-               MMDB_INVALID_DATA_ERROR,
-               "MMDB_get_entry_data_list returns INVALID_DATA_ERROR "
-               "for array with size exceeding remaining data");
+        assert_int_equal_desc(
+            status,
+            MMDB_INVALID_DATA_ERROR,
+            "MMDB_get_entry_data_list returns INVALID_DATA_ERROR "
+            "for array with size exceeding remaining data");
         MMDB_free_entry_data_list(entry_data_list);
     }
 
     MMDB_close(&mmdb);
     free(db_file);
 }
 
-void test_bad_map_size_rejected(void) {
+void test_bad_map_size_rejected(void **UNUSED(state)) {
     char *db_file = bad_database_path("libmaxminddb-oversized-map.mmdb");
 
     MMDB_s mmdb;
     int status = MMDB_open(db_file, MMDB_MODE_MMAP, &mmdb);
-    cmp_ok(status, "==", MMDB_SUCCESS, "opened bad-map-size MMDB");
-
-    if (status != MMDB_SUCCESS) {
-        diag("MMDB_open failed: %s", MMDB_strerror(status));
-        free(db_file);
-        return;
-    }
+    assert_int_equal_desc(status, MMDB_SUCCESS, "opened bad-map-size MMDB");
 
     int gai_error, mmdb_error;
     MMDB_lookup_result_s result =
         MMDB_lookup_string(&mmdb, "1.2.3.4", &gai_error, &mmdb_error);
 
-    cmp_ok(mmdb_error, "==", MMDB_SUCCESS, "lookup succeeded");
-    ok(result.found_entry, "entry found");
+    assert_int_equal_desc(mmdb_error, MMDB_SUCCESS, "lookup succeeded");
+    assert_true_desc(result.found_entry, "entry found");
 
     if (result.found_entry) {
         MMDB_entry_data_list_s *entry_data_list = NULL;
         status = MMDB_get_entry_data_list(&result.entry, &entry_data_list);
-        cmp_ok(status,
-               "==",
-               MMDB_INVALID_DATA_ERROR,
-               "MMDB_get_entry_data_list returns INVALID_DATA_ERROR "
-               "for map with size exceeding remaining data");
+        assert_int_equal_desc(
+            status,
+            MMDB_INVALID_DATA_ERROR,
+            "MMDB_get_entry_data_list returns INVALID_DATA_ERROR "
+            "for map with size exceeding remaining data");
         MMDB_free_entry_data_list(entry_data_list);
     }
 
@@ -71,8 +59,9 @@ void test_bad_map_size_rejected(void) {
 }
 
 int main(void) {
-    plan(NO_PLAN);
-    test_bad_data_size_rejected();
-    test_bad_map_size_rejected();
-    done_testing();
+    const struct CMUnitTest tests[] = {
+        cmocka_unit_test(test_bad_data_size_rejected),
+        cmocka_unit_test(test_bad_map_size_rejected),
+    };
+    return cmocka_run_group_tests(tests, NULL, NULL);
 }