[Tarantool-patches] [PATCH v1] test: add more tests to packaging testing

[Tarantool-patches] [PATCH v1] test: add more tests to packaging testing

[Alexander V. Tikhonov]

Changed number of tests to packaging testing jobs from
suites: "unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/",
to all suites except 'replication/' suite. Also added
testing to the all available packing jobs except 'CentOS 6'.

Removed test-run option that run testing inline to be
able to run it in parallel. Also changed 'test-run' to
'make test-force' command.

Closes #4599
---

Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4599-packages-testing-full-ci
Issue: https://github.com/tarantool/tarantool/issues/4599

 rpm/tarantool.spec | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
index ff95ed646..bc1fb5aff 100644
--- a/rpm/tarantool.spec
+++ b/rpm/tarantool.spec
@@ -70,17 +70,18 @@ BuildRequires: libunwind-devel
 %endif
 
 # For tests
-%if (0%{?fedora} >= 22 || 0%{?rhel} == 7)
-BuildRequires: python >= 2.7
-BuildRequires: python-six >= 1.9.0
-BuildRequires: python-gevent >= 1.0
-BuildRequires: python-yaml >= 3.0.9
-%endif
 %if (0%{?fedora} >= 31 || 0%{?rhel} >= 8)
 BuildRequires: python2 >= 2.7
 BuildRequires: python2-six >= 1.9.0
 BuildRequires: python2-gevent >= 1.0
 BuildRequires: python2-yaml >= 3.0.9
+%else
+%if (0%{?rhel} != 6)
+BuildRequires: python >= 2.7
+BuildRequires: python-six >= 1.9.0
+BuildRequires: python-gevent >= 1.0
+BuildRequires: python-yaml >= 3.0.9
+%endif
 %endif
 
 Name: tarantool
@@ -160,13 +161,13 @@ make %{?_smp_mflags}
 rm -rf %{buildroot}%{_datarootdir}/doc/tarantool/
 
 %check
-%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
 # https://github.com/tarantool/tarantool/issues/1227
 echo "self.skip = True" > ./test/app/socket.skipcond
 # https://github.com/tarantool/tarantool/issues/1322
 echo "self.skip = True" > ./test/app/digest.skipcond
 # run a safe subset of the test suite
-cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/
+%if (0%{?rhel} != 6)
+	TEST_RUN_EXCLUDE='replication/' make test-force
 %endif
 
 %pre
-- 
2.17.1

Re: [Tarantool-patches] [PATCH v1] test: add more tests to packaging testing

[Alexander Turenko]

I have several questions, see below.

WBR, Alexander Turenko.

On Mon, Dec 30, 2019 at 02:10:07PM +0300, Alexander V. Tikhonov wrote:
> Changed number of tests to packaging testing jobs from
> suites: "unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/",
> to all suites except 'replication/' suite. Also added
> testing to the all available packing jobs except 'CentOS 6'.
> 
> Removed test-run option that run testing inline to be
> able to run it in parallel. Also changed 'test-run' to
> 'make test-force' command.
> 
> Closes #4599
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4599-packages-testing-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/4599
> 
>  rpm/tarantool.spec | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
> index ff95ed646..bc1fb5aff 100644
> --- a/rpm/tarantool.spec
> +++ b/rpm/tarantool.spec
> @@ -70,17 +70,18 @@ BuildRequires: libunwind-devel
>  %endif
>  
>  # For tests
> -%if (0%{?fedora} >= 22 || 0%{?rhel} == 7)
> -BuildRequires: python >= 2.7
> -BuildRequires: python-six >= 1.9.0
> -BuildRequires: python-gevent >= 1.0
> -BuildRequires: python-yaml >= 3.0.9
> -%endif
>  %if (0%{?fedora} >= 31 || 0%{?rhel} >= 8)
>  BuildRequires: python2 >= 2.7
>  BuildRequires: python2-six >= 1.9.0
>  BuildRequires: python2-gevent >= 1.0
>  BuildRequires: python2-yaml >= 3.0.9
> +%else
> +%if (0%{?rhel} != 6)

