[v1,4/6] test: fence: Increase timeout for fence test

19949 diff mbox series

On slower machines, a 1s timeout to capture frames with vimc can be too
short and cause test failures. Make the timeout proportional to the
number of frames expected to be captured, using a conservative low
estimate of the frame rate at 2fps.

By itself, that change could increase the test time quite substantially
on fast platforms, so break from the capture loop as soon as we capture
enough frames.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/fence.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Quoting Laurent Pinchart (2024-04-25 00:42:22)
> On slower machines, a 1s timeout to capture frames with vimc can be too
> short and cause test failures. Make the timeout proportional to the
> number of frames expected to be captured, using a conservative low
> estimate of the frame rate at 2fps.
> 
> By itself, that change could increase the test time quite substantially
> on fast platforms, so break from the capture loop as soon as we capture
> enough frames.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  test/fence.cpp | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/test/fence.cpp b/test/fence.cpp
> index 7949bfbb176b..4ce6dfa23590 100644
> --- a/test/fence.cpp
> +++ b/test/fence.cpp
> @@ -315,8 +315,9 @@ int FenceTest::run()
>  
>         /* Loop for one second. */

We're no longer looping for one second ?

>         Timer timer;
> -       timer.start(1000ms);
> -       while (timer.isRunning() && expectedCompletionResult_) {
> +       timer.start(500ms * (signalledRequestId_ + 1));

So that's where the * 2 is now coming in? Maybe 'signalledRequestId_'
just isn't named well...

> +       while (timer.isRunning() && expectedCompletionResult_ &&
> +              completedRequestId_ <= signalledRequestId_ + 1) {
>                 if (completedRequestId_ == signalledRequestId_ - 1 && setFence_)
>                         /*
>                          * signalledRequestId_ has just completed and it has
> -- 
> Regards,
> 
> Laurent Pinchart
>

On Wed, May 29, 2024 at 03:11:19PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-04-25 00:42:22)
> > On slower machines, a 1s timeout to capture frames with vimc can be too
> > short and cause test failures. Make the timeout proportional to the
> > number of frames expected to be captured, using a conservative low
> > estimate of the frame rate at 2fps.
> > 
> > By itself, that change could increase the test time quite substantially
> > on fast platforms, so break from the capture loop as soon as we capture
> > enough frames.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  test/fence.cpp | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/test/fence.cpp b/test/fence.cpp
> > index 7949bfbb176b..4ce6dfa23590 100644
> > --- a/test/fence.cpp
> > +++ b/test/fence.cpp
> > @@ -315,8 +315,9 @@ int FenceTest::run()
> >  
> >         /* Loop for one second. */
> 
> We're no longer looping for one second ?

You're right, I'll update this.

> >         Timer timer;
> > -       timer.start(1000ms);
> > -       while (timer.isRunning() && expectedCompletionResult_) {
> > +       timer.start(500ms * (signalledRequestId_ + 1));
> 
> So that's where the * 2 is now coming in? Maybe 'signalledRequestId_'
> just isn't named well...

signalledRequestId_ is the last request that we need to test. The
timeout allows 500ms per request. This has nothing to do with the * 2.

> > +       while (timer.isRunning() && expectedCompletionResult_ &&
> > +              completedRequestId_ <= signalledRequestId_ + 1) {
> >                 if (completedRequestId_ == signalledRequestId_ - 1 && setFence_)
> >                         /*
> >                          * signalledRequestId_ has just completed and it has