cover change with gtest unittests, clean up cmake script and code includes

.travis.yml

@@ -38,25 +38,22 @@ addons:
       - openssh-server
       - python3
       - python3-setuptools
+      - python3-pip
+      - tree
   homebrew:
     packages:
       - gcc49
       - openssl
       - libgit2
       - python3
     update: true
-      
+
 before_install:
   - git clone https://github.com/dmlc/dmlc-core
   - export TRAVIS=dmlc-core/scripts/travis/
   - source ${TRAVIS}/travis_setup_env.sh
   - ${TRAVIS}/travis_osx_install.sh
-
-install:
-  - if [[ ${TRAVIS_OS_NAME} == "linux" ]]; then sudo apt-get install python3-pip; fi
-  - if [[ ${TRAVIS_OS_NAME} == "osx" ]]; then brew install python3; fi
-  - pip3 install cpplint pylint urllib3 numpy
-  - pip3 install websocket-client kubernetes
+  - source ./scripts/travis_setup.sh
 
 script: scripts/travis_script.sh
 
@@ -78,4 +75,3 @@ notifications:
   email:
     on_success: change
     on_failure: always
-

CMakeLists.txt

@@ -1,85 +1,106 @@
 cmake_minimum_required(VERSION 3.3)
 
-project(rabit VERSION 0.3.0)
-
-include(CheckCXXCompilerFlag)
-CHECK_CXX_COMPILER_FLAG("-std=c++11" COMPILER_SUPPORTS_CXX11)
-if(COMPILER_SUPPORTS_CXX11)
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
-else()
-  message(STATUS "The compiler ${CMAKE_CXX_COMPILER} has no C++11 support. Please use a different C++ compiler.")
-endif()
+project(rabit VERSION 0.3.0 LANGUAGES CXX)
 
 option(RABIT_BUILD_TESTS "Build rabit tests" OFF)
 option(RABIT_BUILD_MPI "Build MPI" OFF)
 option(RABIT_BUILD_DMLC "Include DMLC_CORE in build" OFF)
+option(RABIT_WITH_R_LIB "Fit the strict environment of R" OFF)
+
+option(DMLC_ROOT "Specify root of external dmlc core.")
+# by default point to xgboost/dmlc-core
+set(DMLC_ROOT ${CMAKE_CURRENT_LIST_DIR}/../dmlc-core)
 
 # moved from xgboost build
 if(R_LIB OR MINGW OR WIN32)
-  add_library(rabit_base src/allreduce_base.cc src/engine_base.cc src/c_api.cc)
+  add_library(rabit src/engine_empty.cc src/c_api.cc)
   set(rabit_libs rabit)
-  set_target_properties(rabit PROPERTIES CXX_STANDARD 11 CXX_STANDARD_REQUIRED ON)
-ELSE()
+  set_target_properties(rabit
+          PROPERTIES CXX_STANDARD 11
+          CXX_STANDARD_REQUIRED ON
+          POSITION_INDEPENDENT_CODE ON)
+else()
   add_library(rabit src/allreduce_base.cc src/allreduce_robust.cc src/engine.cc src/c_api.cc)
   add_library(rabit_base src/allreduce_base.cc src/engine_base.cc src/c_api.cc)
   add_library(rabit_empty src/engine_empty.cc src/c_api.cc)
   add_library(rabit_mock_static src/allreduce_base.cc src/allreduce_robust.cc src/engine_mock.cc src/c_api.cc)
   add_library(rabit_mock SHARED src/allreduce_base.cc src/allreduce_robust.cc src/engine_mock.cc src/c_api.cc)
 
   set(rabit_libs rabit rabit_base rabit_empty rabit_mock rabit_mock_static)
-  set_target_properties(rabit rabit_base rabit_empty rabit_mock rabit_mock_static PROPERTIES CXX_STANDARD 11 CXX_STANDARD_REQUIRED ON)
+  set_target_properties(rabit rabit_base rabit_empty rabit_mock rabit_mock_static
+    PROPERTIES CXX_STANDARD 11
+    CXX_STANDARD_REQUIRED ON
+    POSITION_INDEPENDENT_CODE ON)
 ENDIF(R_LIB OR MINGW OR WIN32)
 
 if(RABIT_BUILD_MPI)
   find_package(MPI REQUIRED)