Don't understood: what was actually changed? We have Fedora 28-31 and
CentOS 6-8 in testing / deploying.

> +BuildRequires: python >= 2.7
> +BuildRequires: python-six >= 1.9.0
> +BuildRequires: python-gevent >= 1.0
> +BuildRequires: python-yaml >= 3.0.9
> +%endif
>  %endif
>  
>  Name: tarantool
> @@ -160,13 +161,13 @@ make %{?_smp_mflags}
>  rm -rf %{buildroot}%{_datarootdir}/doc/tarantool/
>  
>  %check
> -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
>  # https://github.com/tarantool/tarantool/issues/1227
>  echo "self.skip = True" > ./test/app/socket.skipcond
>  # https://github.com/tarantool/tarantool/issues/1322
>  echo "self.skip = True" > ./test/app/digest.skipcond

Those two issues now closed. Maybe it is time to enable those tests?

>  # run a safe subset of the test suite

I would file an issue re enabling replication test suite and mention it
here: it looks as good process for me. However, up to you.

> -cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/
> +%if (0%{?rhel} != 6)
> +	TEST_RUN_EXCLUDE='replication/' make test-force
>  %endif

Flaky fails rate will increase on Travis-CI if we'll enable
parallelization. Either discard this change or enable it only for
gitlab-ci.

Nit: Use whitespace (not tab) for indentation here.

Re: [Tarantool-patches] [PATCH v1] test: add more tests to packaging testing

[Alexander Tikhonov]

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


Alexander, thanks for your review, I've made all the changes that you asked.


>Пятница, 21 февраля 2020, 14:19 +03:00 от Alexander Turenko < alexander.turenko@tarantool.org >:
>
>I have several questions, see below.
>
>WBR, Alexander Turenko.
>
>On Mon, Dec 30, 2019 at 02:10:07PM +0300, Alexander V. Tikhonov wrote:
>> Changed number of tests to packaging testing jobs from
>> suites: "unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/",
>> to all suites except 'replication/' suite. Also added
>> testing to the all available packing jobs except 'CentOS 6'.
>> 
>> Removed test-run option that run testing inline to be
>> able to run it in parallel. Also changed 'test-run' to
>> 'make test-force' command.
>> 
>> Closes #4599
>> ---
>> 
>> Github:  https://github.com/tarantool/tarantool/tree/avtikhon/gh-4599-packages-testing-full-ci
>> Issue:  https://github.com/tarantool/tarantool/issues/4599
>> 
>>  rpm/tarantool.spec | 17 +++++++++--------
>>  1 file changed, 9 insertions(+), 8 deletions(-)
>> 
>> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
>> index ff95ed646..bc1fb5aff 100644
>> --- a/rpm/tarantool.spec
>> +++ b/rpm/tarantool.spec
>> @@ -70,17 +70,18 @@ BuildRequires: libunwind-devel
>>  %endif
>> 
>>  # For tests
>> -%if (0%{?fedora} >= 22 || 0%{?rhel} == 7)
>> -BuildRequires: python >= 2.7
>> -BuildRequires: python-six >= 1.9.0
>> -BuildRequires: python-gevent >= 1.0
>> -BuildRequires: python-yaml >= 3.0.9
>> -%endif
>>  %if (0%{?fedora} >= 31 || 0%{?rhel} >= 8)
>>  BuildRequires: python2 >= 2.7
>>  BuildRequires: python2-six >= 1.9.0
>>  BuildRequires: python2-gevent >= 1.0
>>  BuildRequires: python2-yaml >= 3.0.9
>> +%else
>> +%if (0%{?rhel} != 6)
>
>Don't understood: what was actually changed? We have Fedora 28-31 and
>CentOS 6-8 in testing / deploying. 
Please check the next comment.
>
>
>> +BuildRequires: python >= 2.7
>> +BuildRequires: python-six >= 1.9.0
>> +BuildRequires: python-gevent >= 1.0
>> +BuildRequires: python-yaml >= 3.0.9
>> +%endif
>>  %endif
>>  
In the current change and one before, the check of the versions added. Before it
was that only Fedora and CentOS installed the python packages for the testing.
As after the change all OSs (except CentOS 6) will have it installed except
Fedora since 31 version and CentOS since 8 version will have python2.
>
>>  Name: tarantool
>> @@ -160,13 +161,13 @@ make %{?_smp_mflags}
>>  rm -rf %{buildroot}%{_datarootdir}/doc/tarantool/
>> 
>>  %check
>> -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
>>  #  https://github.com/tarantool/tarantool/issues/1227
>>  echo "self.skip = True" > ./test/app/socket.skipcond
>>  #  https://github.com/tarantool/tarantool/issues/1322
>>  echo "self.skip = True" > ./test/app/digest.skipcond
>
>Those two issues now closed. Maybe it is time to enable those tests?
Sure, I'll add separate commit into the current patch set.
>
>>  # run a safe subset of the test suite
>
>I would file an issue re enabling replication test suite and mention it
>here: it looks as good process for me. However, up to you.
Sure - added new issue:
   https://github.com/tarantool/tarantool/issues/4798
