[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 <rbernon at codeweavers.com> 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.
-- 
Rémi Bernon <rbernon at codeweavers.com>