[Tarantool-patches] [PATCH v1] Set full testing for all branches

[Tarantool-patches] [PATCH v1] Set full testing for all branches

[Alexander V. Tikhonov]

Set full testing with deploy builds and tests for all branches.
---

Github: https://github.com/tarantool/tarantool/tree/avtikhon/all-tests-in-branches

 .gitlab-ci.yml | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 256b368c4..55b006ed4 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -8,11 +8,6 @@ variables:
 
 # Jobs templates
 
-.release_only_template: &release_only_definition
-  only:
-    - master
-    - /^.*-full-ci$/
-
 .deploy_only_template: &deploy_only_definition
   only:
     - master
@@ -26,7 +21,8 @@ variables:
     - schedules
     - external_pull_requests
     - merge_requests
-    - /^.*-full-ci$/
+    - branches
+    - tags
 
 .perf_only_template: &perf_only_definition
   only:
@@ -124,7 +120,6 @@ release_clang:
     - ${GITLAB_MAKE} test_debian_no_deps
 
 release_lto:
-  <<: *release_only_definition
   <<: *docker_test_clang8_definition
   variables:
     CMAKE_EXTRA_PARAMS: -DENABLE_LTO=ON
@@ -132,7 +127,6 @@ release_lto:
     - ${GITLAB_MAKE} test_debian_no_deps
 
 release_lto_clang8:
-  <<: *release_only_definition
   <<: *docker_test_clang8_definition
   variables:
     CC: clang-8
@@ -147,7 +141,6 @@ release_asan_clang8:
     - ${GITLAB_MAKE} test_asan_debian_no_deps
 
 osx_14_release:
-  <<: *release_only_definition
   stage: test
   tags:
     - osx_14
@@ -162,7 +155,6 @@ osx_15_release:
     - ${GITLAB_MAKE} test_osx
 
 osx_15_release_lto:
-  <<: *release_only_definition
   stage: test
   tags:
     - osx_15
@@ -466,7 +458,6 @@ static_build:
     - ${GITLAB_MAKE} test_static_build
 
 static_docker_build:
-  <<: *release_only_definition
   stage: test
   tags:
     - deploy_test
-- 
2.17.1

Re: [Tarantool-patches] [PATCH v1] Set full testing for all branches

[Sergey Bronnikov]

Hi, Alexander

thanks for the patch, LGTM

On 21:35 Wed 20 May , Alexander V. Tikhonov wrote:
> Set full testing with deploy builds and tests for all branches.
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/all-tests-in-branches
> 
>  .gitlab-ci.yml | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 256b368c4..55b006ed4 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -8,11 +8,6 @@ variables:
>  
>  # Jobs templates
>  
> -.release_only_template: &release_only_definition
> -  only:
> -    - master
> -    - /^.*-full-ci$/
> -
>  .deploy_only_template: &deploy_only_definition
>    only:
>      - master
> @@ -26,7 +21,8 @@ variables:
>      - schedules
>      - external_pull_requests
>      - merge_requests
> -    - /^.*-full-ci$/
> +    - branches
> +    - tags
>  
>  .perf_only_template: &perf_only_definition
>    only:
> @@ -124,7 +120,6 @@ release_clang:
>      - ${GITLAB_MAKE} test_debian_no_deps
>  
>  release_lto:
> -  <<: *release_only_definition
>    <<: *docker_test_clang8_definition
>    variables:
>      CMAKE_EXTRA_PARAMS: -DENABLE_LTO=ON
> @@ -132,7 +127,6 @@ release_lto:
>      - ${GITLAB_MAKE} test_debian_no_deps
>  
>  release_lto_clang8:
> -  <<: *release_only_definition
>    <<: *docker_test_clang8_definition
>    variables:
>      CC: clang-8
> @@ -147,7 +141,6 @@ release_asan_clang8:
>      - ${GITLAB_MAKE} test_asan_debian_no_deps
>  
>  osx_14_release:
> -  <<: *release_only_definition
>    stage: test
>    tags:
>      - osx_14
> @@ -162,7 +155,6 @@ osx_15_release:
>      - ${GITLAB_MAKE} test_osx
>  
>  osx_15_release_lto:
> -  <<: *release_only_definition
>    stage: test
>    tags:
>      - osx_15
> @@ -466,7 +458,6 @@ static_build:
>      - ${GITLAB_MAKE} test_static_build
>  
>  static_docker_build:
> -  <<: *release_only_definition
>    stage: test
>    tags:
>      - deploy_test
> -- 
> 2.17.1
> 

-- 
sergeyb@

Re: [Tarantool-patches] [PATCH v1] Set full testing for all branches

[Oleg Piskunov]

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


