Re: [Tarantool-patches] [PATCH v4 2/10] gitlab-ci: enable static analysis with luacheck

[Sergey Bronnikov <sergeyb@tarantool.org>]

Hi,

On 23:04 Tue 21 Apr , Alexander Tikhonov wrote:
>
> Sergey, thank you for the patch, I have some questions on it.

We already started discussion on this topic in a previous patch review -
https://lists.tarantool.org/pipermail/tarantool-patches/2020-April/015672.html
Let's continue discussion there.

> Do we really want to have separate stage before the man stages with
> testing ? As I see you want to have the ability to avoid of testing if
> the initial luacheck test fails, but does it mean that issues found by
> the luacheck will always need changes in the code that always will
> change the rest of the jobs, I mean is it strong dependency between this
> check and the others that we can’t do the rest of the testing w/o
> luacheck ? We had the opinion that our testing should produce fails as
> fast as possible that is why we decided to remove different stages, and
> we can see the early fails as fast as possible, because currently all
> the testing start together from the very start, but in your fix will
> have more that 2+ minutes delay before it, while we had strong
> suggestion from Kirill Y. to make the testing fast as much as possible
> and try to make it parallel up to 2 minutes. Right, for now we have
> overall time of testing up to 20 minutes, but in general fastest jobs
> finishes in 1+ minutes and we can see the issues much faster than
> waiting the whole testing process.
>
>
> >Вторник, 21 апреля 2020, 17:02 +03:00 от sergeyb@tarantool.org:
> > 
> >From: Sergey Bronnikov < sergeyb@tarantool.org >
> >
> >Closes #4681
> >---
> > .gitlab-ci.yml | 9 +++++++++
> > .travis.mk | 15 ++++++++++++++-
> > 2 files changed, 23 insertions(+), 1 deletion(-)
> >
> >diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> >index cd710027f..adbd892c8 100644
> >--- a/.gitlab-ci.yml
> >+++ b/.gitlab-ci.yml
> >@@ -1,4 +1,5 @@
> > stages:
> >+ - static_analysis
> >   - test
> >   - perf
> >   - cleanup
> >@@ -96,6 +97,14 @@ variables:
> >   script:
> >     - ${GITLAB_MAKE} perf_run
> > 
> >+# Static Analysis
> >+
> >+luacheck:
> >+ <<: *docker_test_definition
> >+ stage: static_analysis
> >+ script:
> >+ - ${GITLAB_MAKE} test_debian_luacheck
> >+
> > # Tests
> > 
> > release:
> >diff --git a/.travis.mk b/.travis.mk
> >index f709a18b6..d917a1694 100644
> >--- a/.travis.mk
> >+++ b/.travis.mk
> >@@ -77,9 +77,13 @@ deps_buster_clang_8: deps_debian
> > # Release
> > 
> > build_debian:
> >- cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
> >+ # ENABLE_DIST required to enable tarantoolctl for test_debian_luacheck
> >+ cmake . -DENABLE_DIST=ON -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
> >  make -j
> > 
> >+build_debian_install: build_debian
> >+ sudo make install
> >+
> > test_debian_no_deps: build_debian
> >  cd test && /usr/bin/python test-run.py --force $(TEST_RUN_EXTRA_PARAMS)
> > 
> >@@ -145,6 +149,15 @@ test_static_build: deps_debian_static
> > test_static_docker_build:
> >  docker build --network=host --build-arg RUN_TESTS=ON -f Dockerfile.staticbuild .
> > 
> >+# ###################
> >+# Static Analysis
> >+# ###################
> >+
> >+test_debian_luacheck: build_debian_install
> >+ tarantoolctl rocks install luacheck
> >+ # TODO: run in parallel with LuaLanes
> >+ .rocks/bin/luacheck --codes --config .luacheckrc .
> >+
> > #######
> > # OSX #
> > #######
> >--
> >2.23.0
> >  
>  
>  
> --
> Alexander Tikhonov