[PATCH 1/2] dinput/tests: Do not test that reports are identical when polling.

[Rémi Bernon]

On 5/3/22 01:09, Zebediah Figura (she/her) wrote:
> On 5/2/22 17:06, Rémi Bernon wrote:
>> On 5/2/22 23:48, Zebediah Figura wrote:
>>> This is dependent on timing, and currently fails occasionally both on 
>>> Windows
>>> and Wine.
>>>
>>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
>>> ---
>>> For examples of failures:
>>>
>>> http://test.winehq.org/data/4ec67b7a6447dfc4af8c03c141c600b41b90ef53/win81_newtb-w8/dinput:hid.html 
>>>
>>> http://test.winehq.org/data/64b96eec7d0aea470f897a3ed0ac9e1b3a680cc5/linux_fgtb-debian11-win32/dinput:hid.html 
>>>
>>>
>>>   dlls/dinput/tests/hid.c | 6 ++----
>>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/dlls/dinput/tests/hid.c b/dlls/dinput/tests/hid.c
>>> index 4e107625064..1162f154f8e 100644
>>> --- a/dlls/dinput/tests/hid.c
>>> +++ b/dlls/dinput/tests/hid.c
>>> @@ -2312,14 +2312,13 @@ static void test_hidp( HANDLE file, HANDLE 
>>> async_file, int report_id, BOOL polle
>>>           ok( ret, "GetOverlappedResult failed, last error %lu\n", 
>>> GetLastError() );
>>>           ok( value == (report_id ? 3 : 4), "GetOverlappedResult 
>>> returned length %lu, expected %u\n",
>>>               value, (report_id ? 3 : 4) );
>>> -        /* first report should be ready and the same */
>>> +        /* first report should be ready */
>>>           ret = GetOverlappedResult( async_file, &overlapped, &value, 
>>> FALSE );
>>>           ok( ret, "GetOverlappedResult failed, last error %lu\n", 
>>> GetLastError() );
>>>           ok( value == (report_id ? 3 : 4), "GetOverlappedResult 
>>> returned length %lu, expected %u\n",
>>>               value, (report_id ? 3 : 4) );
>>>           ok( memcmp( report, buffer + caps.InputReportByteLength, 
>>> caps.InputReportByteLength ),
>>>               "expected different report\n" );
>>> -        ok( !memcmp( report, buffer, caps.InputReportByteLength ), 
>>> "expected identical reports\n" );
>>>           value = 10;
>>>           SetLastError( 0xdeadbeef );
>>
>>
>> This defeats a bit the purpose of the test, which is to validate that 
>> in polled mode all pending reads should be completed at once when 
>> device poll completes, whereas in non-polled mode, only one pending 
>> read should complete for each received report.
> 
> I figured as much, and my inclination is that the test just isn't worth 
> keeping around in that case.
> 
> Perhaps it can be marked interactive instead.
> 


I don't think interactive tests are useful, especially not here where 
there's no need for user input.


> Alternatively, as I'm sitting here trying to brainstorm, maybe we could, 
> say, set the poll interval to 100 ms, queue 10 reads, and verify that 
> they all complete in less than 200 ms. That'd also allow us to avoid 
> infinite waits.
> 
>> I don't think this one is failing so often.
> 
> It's not often, but we really shouldn't be letting tests fail at all.
> 

It actually never failed, only the second one does.

-- 
Rémi Bernon <rbernon at codeweavers.com>