+  if (NOT MPI_CXX_FOUND)
+    message(FATAL_ERROR "CXX Interface for MPI is required for building MPI backend.")
+  endif (NOT MPI_CXX_FOUND)
   add_library(rabit_mpi src/engine_mpi.cc ${MPI_INCLUDE_PATH})
   target_link_libraries(rabit_mpi ${MPI_CXX_LIBRARIES})
   list(APPEND rabit_libs rabit_mpi)
 endif()
 
+# place binaries and libraries according to GNU standards
+include(GNUInstallDirs)
+set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR})
+set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR})
+set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${CMAKE_INSTALL_BINDIR})
 
-if(RABIT_BUILD_DMLC)
-  foreach(lib ${rabit_libs})
-    #include "./internal/utils.h"
-    target_include_directories(${lib} PUBLIC
-      "$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>"
-      "$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/dmlc-core/include>"
-      )
-  endforeach()
-else()
+# we use this to get code coverage
+if ((CMAKE_CONFIGURATION_TYPES STREQUAL "Debug") AND (CMAKE_CXX_COMPILER_ID MATCHES GNU))
   foreach(lib ${rabit_libs})
-    #include "./internal/utils.h"
-    target_include_directories(${lib} PUBLIC
-      "$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>"
-      "$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/../dmlc_core/include>"
-      )
-  endforeach()
+    target_compile_options(${lib}
+      -fprofile-arcs
+      -ftest-coverage)
+    endforeach()
+endif((CMAKE_CONFIGURATION_TYPES STREQUAL "Debug") AND (CMAKE_CXX_COMPILER_ID MATCHES GNU))
+
+if(RABIT_BUILD_DMLC)
+  set(DMLC_ROOT ${CMAKE_CURRENT_LIST_DIR}/dmlc-core)
 endif()
 
-if(RABIT_BUILD_TESTS)
+if(DMLC_ROOT)
+  message("DMLC_ROOT point to " ${DMLC_ROOT})
+endif(DMLC_ROOT)
+
+foreach(lib ${rabit_libs})
+  target_include_directories(${lib} PUBLIC
+          "$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>"
+          "$<BUILD_INTERFACE:${DMLC_ROOT}/include>")
+endforeach()
+
+if (RABIT_BUILD_TESTS)
+  enable_testing()
+  add_subdirectory(${rabit_SOURCE_DIR}/test/cpp)
+
+  # rabit mock based integration tests
   list(REMOVE_ITEM rabit_libs "rabit_mock_static") # remove here to avoid installing it
   set(tests lazy_recover local_recover model_recover)
 
   foreach(test ${tests})
     add_executable(${test} test/${test}.cc)
     target_link_libraries(${test} rabit_mock_static)
     set_target_properties(${test}  PROPERTIES CXX_STANDARD 11 CXX_STANDARD_REQUIRED ON)
-    install(TARGETS ${test} DESTINATION test)
+    install(TARGETS ${test} DESTINATION test) # Why are we installing these??
   endforeach()
+
   if(RABIT_BUILD_MPI)
     add_executable(speed_test_mpi test/speed_test.cc)
     target_link_libraries(speed_test_mpi rabit_mpi)
     install(TARGETS speed_test_mpi DESTINATION test)
   endif()
-endif()
+endif (RABIT_BUILD_TESTS)
 
 # Installation (https://github.com/forexample/package-example) {
 
 # Layout. This works for all platforms:
 #   * <prefix>/lib/cmake/<PROJECT-NAME>
 #   * <prefix>/lib/
 #   * <prefix>/include/
-set(CMAKE_INSTALL_PREFIX "../")
+set(CMAKE_INSTALL_PREFIX "${rabit_SOURCE_DIR}")
 set(config_install_dir "lib/cmake/${PROJECT_NAME}")
 set(include_install_dir "include")
 

