You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@parquet.apache.org by we...@apache.org on 2016/03/15 07:33:13 UTC

parquet-cpp git commit: PARQUET-564: Add cmake option to run valgrind on each unit test executable

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 <we...@apache.org>

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 <we...@apache.org>
Authored: Mon Mar 14 23:32:30 2016 -0700
Committer: Wes McKinney <we...@apache.org>
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();
 }