LGTM

  
>Четверг, 21 мая 2020, 22:43 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
> 
>Hi, Alexander
>
>thanks for the patch, LGTM
>
>On 21:35 Wed 20 May , Alexander V. Tikhonov wrote:
>> Set full testing with deploy builds and tests for all branches.
>> ---
>>
>> Github:  https://github.com/tarantool/tarantool/tree/avtikhon/all-tests-in-branches
>>
>> .gitlab-ci.yml | 13 ++-----------
>> 1 file changed, 2 insertions(+), 11 deletions(-)
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index 256b368c4..55b006ed4 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -8,11 +8,6 @@ variables:
>>
>> # Jobs templates
>>
>> -.release_only_template: &release_only_definition
>> - only:
>> - - master
>> - - /^.*-full-ci$/
>> -
>> .deploy_only_template: &deploy_only_definition
>> only:
>> - master
>> @@ -26,7 +21,8 @@ variables:
>> - schedules
>> - external_pull_requests
>> - merge_requests
>> - - /^.*-full-ci$/
>> + - branches
>> + - tags
>>
>> .perf_only_template: &perf_only_definition
>> only:
>> @@ -124,7 +120,6 @@ release_clang:
>> - ${GITLAB_MAKE} test_debian_no_deps
>>
>> release_lto:
>> - <<: *release_only_definition
>> <<: *docker_test_clang8_definition
>> variables:
>> CMAKE_EXTRA_PARAMS: -DENABLE_LTO=ON
>> @@ -132,7 +127,6 @@ release_lto:
>> - ${GITLAB_MAKE} test_debian_no_deps
>>
>> release_lto_clang8:
>> - <<: *release_only_definition
>> <<: *docker_test_clang8_definition
>> variables:
>> CC: clang-8
>> @@ -147,7 +141,6 @@ release_asan_clang8:
>> - ${GITLAB_MAKE} test_asan_debian_no_deps
>>
>> osx_14_release:
>> - <<: *release_only_definition
>> stage: test
>> tags:
>> - osx_14
>> @@ -162,7 +155,6 @@ osx_15_release:
>> - ${GITLAB_MAKE} test_osx
>>
>> osx_15_release_lto:
>> - <<: *release_only_definition
>> stage: test
>> tags:
>> - osx_15
>> @@ -466,7 +458,6 @@ static_build:
>> - ${GITLAB_MAKE} test_static_build
>>
>> static_docker_build:
>> - <<: *release_only_definition
>> stage: test
>> tags:
>> - deploy_test
>> --
>> 2.17.1
>>
>--
>sergeyb@ 
 
 
--
Oleg Piskunov
 

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

Re: [Tarantool-patches] [PATCH v1] Set full testing for all branches

[Alexander Turenko]

On Wed, May 20, 2020 at 09:35:31PM +0300, Alexander V. Tikhonov wrote:
> Set full testing with deploy builds and tests for all branches.
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/all-tests-in-branches
> 
>  .gitlab-ci.yml | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 256b368c4..55b006ed4 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -8,11 +8,6 @@ variables:
>  
>  # Jobs templates
>  
> -.release_only_template: &release_only_definition
> -  only:
> -    - master
> -    - /^.*-full-ci$/
> -
>  .deploy_only_template: &deploy_only_definition
>    only:
>      - master
>    except:
>      - schedules
>      - external_pull_requests
>      - merge_requests
>
>  .pack_only_template: &pack_only_definition
>    only:
>      - schedules
>      - external_pull_requests
>      - merge_requests
> -    - /^.*-full-ci$/
> +    - branches
> +    - tags

First, there is not reason to list 'schedules', 'external_pull_requests'
and so if you want to just run it everywhere.

Second, we should not run both 'just packaging' and 'packaging and
deployment' jobs on master. I would make ''pack_only_template' be just
'except: master'. Yep, it is not perfect: 'schedules',
'external_pull_requests' will don't run neither 'pack', nor 'deploy'.
But this change is simple and downsides are acceptable.