>
>
>> -cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/
>> +%if (0%{?rhel} != 6)
>> +	TEST_RUN_EXCLUDE='replication/' make test-force
>>  %endif
>
>Flaky fails rate will increase on Travis-CI if we'll enable
>parallelization. Either discard this change or enable it only for
>gitlab-ci. 
Added mark 'IS_TRAVIS' to be changed by packpack on run.
>
>
>Nit: Use whitespace (not tab) for indentation here. 
Corrected.
>


-- 
Alexander Tikhonov

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

Re: [Tarantool-patches] [PATCH v1] test: add more tests to packaging testing

[Alexander Turenko]

> >>  # For tests
> >> -%if (0%{?fedora} >= 22 || 0%{?rhel} == 7)
> >> -BuildRequires: python >= 2.7
> >> -BuildRequires: python-six >= 1.9.0
> >> -BuildRequires: python-gevent >= 1.0
> >> -BuildRequires: python-yaml >= 3.0.9
> >> -%endif
> >>  %if (0%{?fedora} >= 31 || 0%{?rhel} >= 8)
> >>  BuildRequires: python2 >= 2.7
> >>  BuildRequires: python2-six >= 1.9.0
> >>  BuildRequires: python2-gevent >= 1.0
> >>  BuildRequires: python2-yaml >= 3.0.9
> >> +%else
> >> +%if (0%{?rhel} != 6)
> >
> >Don't understood: what was actually changed? We have Fedora 28-31 and
> >CentOS 6-8 in testing / deploying. 
> Please check the next comment.
> >
> >
> >> +BuildRequires: python >= 2.7
> >> +BuildRequires: python-six >= 1.9.0
> >> +BuildRequires: python-gevent >= 1.0
> >> +BuildRequires: python-yaml >= 3.0.9
> >> +%endif
> >>  %endif
> >>  
> In the current change and one before, the check of the versions added. Before it
> was that only Fedora and CentOS installed the python packages for the testing.
> As after the change all OSs (except CentOS 6) will have it installed except
> Fedora since 31 version and CentOS since 8 version will have python2.

After I discussed it voicely with Alexander I got what was changed.

Fedora 31 and CentOS 8 installs both python-foo and python2-foo
dependencies, while python-foo is actually alias for python3-foo. It
does not fail the build, but is not used.

I'm okay to remove unused Python 3 deps, but let's add a comment here
(right in the spec): it is not obvious, you see.

> >>  %check
> >> -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
> >>  #  https://github.com/tarantool/tarantool/issues/1227
> >>  echo "self.skip = True" > ./test/app/socket.skipcond
> >>  #  https://github.com/tarantool/tarantool/issues/1322
> >>  echo "self.skip = True" > ./test/app/digest.skipcond
> >
> >Those two issues now closed. Maybe it is time to enable those tests?
> Sure, I'll add separate commit into the current patch set.

