From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A2A7D440F3C for ; Wed, 20 Nov 2019 17:40:41 +0300 (MSK) Date: Wed, 20 Nov 2019 17:38:33 +0300 From: Igor Munkin Message-ID: <20191120143833.GD18878@tarantool.org> References: <4f0eded688b6954a1a4231f4a4900f12b9980d8f.1573040909.git.avtikhon@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <4f0eded688b6954a1a4231f4a4900f12b9980d8f.1573040909.git.avtikhon@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v1] test: need ASAN testing in commit process List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Alexander V. Tikhonov" Cc: tarantool-patches@freelists.org, tarantool-patches@dev.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