By the way, I don't know anything about pipeline for tagged commits in
GitLab. What jobs will run on a tagged commit? Are we affected by
[#3745][4] in context of GitLab CI deployments? Just curious, not part
of this task.

Just side note: I still believe that things would be simpler if we would
deploy within a packaging job: this way we need only condition 'when not
to deploy' and don't need its negation. (I proposed this in [1].) Now I
see that it would be implemented not just like as it is done in Travis
CI, because GitLab CI is weird about separate build and deploy stages
(see [2], [3]), but would be implemented inside one jobs. Anyway, I
don't insist, especially within this task.

[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/013893.html
[2]: https://gitlab.com/gitlab-org/gitlab-runner/-/issues/336
[3]: https://gitlab.com/gitlab-org/gitlab/-/issues/29265
[4]: https://github.com/tarantool/tarantool/issues/3745

WBR, Alexander Turenko.

Re: [Tarantool-patches] [PATCH v1] Set full testing for all branches

[Alexander V. Tikhonov]

Hi Alexander, thanks for the review, please check comments below.

On Mon, Jun 01, 2020 at 03:39:50PM +0300, Alexander Turenko wrote:
> On Wed, May 20, 2020 at 09:35:31PM +0300, Alexander V. Tikhonov wrote:
> > Set full testing with deploy builds and tests for all branches.
> > ---
> > 
> > Github: https://github.com/tarantool/tarantool/tree/avtikhon/all-tests-in-branches
> > 
> >  .gitlab-ci.yml | 13 ++-----------
> >  1 file changed, 2 insertions(+), 11 deletions(-)
> > 
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 256b368c4..55b006ed4 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -8,11 +8,6 @@ variables:
> >  
> >  # Jobs templates
> >  
> > -.release_only_template: &release_only_definition
> > -  only:
> > -    - master
> > -    - /^.*-full-ci$/
> > -
> >  .deploy_only_template: &deploy_only_definition
> >    only:
> >      - master
> >    except:
> >      - schedules
> >      - external_pull_requests
> >      - merge_requests
> >
> >  .pack_only_template: &pack_only_definition
> >    only:
> >      - schedules
> >      - external_pull_requests
> >      - merge_requests
> > -    - /^.*-full-ci$/
> > +    - branches
> > +    - tags
> 
> First, there is not reason to list 'schedules', 'external_pull_requests'
> and so if you want to just run it everywhere.
> 

Actually I just wanted to make it more visible for the users, but anyway
if it not really needed I'll change it to "except: master".

> Second, we should not run both 'just packaging' and 'packaging and
> deployment' jobs on master. I would make ''pack_only_template' be just
> 'except: master'. Yep, it is not perfect: 'schedules',
> 'external_pull_requests' will don't run neither 'pack', nor 'deploy'.
> But this change is simple and downsides are acceptable.
> 

Right, it is the same as previous my comment.

> By the way, I don't know anything about pipeline for tagged commits in
> GitLab. What jobs will run on a tagged commit? Are we affected by
> [#3745][4] in context of GitLab CI deployments? Just curious, not part
> of this task.
> 

Right, tagged commit as I understood is the same as have tagged version
in the git, but sure it may be some other meaning, due to we never saw
it running.

> Just side note: I still believe that things would be simpler if we would
> deploy within a packaging job: this way we need only condition 'when not
> to deploy' and don't need its negation. (I proposed this in [1].) Now I
> see that it would be implemented not just like as it is done in Travis
> CI, because GitLab CI is weird about separate build and deploy stages
> (see [2], [3]), but would be implemented inside one jobs. Anyway, I
> don't insist, especially within this task.
>

Current patch is the simple change, while I already prepared another one
with overall changes in gitlab-ci configuration, you can check it in the
new branch avtikhon/branches-gitlab-ci .

> [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/013893.html
> [2]: https://gitlab.com/gitlab-org/gitlab-runner/-/issues/336
> [3]: https://gitlab.com/gitlab-org/gitlab/-/issues/29265
> [4]: https://github.com/tarantool/tarantool/issues/3745
> 
> WBR, Alexander Turenko.

Re: [Tarantool-patches] [PATCH v1] Set full testing for all branches

[Alexander Turenko]

I have no more objections.

If everything works as expected and we're ready to such amount of jobs,
the change will allow to catch some problems before a patch will land
into master.

BTW, please add 'gitlab-ci' prefix.

> > >  .pack_only_template: &pack_only_definition
> > >    only:
> > >      - schedules
> > >      - external_pull_requests
> > >      - merge_requests
> > > -    - /^.*-full-ci$/
> > > +    - branches
> > > +    - tags
> > 
> > First, there is not reason to list 'schedules', 'external_pull_requests'
> > and so if you want to just run it everywhere.
> > 
> 
> Actually I just wanted to make it more visible for the users, but anyway
> if it not really needed I'll change it to "except: master".
> 
> > Second, we should not run both 'just packaging' and 'packaging and
> > deployment' jobs on master. I would make ''pack_only_template' be just
> > 'except: master'. Yep, it is not perfect: 'schedules',
> > 'external_pull_requests' will don't run neither 'pack', nor 'deploy'.
> > But this change is simple and downsides are acceptable.
> > 
> 
> Right, it is the same as previous my comment.

I would mention the known downside somewhere, preferably just in this
config.

Re: [Tarantool-patches] [PATCH v1] Set full testing for all branches

[Kirill Yukhin]

Hello,

On 20 май 21:35, Alexander V. Tikhonov wrote:
> Set full testing with deploy builds and tests for all branches.
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/all-tests-in-branches

I've checked your patch into 1.10, 2.3, 2.4 and master.

--
Reagrds, Kirill Yukhin