Re: [Tarantool-patches] [PATCH v1] test: need ASAN testing in commit process

[Igor Munkin <imun@tarantool.org>]

Sasha,

The patch doesn't seem to be cherry-picked but partially based on
55f7586. Please adjust the commit message and changes considering the
comments I left below.

On 06.11.19, Alexander V. Tikhonov wrote:
> Added ASAN tesing in commit process, used clang-8 for
> ASAN build under debian-buster image. Added for testing
> only the passing test suites, the rest of the tests
> not used and will be enabled durring issue #4360.

Please drop few lines about the fact you've reduced a number of targets
being tested via Travis, therefore this part is stripped from the
original patch.

> 
> Closes #4359
> 
> (cherry picked from commit 55f7586ac36760f31255fee7b70606f466f30e04)

As I mentioned above, adjust this line, because it's not a cherry-pick
but adapting to 1.10 branch.

> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/asan_1.10
> 
>  .gitlab-ci.yml             |  5 +++++
>  .travis.mk                 | 24 ++++++++++++++++++++++++
>  test/app-tap/json.skipcond |  7 +++++++
>  test/unit/guard.skipcond   |  7 +++++++
>  test/unit/msgpack.skipcond |  7 +++++++
>  5 files changed, 50 insertions(+)
>  create mode 100644 test/app-tap/json.skipcond
>  create mode 100644 test/unit/guard.skipcond
>  create mode 100644 test/unit/msgpack.skipcond
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 04bb4b1cc..5d3702ae5 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -85,6 +85,11 @@ release_lto_clang8:
>    script:
>      - ${GITLAB_MAKE} test_debian_no_deps
>  
> +release_asan_clang8:
> +  <<: *docker_test_clang8_definition
> +  script:
> +    - ${GITLAB_MAKE} test_asan_debian_no_deps
> +
>  osx_13_release:
>    <<: *release_only_definition
>    <<: *vbox_definition
> diff --git a/.travis.mk b/.travis.mk
> index bfb3016da..8454eb4a5 100644
> --- a/.travis.mk
> +++ b/.travis.mk
> @@ -16,6 +16,9 @@ test: test_$(TRAVIS_OS_NAME)
>  # Redirect some targets via docker
>  test_linux: docker_test_debian
>  coverage: docker_coverage_debian
> +lto: docker_test_debian
> +lto_clang8: docker_test_debian_clang8

All LTO related stuff has been stripped from the original patch, thus
these definitions seems to be excess.

> +asan: docker_test_asan_debian
>  
>  docker_%:
>  	mkdir -p ~/.cache/ccache
> @@ -68,6 +71,8 @@ deps_buster_clang_8: deps_debian
>  	apt-get update
>  	apt-get install -y clang-8 llvm-8-dev
>  
> +# Release
> +
>  build_debian:
>  	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
>  	make -j
> @@ -77,6 +82,10 @@ test_debian_no_deps: build_debian
>  
>  test_debian: deps_debian test_debian_no_deps
>  
> +test_debian_clang8: deps_debian deps_buster_clang_8 test_debian_no_deps
> +
> +# Debug with coverage
> +
>  build_coverage_debian:
>  	cmake . -DCMAKE_BUILD_TYPE=Debug -DENABLE_GCOV=ON
>  	make -j
> @@ -97,6 +106,21 @@ test_coverage_debian_no_deps: build_coverage_debian
>  
>  coverage_debian: deps_debian test_coverage_debian_no_deps
>  
> +# ASAN
> +
> +build_asan_debian:
> +	CC=clang-8 CXX=clang++-8 cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
> +	CC=clang-8 CXX=clang++-8 cmake . -DENABLE_ASAN=ON ${CMAKE_EXTRA_PARAMS}
> +	make -j
> +
> +test_asan_debian_no_deps: build_asan_debian
> +	# temporary excluded engine/ and replication/ suites with some tests from other suites by issue #4360
> +	cd test && ASAN=ON ASAN_OPTIONS=detect_leaks=0 ./test-run.py --force $(TEST_RUN_EXTRA_PARAMS) \
> +		app/ app-tap/ box/ box-py/ box-tap/ engine_long/ long_run-py/ luajit-tap/ \
> +		replication-py/ small/ sql/ sql-tap/ swim/ unit/ vinyl/ wal_off/ xlog/ xlog-py/
> +
> +test_asan_debian: deps_debian deps_buster_clang_8 test_asan_debian_no_deps
> +
>  #######
>  # OSX #
>  #######

Side note: I guess it's a good practice for future to write a rationale
for excluded tests right to skipcond file.

> diff --git a/test/app-tap/json.skipcond b/test/app-tap/json.skipcond
> new file mode 100644
> index 000000000..e46fd1088
> --- /dev/null
> +++ b/test/app-tap/json.skipcond
> @@ -0,0 +1,7 @@
> +import os
> +
> +# Disabled at ASAN build due to issue #4360.
> +if os.getenv("ASAN") == 'ON':
> +    self.skip = 1
> +
> +# vim: set ft=python:
> diff --git a/test/unit/guard.skipcond b/test/unit/guard.skipcond
> new file mode 100644
> index 000000000..e46fd1088
> --- /dev/null
> +++ b/test/unit/guard.skipcond
> @@ -0,0 +1,7 @@
> +import os
> +
> +# Disabled at ASAN build due to issue #4360.
> +if os.getenv("ASAN") == 'ON':
> +    self.skip = 1
> +
> +# vim: set ft=python:
> diff --git a/test/unit/msgpack.skipcond b/test/unit/msgpack.skipcond
> new file mode 100644
> index 000000000..e46fd1088
> --- /dev/null
> +++ b/test/unit/msgpack.skipcond
> @@ -0,0 +1,7 @@
> +import os
> +
> +# Disabled at ASAN build due to issue #4360.
> +if os.getenv("ASAN") == 'ON':
> +    self.skip = 1
> +
> +# vim: set ft=python:

> -- 
> 2.17.1
> 

-- 
Best regards,
IM