Rémi Bernon rbernon@codeweavers.com writes:
On 4/19/22 11:57, Bernhard Kölbl wrote:
WaitForSingleObject(compilation_handler.event_finished,INFINITE);
todo_wineok(!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.