ci: increase unit test timeout

Message ID 20200128162854.3367823-1-ferruh.yigit@intel.com (mailing list archive)
State Rejected, archived
Delegated to: David Marchand
Headers
Series ci: increase unit test timeout |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation fail Compilation issues

Commit Message

Ferruh Yigit Jan. 28, 2020, 4:28 p.m. UTC
  Timeout multiplier was 3, which gives 30 seconds for unit test but still
some unit test was timing out time to time and travis reporting false
positive failures.

Increasing the multiplier to 10, which makes timeout duration
100seconds.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 .ci/linux-build.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Michael Santana Jan. 28, 2020, 6:39 p.m. UTC | #1
I have been out of the loop for a while, so I do not know how many false 
positives there are. Looking at 
https://travis-ci.com/ovsrobot/dpdk/builds up to two weeks back it seems 
that hash_readwrite_lf_autotest is the only culprit. It looks like it 
lives in the neighborhood of 20-30s seconds so increasing the time out 
for this test seems appropriate as sometimes it halts at exactly 30s 
because of the time out.

Acked-by: Michael Santana <maicolgabriel@hotmail.com>

On 1/28/2020 11:28 AM, Ferruh Yigit wrote:
> Timeout multiplier was 3, which gives 30 seconds for unit test but still
> some unit test was timing out time to time and travis reporting false
> positive failures.
>
> Increasing the multiplier to 10, which makes timeout duration
> 100seconds.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>   .ci/linux-build.sh | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index ccc3a7ccd..be3dc4940 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -37,5 +37,5 @@ if [ "$AARCH64" != "1" ]; then
>   fi
>   
>   if [ "$RUN_TESTS" = "1" ]; then
> -    sudo meson test -C build --suite fast-tests -t 3
> +    sudo meson test -C build --suite fast-tests -t 10
>   fi
  
Aaron Conole Jan. 28, 2020, 8:53 p.m. UTC | #2
Ferruh Yigit <ferruh.yigit@intel.com> writes:

> Timeout multiplier was 3, which gives 30 seconds for unit test but still
> some unit test was timing out time to time and travis reporting false
> positive failures.
>
> Increasing the multiplier to 10, which makes timeout duration
> 100seconds.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---

It's okay to me.  I thought there was an effort to split out performance
part of this test from the functional part, but that seems to not have
gone anywhere.

Acked-by: Aaron Conole <aconole@redhat.com>


>  .ci/linux-build.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index ccc3a7ccd..be3dc4940 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -37,5 +37,5 @@ if [ "$AARCH64" != "1" ]; then
>  fi
>  
>  if [ "$RUN_TESTS" = "1" ]; then
> -    sudo meson test -C build --suite fast-tests -t 3
> +    sudo meson test -C build --suite fast-tests -t 10
>  fi
  
Thomas Monjalon Jan. 30, 2020, 11:03 a.m. UTC | #3
28/01/2020 21:53, Aaron Conole:
> Ferruh Yigit <ferruh.yigit@intel.com> writes:
> 
> > Timeout multiplier was 3, which gives 30 seconds for unit test but still
> > some unit test was timing out time to time and travis reporting false
> > positive failures.
> >
> > Increasing the multiplier to 10, which makes timeout duration
> > 100seconds.
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> 
> It's okay to me.  I thought there was an effort to split out performance
> part of this test from the functional part, but that seems to not have
> gone anywhere.
> 
> Acked-by: Aaron Conole <aconole@redhat.com>

NACK
The fix should be to split perf tests out of fast-tests.

The following patch is splitting hash_readwrite_autotest:
	https://patchwork.dpdk.org/patch/58726/
But we are still waiting for a patch splitting hash_readwrite_lf_autotest.
Please consider working on unit tests as a HIGH PRIORITY (using uppercase ;).
We should not have to wait so long to see performance tests removed
from fast unit tests (while keeping the functional coverage).
  
