From 832f074a1c0f28af417275993944d211365193e1 Mon Sep 17 00:00:00 2001 From: Roshan Khatri <117414976+roshkhatri@users.noreply.github.com> Date: Fri, 14 Mar 2025 15:34:40 -0700 Subject: [PATCH] Enable tests to run with ASAN build. (#45) --------- Signed-off-by: Roshan Khatri --- .github/workflows/ci.yml | 25 ++++++++++++++++++++++++- CMakeLists.txt | 30 +++++++++++++++++++++++++++--- README.md | 5 +++++ build.sh | 13 ++++++++++--- requirements.txt | 2 +- tst/integration/json_test_case.py | 2 +- tst/integration/run.sh | 27 ++++++++++++++++++++++++++- tst/unit/CMakeLists.txt | 8 ++++++++ 8 files changed, 102 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a4cbf5..dd3da8f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,4 +22,27 @@ jobs: python -m pip install --upgrade pip pip install -r requirements.txt - name: Build and Run tests. - run: ./build.sh \ No newline at end of file + run: ./build.sh + + build-asan-ubuntu-latest: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + server_version: ['unstable', '8.0'] + steps: + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + - name: Set the server version for python integration tests + run: echo "SERVER_VERSION=${{ matrix.server_version }}" >> $GITHUB_ENV + - name: Set up Python + uses: actions/setup-python@3542bca2639a428e1796aaa6a2ffef0c0f575566 #v3.1.4 + with: + python-version: '3.9' + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + - name: Build and Run tests. + run: | + export ASAN_BUILD=true + ./build.sh diff --git a/CMakeLists.txt b/CMakeLists.txt index ac6c9c9..a68977a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,6 +19,15 @@ endif() # Project definition project(ValkeyJSONModule VERSION 0.0.9 LANGUAGES C CXX) +# ASAN build option +option(ENABLE_ASAN "Enable Address Sanitizer" OFF) + +# ASan flags configuration +if(ENABLE_ASAN) + message("Building with Address Sanitizer enabled") + set(ASAN_FLAGS "-fsanitize=address") +endif() + # Set the name of the JSON shared library set(JSON_MODULE_LIB json) @@ -37,16 +46,31 @@ if(NOT CFLAGS) # Include debug symbols and set optimize level set(CFLAGS "-g -O3 -fno-omit-frame-pointer -Wall -Werror -Wextra") endif() + +# Add ASan flags if enabled +if(ENABLE_ASAN) + set(CFLAGS "${CFLAGS} ${ASAN_FLAGS}") + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${ASAN_FLAGS}") + set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${ASAN_FLAGS}") +endif() + message("CFLAGS: ${CFLAGS}") -# Download and build Valkey +if(ENABLE_ASAN) + message("Building Valkey engine with Address Sanitizer enabled") + set(BUILD_CMD make distclean && make -j SANITIZER=address) +else() + set(BUILD_CMD make distclean && make -j) +endif() + +# Download and build Valkey with ASAN if enabled ExternalProject_Add( valkey - GIT_REPOSITORY https://github.com/valkey-io/valkey.git # Replace with actual URL + GIT_REPOSITORY https://github.com/valkey-io/valkey.git GIT_TAG ${VALKEY_VERSION} PREFIX ${VALKEY_DOWNLOAD_DIR} CONFIGURE_COMMAND "" - BUILD_COMMAND make distclean && make -j + BUILD_COMMAND ${BUILD_CMD} INSTALL_COMMAND "" BUILD_IN_SOURCE 1 ) diff --git a/README.md b/README.md index c21bf3d..0884c29 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,11 @@ Custom compiler flags can be passed to the build script via environment variable ```text CFLAGS="-O0 -Wno-unused-function" ./build.sh ``` +#### To build the module with ASAN and run tests +```text +export ASAN_BUILD=true +./build.sh +``` #### To build just the module ```text diff --git a/build.sh b/build.sh index 464e809..47dccdc 100755 --- a/build.sh +++ b/build.sh @@ -23,10 +23,17 @@ if [ ! -d "$BUILD_DIR" ]; then mkdir $BUILD_DIR fi cd $BUILD_DIR -if [ -z "${CFLAGS}" ]; then - cmake .. -DVALKEY_VERSION=${SERVER_VERSION} + +if [ ! -z "${ASAN_BUILD}" ]; then + CMAKE_FLAGS="-DCMAKE_BUILD_TYPE=Debug -DENABLE_ASAN=ON" else - cmake .. -DVALKEY_VERSION=${SERVER_VERSION} -DCFLAGS=${CFLAGS} + CMAKE_FLAGS="" +fi + +if [ -z "${CFLAGS}" ]; then + cmake .. -DVALKEY_VERSION=${SERVER_VERSION} ${CMAKE_FLAGS} +else + cmake .. -DVALKEY_VERSION=${SERVER_VERSION} -DCFLAGS=${CFLAGS} ${CMAKE_FLAGS} fi make diff --git a/requirements.txt b/requirements.txt index 9e95d79..04b527f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ valkey -pytest==4 +pytest==6 pytest-html \ No newline at end of file diff --git a/tst/integration/json_test_case.py b/tst/integration/json_test_case.py index 7ee27e9..f38dac7 100644 --- a/tst/integration/json_test_case.py +++ b/tst/integration/json_test_case.py @@ -20,7 +20,7 @@ class SimpleTestCase(ValkeyTestCase): def teardown(self): if self.is_connected(): - self.client.execute_command("FLUSHALL") + self.client.execute_command("FLUSHALL SYNC") logging.info("executed FLUSHALL at teardown") super(SimpleTestCase, self).teardown() diff --git a/tst/integration/run.sh b/tst/integration/run.sh index 7cb2bb5..3f81717 100755 --- a/tst/integration/run.sh +++ b/tst/integration/run.sh @@ -34,7 +34,32 @@ if [[ ! -f "${BINARY_PATH}" ]] ; then fi if [[ $1 == "test" ]] ; then - python -m pytest --html=report.html --cache-clear -v ${TEST_FLAG} ./ ${TEST_PATTERN} + if [ ! -z "${ASAN_BUILD}" ]; then + echo "Running tests and checking for memory leaks" + python -m pytest --capture=sys --html=report.html --cache-clear -v ${TEST_FLAG} ./ ${TEST_PATTERN} 2>&1 | tee test_output.tmp + # Check for memory leaks in the output + if grep -q "LeakSanitizer: detected memory leaks" test_output.tmp; then + RED='\033[0;31m' + echo -e "${RED}Memory leaks detected in the following tests:" + LEAKING_TESTS=$(grep -B 2 "LeakSanitizer: detected memory leaks" test_output.tmp | \ + grep -v "LeakSanitizer" | \ + grep ".*\.py::") + + LEAK_COUNT=$(echo "$LEAKING_TESTS" | wc -l) + + # Output each leaking test + echo "$LEAKING_TESTS" | while read -r line; do + echo "::error::Test with leak: $line" + done + + echo -e "\n$LEAK_COUNT python integration tests have leaks detected in them" + rm test_output.tmp + exit 1 + fi + rm test_output.tmp + else + python -m pytest --html=report.html --cache-clear -v ${TEST_FLAG} ./ ${TEST_PATTERN} + fi else echo "Unknown target: $1" exit 1 diff --git a/tst/unit/CMakeLists.txt b/tst/unit/CMakeLists.txt index 9e1e500..43329d5 100644 --- a/tst/unit/CMakeLists.txt +++ b/tst/unit/CMakeLists.txt @@ -21,6 +21,13 @@ file(GLOB_RECURSE UNIT_TEST_SRC "*.cc") # This defines each unit test and associates it with its sources add_executable(unitTests ${UNIT_TEST_SRC}) +if(ENABLE_ASAN) + message(STATUS "Enabling Address Sanitizer for unit tests") + set(GTEST_PROPERTIES "${CMAKE_CXX_FLAGS}") +else() + set(GTEST_PROPERTIES "") +endif() + # Build with C11 & C++17 set_target_properties( unitTests @@ -30,6 +37,7 @@ set_target_properties( CXX_STANDARD 17 CXX_STANDARD_REQUIRED ON POSITION_INDEPENDENT_CODE ON + COMPILE_FLAGS "${GTEST_PROPERTIES} -Wall -Wextra" ) target_include_directories(unitTests