Re: [PATCH v4 1/4] windows.media.speech/tests: Add test to check if IAsyncInfo_Close non is blocking.

[Rémi Bernon]

On 4/19/22 17:28, Alexandre Julliard wrote:
> Rémi Bernon  writes:
> > On 4/19/22 11:57, Bernhard Kölbl wrote:
> > > > > -
> > > > >    WaitForSingleObject(compilation_handler.event_finished,
> > > >
> > > > INFINITE);
> > > > > -
> > > > >    todo_wine
> > > >
> > > > ok(!WaitForSingleObject(compilation_handler.event_finished, 1000), "Wait
> > > > for event_finished failed.\n");
> > > > >         CloseHandle(compilation_handler.event_finished);
> > > > >
> > > > >         hr =
> > > >
> > > > IAsyncOperation_SpeechRecognitionCompilationResult_put_Completed(operation,
> > > > NULL);
> > > >
> > > > I don't understand why you need to change this now? If it is going to
> > > > fail and timeout after you stubbed some method, maybe it'd be better to
> > > > make the change with the stub.
> > >
> > > The reason for this change is so the tests can't be blocked forever
> > > and you
> > > get a message if it failed. 1000ms is long enough for the finished event to
> > > fire. Makes debugging the tests easier imho.
> >
> > I think having a non-INFINITE timeout makes the intent less clear. If
> > the wait is never supposed to timeout, INFINITE makes it obvious
> > imho. It also makes sure you will never get spurious failures even if
> > for some reason a run gets particularly slow.
> >
> > If for some reason the test breaks and the wait starts blocking, the
> > testbot timeout is there to catch it, although yes, it's a bit longer.
>
> When I'm running make test before pushing commits, I don't get the
> testbot timeout, so it's nice if it doesn't get stuck forever.

Okay, nevermind what I said then.