Skip to content

Commit

Permalink
Use bundled gtest (#4900)
Browse files Browse the repository at this point in the history
* Suggest to use gtest bundled with dmlc

* Use dmlc bundled gtest in all CI scripts

* Make clang-tidy to use dmlc embedded gtest
  • Loading branch information
trams authored and hcho3 committed Oct 9, 2019
1 parent 095de3b commit 8097718
Show file tree
Hide file tree
Showing 9 changed files with 17 additions and 60 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ option(JVM_BINDINGS "Build JVM bindings" OFF)
option(R_LIB "Build shared library for R package" OFF)
## Dev
option(GOOGLE_TEST "Build google tests" OFF)
option(USE_DMLC_GTEST "Use google tests bundled with dmlc-core submodule (EXPERIMENTAL)" OFF)
option(USE_DMLC_GTEST "Use google tests bundled with dmlc-core submodule" OFF)
option(USE_NVTX "Build with cuda profiling annotations. Developers only." OFF)
set(NVTX_HEADER_DIR "" CACHE PATH "Path to the stand-alone nvtx header")
option(RABIT_MOCK "Build rabit with mock" OFF)
Expand Down
2 changes: 1 addition & 1 deletion Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def ClangTidy() {
def docker_binary = "docker"
def dockerArgs = "--build-arg CUDA_VERSION=9.2"
sh """
${dockerRun} ${container_type} ${docker_binary} ${dockerArgs} tests/ci_build/clang_tidy.sh
${dockerRun} ${container_type} ${docker_binary} ${dockerArgs} python3 tests/ci_build/tidy.py
"""
deleteDir()
}
Expand Down
8 changes: 3 additions & 5 deletions doc/contrib/coding_guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -118,21 +118,19 @@ To run this check locally, run the following command from the top level source t
.. code-block:: bash
cd /path/to/xgboost/
python3 tests/ci_build/tidy.py --gtest-path=/path/to/google-test
where ``--gtest-path`` option specifies the full path of Google Test library.
python3 tests/ci_build/tidy.py
Also, the script accepts two optional integer arguments, namely ``--cpp`` and ``--cuda``. By default they are both set to 1, meaning that both C++ and CUDA code will be checked. If the CUDA toolkit is not installed on your machine, you'll encounter an error. To exclude CUDA source from linting, use:

.. code-block:: bash
cd /path/to/xgboost/
python3 tests/ci_build/tidy.py --cuda=0 --gtest-path=/path/to/google-test
python3 tests/ci_build/tidy.py --cuda=0
Similarly, if you want to exclude C++ source from linting:

.. code-block:: bash
cd /path/to/xgboost/
python3 tests/ci_build/tidy.py --cpp=0 --gtest-path=/path/to/google-test
python3 tests/ci_build/tidy.py --cpp=0
7 changes: 3 additions & 4 deletions doc/contrib/unit_tests.rst
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,13 @@ In addition, to test CUDA code, run:
C++: Google Test
================

To build and run C++ unit tests, install `Google Test <https://github.com/google/googletest>`_ library with headers
and then enable tests while running CMake:
To build and run C++ unit tests enable tests while running CMake:

.. code-block:: bash
mkdir build
cd build
cmake -DGOOGLE_TEST=ON -DGTEST_ROOT=/path/to/google-test ..
cmake -DGOOGLE_TEST=ON -DUSE_DMLC_GTEST=ON ..
make
make test
Expand All @@ -121,7 +120,7 @@ To enable tests for CUDA code, add ``-DUSE_CUDA=ON`` and ``-DUSE_NCCL=ON`` (CUDA
mkdir build
cd build
cmake -DGOOGLE_TEST=ON -DGTEST_ROOT=/path/to/google-test -DUSE_CUDA=ON -DUSE_NCCL=ON ..
cmake -DGOOGLE_TEST=ON -DUSE_DMLC_GTEST=ON -DUSE_CUDA=ON -DUSE_NCCL=ON ..
make
make test
Expand Down
12 changes: 1 addition & 11 deletions tests/ci_build/build_via_cmake.sh
Original file line number Diff line number Diff line change
@@ -1,20 +1,10 @@
#!/usr/bin/env bash
set -e

# Build gtest via cmake
rm -rf gtest
wget -nc https://github.com/google/googletest/archive/release-1.7.0.zip
unzip -n release-1.7.0.zip
mv googletest-release-1.7.0 gtest && cd gtest
cmake . && make
mkdir lib && mv libgtest.a lib
cd ..
rm -rf release-1.7.0.zip*

rm -rf build
mkdir build
cd build
cmake .. "$@" -DGOOGLE_TEST=ON -DGTEST_ROOT=$PWD/../gtest -DCMAKE_VERBOSE_MAKEFILE=ON
cmake .. "$@" -DGOOGLE_TEST=ON -DUSE_DMLC_GTEST=ON -DCMAKE_VERBOSE_MAKEFILE=ON
make clean
make -j$(nproc)
cd ..
18 changes: 0 additions & 18 deletions tests/ci_build/clang_tidy.sh

This file was deleted.

15 changes: 6 additions & 9 deletions tests/ci_build/tidy.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,12 @@ class ClangTidy(object):
'''
clang tidy wrapper.
Args:
gtest_path: Full path of Google Test library.
cpp_lint: Run linter on C++ source code.
cuda_lint: Run linter on CUDA source code.
'''
def __init__(self, gtest_path, cpp_lint, cuda_lint):
self.gtest_path = gtest_path
def __init__(self, cpp_lint, cuda_lint):
self.cpp_lint = cpp_lint
self.cuda_lint = cuda_lint
print('Using Google Test from {}'.format(self.gtest_path))
print('Run linter on CUDA: ', self.cuda_lint)
print('Run linter on C++:', self.cpp_lint)
if not self.cpp_lint and not self.cuda_lint:
Expand All @@ -61,8 +58,7 @@ def _generate_cdb(self):
os.mkdir(self.cdb_path)
os.chdir(self.cdb_path)
cmake_args = ['cmake', '..', '-DCMAKE_EXPORT_COMPILE_COMMANDS=ON',
'-DGOOGLE_TEST=ON', '-DGTEST_ROOT={}'.format(
self.gtest_path)]
'-DGOOGLE_TEST=ON', '-DUSE_DMLC_GTEST=ON']
if self.cuda_lint:
cmake_args.extend(['-DUSE_CUDA=ON', '-DUSE_NCCL=ON'])
subprocess.run(cmake_args)
Expand Down Expand Up @@ -108,6 +104,8 @@ def convert_nvcc_command_to_clang(self, command):
'--cuda-gpu-arch=sm_' + capability)
elif components[i].find('--std=c++11') != -1:
converted_components.append('-std=c++11')
elif components[i].startswith('-isystem='):
converted_components.extend(components[i].split('='))
else:
converted_components.append(components[i])