As I see on the branch (avtikhon/gh-4599-packages-testing-full-ci), you
removed it right in this commit. Anyway, it is okay for me.

> >> -cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/
> >> +%if (0%{?rhel} != 6)
> >> +	TEST_RUN_EXCLUDE='replication/' make test-force
> >>  %endif
> >
> >Flaky fails rate will increase on Travis-CI if we'll enable
> >parallelization. Either discard this change or enable it only for
> >gitlab-ci. 
> Added mark 'IS_TRAVIS' to be changed by packpack on run.

Commented in https://github.com/packpack/packpack/pull/116

Re: [Tarantool-patches] [PATCH v1] test: add more tests to packaging testing

[Alexander Tikhonov]

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

Alexander, thanks for the review, added comment as you suggested.


>Четверг, 12 марта 2020, 14:03 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
>
>> >>  # For tests
>> >> -%if (0%{?fedora} >= 22 || 0%{?rhel} == 7)
>> >> -BuildRequires: python >= 2.7
>> >> -BuildRequires: python-six >= 1.9.0
>> >> -BuildRequires: python-gevent >= 1.0
>> >> -BuildRequires: python-yaml >= 3.0.9
>> >> -%endif
>> >>  %if (0%{?fedora} >= 31 || 0%{?rhel} >= 8)
>> >>  BuildRequires: python2 >= 2.7
>> >>  BuildRequires: python2-six >= 1.9.0
>> >>  BuildRequires: python2-gevent >= 1.0
>> >>  BuildRequires: python2-yaml >= 3.0.9
>> >> +%else
>> >> +%if (0%{?rhel} != 6)
>> >
>> >Don't understood: what was actually changed? We have Fedora 28-31 and
>> >CentOS 6-8 in testing / deploying. 
>> Please check the next comment.
>> >
>> >
>> >> +BuildRequires: python >= 2.7
>> >> +BuildRequires: python-six >= 1.9.0
>> >> +BuildRequires: python-gevent >= 1.0
>> >> +BuildRequires: python-yaml >= 3.0.9
>> >> +%endif
>> >>  %endif
>> >> 
>> In the current change and one before, the check of the versions added. Before it
>> was that only Fedora and CentOS installed the python packages for the testing.
>> As after the change all OSs (except CentOS 6) will have it installed except
>> Fedora since 31 version and CentOS since 8 version will have python2.
>
>After I discussed it voicely with Alexander I got what was changed.
>
>Fedora 31 and CentOS 8 installs both python-foo and python2-foo
>dependencies, while python-foo is actually alias for python3-foo. It
>does not fail the build, but is not used.
>
>I'm okay to remove unused Python 3 deps, but let's add a comment here
>(right in the spec): it is not obvious, you see.
>
>> >>  %check
>> >> -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
>> >>  #  https://github.com/tarantool/tarantool/issues/1227
>> >>  echo "self.skip = True" > ./test/app/socket.skipcond
>> >>  #  https://github.com/tarantool/tarantool/issues/1322
>> >>  echo "self.skip = True" > ./test/app/digest.skipcond
>> >
>> >Those two issues now closed. Maybe it is time to enable those tests?
>> Sure, I'll add separate commit into the current patch set.
>
>As I see on the branch (avtikhon/gh-4599-packages-testing-full-ci), you
>removed it right in this commit. Anyway, it is okay for me.
>
>> >> -cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/
>> >> +%if (0%{?rhel} != 6)
>> >> +	TEST_RUN_EXCLUDE='replication/' make test-force
>> >>  %endif
>> >
>> >Flaky fails rate will increase on Travis-CI if we'll enable
>> >parallelization. Either discard this change or enable it only for
>> >gitlab-ci. 
>> Added mark 'IS_TRAVIS' to be changed by packpack on run.
>
>Commented in  https://github.com/packpack/packpack/pull/116


-- 
Alexander Tikhonov

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