cmake/googletest-download.cmake

@@ -0,0 +1,20 @@
+# code copied from https://crascit.com/2015/07/25/cmake-gtest/
+cmake_minimum_required(VERSION 3.5 FATAL_ERROR)
+
+project(googletest-download NONE)
+
+include(ExternalProject)
+
+ExternalProject_Add(
+        googletest
+        SOURCE_DIR "@GOOGLETEST_DOWNLOAD_ROOT@/googletest-src"
+        BINARY_DIR "@GOOGLETEST_DOWNLOAD_ROOT@/googletest-build"
+        GIT_REPOSITORY
+        https://github.com/google/googletest.git
+        GIT_TAG
+        release-1.8.0
+        CONFIGURE_COMMAND ""
+        BUILD_COMMAND ""
+        INSTALL_COMMAND ""
+        TEST_COMMAND ""
+)

cmake/googletest.cmake

@@ -0,0 +1,32 @@
+# the following code to fetch googletest
+# is inspired by and adapted after https://crascit.com/2015/07/25/cmake-gtest/
+# download and unpack googletest at configure time
+
+macro(fetch_googletest _download_module_path _download_root)
+    set(GOOGLETEST_DOWNLOAD_ROOT ${_download_root})
+    configure_file(
+            ${_download_module_path}/googletest-download.cmake
+            ${_download_root}/CMakeLists.txt
+            @ONLY
+    )
+    unset(GOOGLETEST_DOWNLOAD_ROOT)
+
+    execute_process(
+            COMMAND
+            "${CMAKE_COMMAND}" -G "${CMAKE_GENERATOR}" .
+            WORKING_DIRECTORY
+            ${_download_root}
+    )
+    execute_process(
+            COMMAND
+            "${CMAKE_COMMAND}" --build .
+            WORKING_DIRECTORY
+            ${_download_root}
+    )
+
+    # adds the targers: gtest, gtest_main, gmock, gmock_main
+    add_subdirectory(
+            ${_download_root}/googletest-src
+            ${_download_root}/googletest-build
+    )
+endmacro()

include/rabit/internal/engine.h

@@ -7,7 +7,7 @@
 #ifndef RABIT_INTERNAL_ENGINE_H_
 #define RABIT_INTERNAL_ENGINE_H_
 #include <string>
-#include "../serializable.h"
+#include "rabit/serializable.h"
 
 #if (defined(__GNUC__) && !defined(__clang__))
 #define _FILE  __builtin_FILE()

include/rabit/internal/io.h

@@ -11,8 +11,8 @@
 #include <cstring>
 #include <string>
 #include <algorithm>