Expand Down Expand Up @@ -156,6 +154,7 @@ def should_lint(path):
return False
isxgb = path.find('rabit') == -1
isxgb = isxgb and path.find('dmlc-core') == -1
isxgb = isxgb and (not path.startswith(self.cdb_path))
if isxgb:
return True

Expand Down Expand Up @@ -235,13 +234,11 @@ def test_tidy():
parser = argparse.ArgumentParser(description='Run clang-tidy.')
parser.add_argument('--cpp', type=int, default=1)
parser.add_argument('--cuda', type=int, default=1)
parser.add_argument('--gtest-path', required=True,
help='Full path of Google Test library directory')
args = parser.parse_args()

test_tidy()

with ClangTidy(args.gtest_path, args.cpp, args.cuda) as linter:
with ClangTidy(args.cpp, args.cuda) as linter:
passed = linter.run()
if not passed:
sys.exit(1)
2 changes: 1 addition & 1 deletion tests/cpp/common/test_enum_class_param.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ TEST(EnumClassParam, Basic) {
{"foo", "frog"}, {"bar", "10"}
};
// try initializing
param.Init(kwargs);
param.Init(kwargs); // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)
ASSERT_EQ(param.foo, Foo::kFrog);
ASSERT_EQ(param.bar, 10);

Expand Down
11 changes: 1 addition & 10 deletions tests/travis/run_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,11 @@ fi

if [ ${TASK} == "cmake_test" ]; then
set -e
# Build gtest via cmake
wget -nc https://github.com/google/googletest/archive/release-1.7.0.zip
unzip -n release-1.7.0.zip
mv googletest-release-1.7.0 gtest && cd gtest
CC=gcc-7 CXX=g++-7 cmake . && make
mkdir lib && mv libgtest.a lib
cd ..
rm -rf release-1.7.0.zip

# Build/test
rm -rf build
mkdir build && cd build
PLUGINS="-DPLUGIN_LZ4=ON -DPLUGIN_DENSE_PARSER=ON"
CC=gcc-7 CXX=g++-7 cmake .. -DGOOGLE_TEST=ON -DGTEST_ROOT=$PWD/../gtest/ ${PLUGINS}
CC=gcc-7 CXX=g++-7 cmake .. -DGOOGLE_TEST=ON -DUSE_DMLC_GTEST=ON ${PLUGINS}
make
./testxgboost
cd ..
Expand Down

0 comments on commit 8097718

Please sign in to comment.