From d827b11b670f3331e5d924eadfe35c497b08035a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 28 Jun 2019 13:20:54 +0200 Subject: [PATCH] tests: execute leak checker via CTest directly Right now, we have an awful hack in our test CI setup that extracts the test command from CTest's output and then prepends the leak checker. This is dependent on non-machine-parseable output from CMake and also breaks on various ocassions, like for example when we have spaces in the current path or when the path contains backslashes. Both conditions may easily be triggered on Win32 systems, and in fact they do break our Azure Pipelines builds. Remove the awful hack in favour of a new CMake build option "USE_LEAK_CHECKER". If specifying e.g. "-DUSE_LEAK_CHECKER=valgrind", then we will set up all tests to be run under valgrind. Like this, we can again simply execute ctest without needing to rely on evil sourcery. --- CMakeLists.txt | 1 + azure-pipelines.yml | 15 ++++-------- azure-pipelines/nightly.yml | 21 ++++++----------- ci/test.sh | 28 ++--------------------- script/leaks.sh | 6 +++++ script/valgrind.sh | 2 ++ libgit2_clar.supp => script/valgrind.supp | 0 tests/CMakeLists.txt | 20 +++++++++++----- 8 files changed, 37 insertions(+), 56 deletions(-) create mode 100755 script/leaks.sh create mode 100755 script/valgrind.sh rename libgit2_clar.supp => script/valgrind.supp (100%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 292fadd3c..6e6443a0c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -57,6 +57,7 @@ OPTION(USE_HTTPS "Enable HTTPS support. Can be set to a specific backend" ON) OPTION(USE_SHA1 "Enable SHA1. Can be set to CollisionDetection(ON)/HTTPS/Generic" ON) OPTION(USE_GSSAPI "Link with libgssapi for SPNEGO auth" OFF) OPTION(USE_STANDALONE_FUZZERS "Enable standalone fuzzers (compatible with gcc)" OFF) +OPTION(USE_LEAK_CHECKER "Run tests with leak checker" OFF) OPTION(VALGRIND "Configure build for valgrind" OFF) OPTION(DEBUG_POOL "Enable debug pool allocator" OFF) OPTION(ENABLE_WERROR "Enable compilation with -Werror" OFF) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 959960f07..0f92df38c 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -16,8 +16,7 @@ jobs: imageName: 'libgit2/trusty-amd64:latest' environmentVariables: | CC=gcc - CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DREGEX_BACKEND=builtin -DDEPRECATE_HARD=ON - LEAK_CHECK=valgrind + CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DREGEX_BACKEND=builtin -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind - job: linux_amd64_trusty_gcc_mbedtls displayName: 'Linux (amd64; Trusty; GCC; mbedTLS)' @@ -29,8 +28,7 @@ jobs: imageName: 'libgit2/trusty-amd64:latest' environmentVariables: | CC=gcc - CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DDEPRECATE_HARD=ON - LEAK_CHECK=valgrind + CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind - job: linux_amd64_trusty_clang_openssl displayName: 'Linux (amd64; Trusty; Clang; OpenSSL)' @@ -42,8 +40,7 @@ jobs: imageName: 'libgit2/trusty-amd64:latest' environmentVariables: | CC=clang - CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON - LEAK_CHECK=valgrind + CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind - job: linux_amd64_trusty_clang_mbedtls displayName: 'Linux (amd64; Trusty; Clang; mbedTLS)' @@ -55,8 +52,7 @@ jobs: imageName: 'libgit2/trusty-amd64:latest' environmentVariables: | CC=clang - CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DREGEX_BACKEND=pcre -DDEPRECATE_HARD=ON - LEAK_CHECK=valgrind + CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DREGEX_BACKEND=pcre -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind - job: macos displayName: 'macOS' @@ -70,8 +66,7 @@ jobs: environmentVariables: TMPDIR: $(Agent.TempDirectory) PKG_CONFIG_PATH: /usr/local/opt/openssl/lib/pkgconfig - LEAK_CHECK: leaks - CMAKE_OPTIONS: -G Ninja -DREGEX_BACKEND=regcomp_l -DDEPRECATE_HARD=ON + CMAKE_OPTIONS: -G Ninja -DREGEX_BACKEND=regcomp_l -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=leaks SKIP_SSH_TESTS: true - job: windows_vs_amd64 diff --git a/azure-pipelines/nightly.yml b/azure-pipelines/nightly.yml index c2997160c..a79e92680 100644 --- a/azure-pipelines/nightly.yml +++ b/azure-pipelines/nightly.yml @@ -12,8 +12,7 @@ jobs: imageName: 'libgit2/trusty-amd64:latest' environmentVariables: | CC=gcc - CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DREGEX_BACKEND=builtin -DDEPRECATE_HARD=ON - LEAK_CHECK=valgrind + CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DREGEX_BACKEND=builtin -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind RUN_INVASIVE_TESTS=true - job: linux_amd64_trusty_gcc_mbedtls @@ -26,8 +25,7 @@ jobs: imageName: 'libgit2/trusty-amd64:latest' environmentVariables: | CC=gcc - CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DDEPRECATE_HARD=ON - LEAK_CHECK=valgrind + CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind RUN_INVASIVE_TESTS=true - job: linux_amd64_trusty_clang_openssl @@ -40,8 +38,7 @@ jobs: imageName: 'libgit2/trusty-amd64:latest' environmentVariables: | CC=clang - CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON - LEAK_CHECK=valgrind + CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind RUN_INVASIVE_TESTS=true - job: linux_amd64_trusty_clang_mbedtls @@ -54,8 +51,7 @@ jobs: imageName: 'libgit2/trusty-amd64:latest' environmentVariables: | CC=clang - CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DREGEX_BACKEND=pcre -DDEPRECATE_HARD=ON - LEAK_CHECK=valgrind + CMAKE_OPTIONS=-DUSE_HTTPS=mbedTLS -DUSE_SHA1=HTTPS -DREGEX_BACKEND=pcre -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind RUN_INVASIVE_TESTS=true - job: macos @@ -70,8 +66,7 @@ jobs: environmentVariables: TMPDIR: $(Agent.TempDirectory) PKG_CONFIG_PATH: /usr/local/opt/openssl/lib/pkgconfig - LEAK_CHECK: leaks - CMAKE_OPTIONS: -G Ninja -DREGEX_BACKEND=regcomp_l -DDEPRECATE_HARD=ON + CMAKE_OPTIONS: -G Ninja -DREGEX_BACKEND=regcomp_l -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=leaks RUN_INVASIVE_TESTS: true SKIP_SSH_TESTS: true @@ -139,8 +134,7 @@ jobs: imageName: 'libgit2/bionic-x86:latest' environmentVariables: | CC=gcc - CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON - LEAK_CHECK=valgrind + CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind RUN_INVASIVE_TESTS=true - job: linux_x86_bionic_clang_openssl @@ -154,8 +148,7 @@ jobs: imageName: 'libgit2/bionic-x86:latest' environmentVariables: | CC=clang - CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON - LEAK_CHECK=valgrind + CMAKE_OPTIONS=-DUSE_HTTPS=OpenSSL -DDEPRECATE_HARD=ON -DUSE_LEAK_CHECKER=valgrind RUN_INVASIVE_TESTS=true - job: linux_arm32_bionic_gcc_openssl diff --git a/ci/test.sh b/ci/test.sh index cbc3925a9..69b4273a5 100755 --- a/ci/test.sh +++ b/ci/test.sh @@ -13,9 +13,6 @@ USER=${USER:-$(whoami)} SUCCESS=1 -VALGRIND="valgrind --leak-check=full --show-reachable=yes --error-exitcode=125 --num-callers=50 --suppressions=\"$SOURCE_DIR/libgit2_clar.supp\"" -LEAKS="MallocStackLogging=1 MallocScribble=1 MallocLogFile=/dev/null CLAR_AT_EXIT=\"leaks -quiet \$PPID\"" - cleanup() { echo "Cleaning up..." @@ -32,29 +29,7 @@ cleanup() { echo "Done." } -# Ask ctest what it would run if we were to invoke it directly. This lets -# us manage the test configuration in a single place (tests/CMakeLists.txt) -# instead of running clar here as well. But it allows us to wrap our test -# harness with a leak checker like valgrind. Append the option to write -# JUnit-style XML files. run_test() { - TEST_CMD=$(ctest -N -V -R "^${1}$" | sed -n 's/^[0-9]*: Test command: //p') - - if [ -z "$TEST_CMD" ]; then - echo "Could not find tests: $1" - exit 1 - fi - - TEST_CMD="${TEST_CMD} -r${BUILD_DIR}/results_${1}.xml" - - if [ "$LEAK_CHECK" = "valgrind" ]; then - RUNNER="$VALGRIND $TEST_CMD" - elif [ "$LEAK_CHECK" = "leaks" ]; then - RUNNER="$LEAKS $TEST_CMD" - else - RUNNER="$TEST_CMD" - fi - if [[ "$GITTEST_FLAKY_RETRY" > 0 ]]; then ATTEMPTS_REMAIN=$GITTEST_FLAKY_RETRY else @@ -70,7 +45,8 @@ run_test() { fi RETURN_CODE=0 - eval $RUNNER || RETURN_CODE=$? && true + + CLAR_SUMMARY="${BUILD_DIR}/results_${1}.xml" ctest -V -R "^${1}$" || RETURN_CODE=$? && true if [ "$RETURN_CODE" -eq 0 ]; then break diff --git a/script/leaks.sh b/script/leaks.sh new file mode 100755 index 000000000..efeead516 --- /dev/null +++ b/script/leaks.sh @@ -0,0 +1,6 @@ +#!/bin/sh +export MallocStackLogging=1 +export MallocScribble=1 +export MallocLogFile=/dev/null +export CLAR_AT_EXIT="leaks -quiet \$PPID" +exec "$@" diff --git a/script/valgrind.sh b/script/valgrind.sh new file mode 100755 index 000000000..b5deed2b0 --- /dev/null +++ b/script/valgrind.sh @@ -0,0 +1,2 @@ +#!/bin/bash +exec valgrind --leak-check=full --show-reachable=yes --error-exitcode=125 --num-callers=50 --suppressions="$(dirname "${BASH_SOURCE[0]}")/valgrind.supp" "$@" diff --git a/libgit2_clar.supp b/script/valgrind.supp similarity index 100% rename from libgit2_clar.supp rename to script/valgrind.supp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e39fd6f7b..a97e8fe21 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -57,9 +57,17 @@ IF (MSVC_IDE) SET_SOURCE_FILES_PROPERTIES("precompiled.c" COMPILE_FLAGS "/Ycprecompiled.h") ENDIF () -ADD_TEST(offline "${libgit2_BINARY_DIR}/libgit2_clar" -v -xonline) -ADD_TEST(invasive "${libgit2_BINARY_DIR}/libgit2_clar" -v -score::ftruncate -sfilter::stream::bigfile -sodb::largefiles -siterator::workdir::filesystem_gunk -srepo::init -srepo::init::at_filesystem_root) -ADD_TEST(online "${libgit2_BINARY_DIR}/libgit2_clar" -v -sonline) -ADD_TEST(gitdaemon "${libgit2_BINARY_DIR}/libgit2_clar" -v -sonline::push) -ADD_TEST(ssh "${libgit2_BINARY_DIR}/libgit2_clar" -v -sonline::push -sonline::clone::ssh_cert -sonline::clone::ssh_with_paths) -ADD_TEST(proxy "${libgit2_BINARY_DIR}/libgit2_clar" -v -sonline::clone::proxy) +FUNCTION(ADD_CLAR_TEST name) + IF (NOT USE_LEAK_CHECKER STREQUAL "OFF") + ADD_TEST(${name} "${libgit2_SOURCE_DIR}/script/${USE_LEAK_CHECKER}.sh" "${libgit2_BINARY_DIR}/libgit2_clar" ${ARGN}) + ELSE() + ADD_TEST(${name} "${libgit2_BINARY_DIR}/libgit2_clar" ${ARGN}) + ENDIF() +ENDFUNCTION(ADD_CLAR_TEST) + +ADD_CLAR_TEST(offline -v -xonline) +ADD_CLAR_TEST(invasive -v -score::ftruncate -sfilter::stream::bigfile -sodb::largefiles -siterator::workdir::filesystem_gunk -srepo::init -srepo::init::at_filesystem_root) +ADD_CLAR_TEST(online -v -sonline) +ADD_CLAR_TEST(gitdaemon -v -sonline::push) +ADD_CLAR_TEST(ssh -v -sonline::push -sonline::clone::ssh_cert -sonline::clone::ssh_with_paths) +ADD_CLAR_TEST(proxy -v -sonline::clone::proxy)