-#include "./utils.h"
-#include "../serializable.h"
+#include "rabit/internal/utils.h"
+#include "rabit/serializable.h"
 
 namespace rabit {
 namespace utils {

include/rabit/internal/rabit-inl.h

@@ -10,9 +10,9 @@
 // use engine for implementation
 #include <vector>
 #include <string>
-#include "./io.h"
-#include "./utils.h"
-#include "../rabit.h"
+#include "rabit/internal/io.h"
+#include "rabit/internal/utils.h"
+#include "rabit/rabit.h"
 
 namespace rabit {
 namespace engine {

src/socket.h → include/rabit/internal/socket.h

@@ -4,8 +4,8 @@
  * \brief this file aims to provide a wrapper of sockets
  * \author Tianqi Chen
  */
-#ifndef RABIT_SOCKET_H_
-#define RABIT_SOCKET_H_
+#ifndef RABIT_INTERNAL_SOCKET_H_
+#define RABIT_INTERNAL_SOCKET_H_
 #if defined(_WIN32)
 #include <winsock2.h>
 #include <ws2tcpip.h>
@@ -26,7 +26,7 @@
 #include <cstring>
 #include <vector>
 #include <unordered_map>
-#include "../include/rabit/internal/utils.h"
+#include "utils.h"
 
 #if defined(_WIN32) || defined(__MINGW32__)
 typedef int ssize_t;
@@ -533,4 +533,4 @@ struct PollHelper {
 };
 }  // namespace utils
 }  // namespace rabit
-#endif  // RABIT_SOCKET_H_
+#endif  // RABIT_INTERNAL_SOCKET_H_

src/thread_local.h → include/rabit/internal/thread_local.h

@@ -3,8 +3,8 @@
  * \file thread_local.h
  * \brief Common utility for thread local storage.
  */
-#ifndef RABIT_THREAD_LOCAL_H_
-#define RABIT_THREAD_LOCAL_H_
+#ifndef RABIT_INTERNAL_THREAD_LOCAL_H_
+#define RABIT_INTERNAL_THREAD_LOCAL_H_
 
 #include "../include/dmlc/base.h"
 
@@ -84,4 +84,4 @@ class ThreadLocalStore {
   std::vector<T*> data_;
 };
 }  // namespace rabit
-#endif  // RABIT_THREAD_LOCAL_H_
+#endif  // RABIT_INTERNAL_THREAD_LOCAL_H_

include/rabit/serializable.h

@@ -8,10 +8,10 @@
 #define RABIT_SERIALIZABLE_H_
 #include <vector>
 #include <string>
-#include "./internal/utils.h"
+#include "rabit/internal/utils.h"
 
 #ifndef DMLC_IO_H_
-#include "../../dmlc-core/include/dmlc/io.h"
+#include "dmlc/io.h"
 #endif  // DMLC_IO_H_
 
 namespace rabit {

scripts/travis_script.sh

@@ -24,9 +24,12 @@ fi
 if [ ${TASK} == "cmake-test" ]; then
     mkdir build
     cd build
-    cmake -DRABIT_BUILD_TESTS=ON -DRABIT_BUILD_DMLC=ON ..
+    cmake -DRABIT_BUILD_TESTS=ON -DRABIT_BUILD_DMLC=ON -DGTEST_ROOT=${HOME}/.local ..
+    #unit tests
+    make
+    make test
     make install || exit -1
     cd ../test
     ../scripts/travis_runtest.sh || exit -1
     rm -rf ../build
-fi
+fi

scripts/travis_setup.sh

@@ -0,0 +1,35 @@
+#!/bin/bash
+
+echo "Testing on: ${TRAVIS_OS_NAME}, Home directory: ${HOME}"
+
+pip3 install cpplint pylint urllib3 numpy cpplint
+pip3 install websocket-client kubernetes
+
+
+# Install googletest under home directory
+GTEST_VERSION=1.8.1
+GTEST_RELEASE=release-${GTEST_VERSION}.tar.gz
+GTEST_TAR_BALL=googletest_${GTEST_RELEASE}
+
+wget https://github.com/google/googletest/archive/${GTEST_RELEASE} -O ${GTEST_TAR_BALL}
+echo "152b849610d91a9dfa1401293f43230c2e0c33f8 ${GTEST_TAR_BALL}" | sha1sum -c
+tar -xf ${GTEST_TAR_BALL}
+pushd .
+
+cd googletest-release-${GTEST_VERSION}
+mkdir build
+cd build
+echo "Installing to ${HOME}/.local"
+cmake .. -DBUILD_SHARED_LIBS=ON -DCMAKE_INSTALL_PREFIX=${HOME}/.local
+make -j$(nproc)
+make install
+
+popd
+
+if [ ${TRAVIS_OS_NAME} == "linux" ]; then
+    sudo apt-get install python3-pip tree
+fi
+
+if [ ${TRAVIS_OS_NAME} == "osx" ]; then
+    brew install python3
+fi

src/CMakeLists.txt

@@ -0,0 +1,17 @@
+option(DMLC_ROOT "Specify root of external dmlc core.")
+
+add_library(allreduce_base "")
+
+target_sources(
+        allreduce_base
+        PRIVATE
+        allreduce_base.cc
+        PUBLIC
+        ${CMAKE_CURRENT_LIST_DIR}/allreduce_base.h
+)
+
+target_include_directories(
+        allreduce_base
+        PUBLIC
+		${DMLC_ROOT}/include
+        ${CMAKE_CURRENT_LIST_DIR}/../../include)