Honnappa Nagarahalli Jan. 30, 2020, 3:35 p.m. UTC | #4
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, January 30, 2020 5:03 AM
> To: Ferruh Yigit <ferruh.yigit@intel.com>; Aaron Conole
> <aconole@redhat.com>; Amit Gupta <agupta3@marvell.com>
> Cc: dev@dpdk.org; Michael Santana <maicolgabriel@hotmail.com>;
> dev@dpdk.org; Aaron Conole <aconole@redhat.com>;
> david.marchand@redhat.com; yipeng1.wang@intel.com; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Subject: Re: [dpdk-dev] [PATCH] ci: increase unit test timeout
> 
> 28/01/2020 21:53, Aaron Conole:
> > Ferruh Yigit <ferruh.yigit@intel.com> writes:
> >
> > > Timeout multiplier was 3, which gives 30 seconds for unit test but
> > > still some unit test was timing out time to time and travis
> > > reporting false positive failures.
> > >
> > > Increasing the multiplier to 10, which makes timeout duration
> > > 100seconds.
> > >
> > > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > > ---
> >
> > It's okay to me.  I thought there was an effort to split out
> > performance part of this test from the functional part, but that seems
> > to not have gone anywhere.
> >
> > Acked-by: Aaron Conole <aconole@redhat.com>
> 
> NACK
> The fix should be to split perf tests out of fast-tests.
> 
> The following patch is splitting hash_readwrite_autotest:
> 	https://patchwork.dpdk.org/patch/58726/
> But we are still waiting for a patch splitting hash_readwrite_lf_autotest.
> Please consider working on unit tests as a HIGH PRIORITY (using uppercase ;).
> We should not have to wait so long to see performance tests removed from
> fast unit tests (while keeping the functional coverage).
Apologies, it slipped from my radar. Will send out a patch soon.

> 
>
  
Honnappa Nagarahalli Jan. 31, 2020, 3:44 p.m. UTC | #5
> > Subject: Re: [dpdk-dev] [PATCH] ci: increase unit test timeout
> >
> > 28/01/2020 21:53, Aaron Conole:
> > > Ferruh Yigit <ferruh.yigit@intel.com> writes:
> > >
> > > > Timeout multiplier was 3, which gives 30 seconds for unit test but
> > > > still some unit test was timing out time to time and travis
> > > > reporting false positive failures.
> > > >
> > > > Increasing the multiplier to 10, which makes timeout duration
> > > > 100seconds.
> > > >
> > > > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > > > ---
> > >
> > > It's okay to me.  I thought there was an effort to split out
> > > performance part of this test from the functional part, but that
> > > seems to not have gone anywhere.
> > >
> > > Acked-by: Aaron Conole <aconole@redhat.com>
> >
> > NACK
> > The fix should be to split perf tests out of fast-tests.
> >
> > The following patch is splitting hash_readwrite_autotest:
> > https://patchwork.dpdk.org/patch/58726/
Thomas/David, is it possible to merge this patch? It is acked already. It would be good to have these changes for LF functional tests.

> > But we are still waiting for a patch splitting hash_readwrite_lf_autotest.
> > Please consider working on unit tests as a HIGH PRIORITY (using uppercase ;).
> > We should not have to wait so long to see performance tests removed
> > from fast unit tests (while keeping the functional coverage).
> Apologies, it slipped from my radar. Will send out a patch soon.
> 
> >
> >
>
  
David Marchand Feb. 5, 2020, 6:47 p.m. UTC | #6
On Thu, Jan 30, 2020 at 12:03 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 28/01/2020 21:53, Aaron Conole:
> > Ferruh Yigit <ferruh.yigit@intel.com> writes:
> >
> > > Timeout multiplier was 3, which gives 30 seconds for unit test but still
> > > some unit test was timing out time to time and travis reporting false
> > > positive failures.
> > >
> > > Increasing the multiplier to 10, which makes timeout duration
> > > 100seconds.
> > >
> > > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > > ---
> >
> > It's okay to me.  I thought there was an effort to split out performance
> > part of this test from the functional part, but that seems to not have
> > gone anywhere.
> >
> > Acked-by: Aaron Conole <aconole@redhat.com>
>
> NACK
> The fix should be to split perf tests out of fast-tests.
>
> The following patch is splitting hash_readwrite_autotest:
>         https://patchwork.dpdk.org/patch/58726/
> But we are still waiting for a patch splitting hash_readwrite_lf_autotest.
> Please consider working on unit tests as a HIGH PRIORITY (using uppercase ;).
> We should not have to wait so long to see performance tests removed
> from fast unit tests (while keeping the functional coverage).

Just applied series
https://patchwork.dpdk.org/project/dpdk/list/?series=8401&state=*.
So marking this patch as rejected.

Thanks.
  

Patch

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index ccc3a7ccd..be3dc4940 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -37,5 +37,5 @@  if [ "$AARCH64" != "1" ]; then
 fi
 
 if [ "$RUN_TESTS" = "1" ]; then
-    sudo meson test -C build --suite fast-tests -t 3
+    sudo meson test -C build --suite fast-tests -t 10
 fi