Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use bundled gtest #4900

Merged
merged 3 commits into from
Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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