Repository: parquet-cpp Updated Branches: refs/heads/master 64ed94f5f -> 5c4f64557
PARQUET-564: Add cmake option to run valgrind on each unit test executable This also enables the current test suite to pass cleanly under `ctest` with this option enabled. Author: Wes McKinney <[email protected]> Closes #79 from wesm/PARQUET-564 and squashes the following commits: a67941e [Wes McKinney] Do not squelch other COMPILE_FLAGS on tests. Remove non-portable ccache code a80a6ca [Wes McKinney] Skip ResizeOOM test when running with valgrind. Fix test case valgrind error ddbf80f [Wes McKinney] Add cmake option to run tests with memcheck Project: http://git-wip-us.apache.org/repos/asf/parquet-cpp/repo Commit: http://git-wip-us.apache.org/repos/asf/parquet-cpp/commit/5c4f6455 Tree: http://git-wip-us.apache.org/repos/asf/parquet-cpp/tree/5c4f6455 Diff: http://git-wip-us.apache.org/repos/asf/parquet-cpp/diff/5c4f6455 Branch: refs/heads/master Commit: 5c4f6455760866025f04423c3fc51a9134eef262 Parents: 64ed94f Author: Wes McKinney <[email protected]> Authored: Mon Mar 14 23:32:30 2016 -0700 Committer: Wes McKinney <[email protected]> Committed: Mon Mar 14 23:32:30 2016 -0700 ---------------------------------------------------------------------- .travis.yml | 4 ++-- CMakeLists.txt | 22 +++++++++++++--------- src/parquet/schema/schema-types-test.cc | 2 +- src/parquet/util/buffer-test.cc | 15 ++++++++++++++- src/parquet/util/buffer.cc | 2 +- 5 files changed, 31 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/5c4f6455/.travis.yml ---------------------------------------------------------------------- diff --git a/.travis.yml b/.travis.yml index 24d2a20..8d49b50 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,12 +26,12 @@ matrix: os: linux before_script: - source $TRAVIS_BUILD_DIR/ci/before_script_travis.sh - - cmake -DCMAKE_CXX_FLAGS="-Werror" -DPARQUET_GENERATE_COVERAGE=1 $TRAVIS_BUILD_DIR + - cmake -DCMAKE_CXX_FLAGS="-Werror" -DPARQUET_TEST_MEMCHECK=ON -DPARQUET_GENERATE_COVERAGE=1 $TRAVIS_BUILD_DIR - export PARQUET_TEST_DATA=$TRAVIS_BUILD_DIR/data script: - make lint - make -j4 || exit 1 - - valgrind --tool=memcheck --leak-check=yes --error-exitcode=1 ctest + - ctest || exit 1 - sudo pip install cpp_coveralls - export PARQUET_ROOT=$TRAVIS_BUILD_DIR - $TRAVIS_BUILD_DIR/ci/upload_coverage.sh http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/5c4f6455/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/CMakeLists.txt b/CMakeLists.txt index a7cb439..661d813 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,13 +25,6 @@ if (NOT "$ENV{PARQUET_GCC_ROOT}" STREQUAL "") set(CMAKE_CXX_COMPILER ${GCC_ROOT}/bin/g++) endif() -if ("$ENV{PARQUET_USE_CCACHE}") - set(CMAKE_C_COMPILER_ARG1 ${CMAKE_C_COMPILER}) - set(CMAKE_CXX_COMPILER_ARG1 ${CMAKE_CXX_COMPILER}) - set(CMAKE_C_COMPILER ccache) - set(CMAKE_CXX_COMPILER ccache) -endif() - # generate CTest input files enable_testing() @@ -70,6 +63,9 @@ if ("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}") option(PARQUET_BUILD_TESTS "Build the libparquet test suite" ON) + option(PARQUET_TEST_MEMCHECK + "Run the test suite using valgrind --tool=memcheck" + OFF) option(PARQUET_BUILD_EXECUTABLES "Build the libparquet executable CLI tools" ON) @@ -131,8 +127,16 @@ function(ADD_PARQUET_TEST REL_TEST_NAME) set(TEST_PATH ${CMAKE_CURRENT_SOURCE_DIR}/${REL_TEST_NAME}) endif() - add_test(${TEST_NAME} - ${BUILD_SUPPORT_DIR}/run-test.sh ${TEST_PATH}) + if (PARQUET_TEST_MEMCHECK) + SET_PROPERTY(TARGET ${TEST_NAME} + APPEND_STRING PROPERTY + COMPILE_FLAGS " -DPARQUET_VALGRIND") + add_test(${TEST_NAME} + valgrind --tool=memcheck --leak-check=full --error-exitcode=1 ${TEST_PATH}) + else() + add_test(${TEST_NAME} + ${BUILD_SUPPORT_DIR}/run-test.sh ${TEST_PATH}) + endif() if(ARGN) set_tests_properties(${TEST_NAME} PROPERTIES ${ARGN}) endif() http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/5c4f6455/src/parquet/schema/schema-types-test.cc ---------------------------------------------------------------------- diff --git a/src/parquet/schema/schema-types-test.cc b/src/parquet/schema/schema-types-test.cc index f512f0f..fa4718a 100644 --- a/src/parquet/schema/schema-types-test.cc +++ b/src/parquet/schema/schema-types-test.cc @@ -39,7 +39,7 @@ namespace schema { class TestPrimitiveNode : public ::testing::Test { public: - void setUp() { + void SetUp() { name_ = "name"; id_ = 5; } http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/5c4f6455/src/parquet/util/buffer-test.cc ---------------------------------------------------------------------- diff --git a/src/parquet/util/buffer-test.cc b/src/parquet/util/buffer-test.cc index 7772e49..f494326 100644 --- a/src/parquet/util/buffer-test.cc +++ b/src/parquet/util/buffer-test.cc @@ -18,6 +18,7 @@ #include <gtest/gtest.h> #include <cstdlib> #include <cstdint> +#include <exception> #include <limits> #include <memory> #include <string> @@ -48,10 +49,22 @@ TEST_F(TestBuffer, Resize) { TEST_F(TestBuffer, ResizeOOM) { // realloc fails, even though there may be no explicit limit + + // Tests that deliberately throw Exceptions foul up valgrind and report + // red herring memory leaks +#ifndef PARQUET_VALGRIND OwnedMutableBuffer buf; ASSERT_NO_THROW(buf.Resize(100)); int64_t to_alloc = std::numeric_limits<int64_t>::max(); - ASSERT_THROW(buf.Resize(to_alloc), ParquetException); + try { + buf.Resize(to_alloc); + FAIL() << "Exception not thrown"; + } catch (const ParquetException& e) { + // pass + } catch(const std::exception& e) { + FAIL() << "Different exception thrown"; + } +#endif } } // namespace parquet_cpp http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/5c4f6455/src/parquet/util/buffer.cc ---------------------------------------------------------------------- diff --git a/src/parquet/util/buffer.cc b/src/parquet/util/buffer.cc index c069419..6647c15 100644 --- a/src/parquet/util/buffer.cc +++ b/src/parquet/util/buffer.cc @@ -38,12 +38,12 @@ OwnedMutableBuffer::OwnedMutableBuffer() : ResizableBuffer(nullptr, 0) {} void OwnedMutableBuffer::Resize(int64_t new_size) { - size_ = new_size; try { buffer_owner_.resize(new_size); } catch (const std::bad_alloc& e) { throw ParquetException("OOM: resize failed"); } + size_ = new_size; data_ = buffer_owner_.data(); mutable_data_ = buffer_owner_.data(); }
