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

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

[Alexander V. Tikhonov]

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.

Closes #4359

(cherry picked from commit 55f7586ac36760f31255fee7b70606f466f30e04)
---

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
+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 #
 #######
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

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

[Igor Munkin]

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

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

[Alexander Tikhonov]

[-- Attachment #1: Type: text/plain, Size: 5518 bytes --]


Igor,

Thank you for your review, I've made all changes that you suggested. 
>Среда, 20 ноября 2019, 17:40 +03:00 от 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. 
Done.
>
>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.
Wrote more information on LTO.
>
>> 
>> 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.
Corrected the message.
>
>> ---
>> 
>> 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.
Right, that was mistake - removed these lines.
>
>> +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.
That is the traditional way that is in use for now, may be decide to change
in the future, but it will be the standalone patch for all the skipcond files.
>
>> 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
>


-- 
Alexander Tikhonov

[-- Attachment #2: Type: text/html, Size: 7954 bytes --]