[PATCH] dinput/tests: Make overlapped format tests more robust.

[Rémi Bernon]

On 8/5/21 9:57 AM, Arkadiusz Hiler wrote:
> On Wed, Aug 04, 2021 at 11:44:14PM +0200, Rémi Bernon wrote:
>> On 8/4/21 11:00 PM, Arkadiusz Hiler wrote:
>>> On Wed, Aug 04, 2021 at 02:25:49PM -0500, Zebediah Figura (she/her) wrote:
>>>> On 8/4/21 2:20 PM, Rémi Bernon wrote:
> <SNIP>
>>>>>> +    tries = 2;
>>>>>> +    do
>>>>>> +    {
>>>>>> +        /* press D */
>>>>>> +        keybd_event(0, DIK_D, KEYEVENTF_SCANCODE, 0);
>>>>>> +        pump_messages();
>>>>>> +    } while (!wait_for_device_data_and_discard(keyboard) && tries--);
>>>>>>         memset(&state, 0xFF, sizeof(state));
>>>>>>         hr = IDirectInputDevice_GetDeviceState(keyboard,
>>>>>> sizeof(state), &state);
>>>>>
>>>>> I think it could be worth having a dedicated helper to acquire and wait,
>>>>> instead of inlining these loops. The same helper could then be (copied)
>>>>> and used in the other tests.
>>>>>
>>>>> The tries counter and its hardcoded value is a bit confusing at first,
>>>>> and instead of keeping the key pressed the helper could also make sure
>>>>> it's released on function exit (maybe use a dedicated unusual key for
>>>>> that?).
>>>
>>> "Unusual key" may be hard to pick as AFAIU it has to be present in the
>>> data format.
>>>
>>> How about something like:
>>>
>>> static void flush_acquire_keyboard_(int line, IDirectInputDeviceA *device, DWORD valid_dik)
>>> {
>>>       int tries = 2;
>>>       do
>>>       {
>>>           keybd_event(0, valid_dik, KEYEVENTF_SCANCODE, 0);
>>>       } while (!wait_for_device_data_and_discard(device) && tries--);
>>>
>>>       keybd_event(0, valid_dik, KEYEVENTF_SCANCODE|KEYEVENTF_KEYUP, 0);
>>>       ok_(__FILE__, line)(wait_for_device_data_and_discard(device),
>>>               "Timed out while waiting for injected events to be picked up by DirectInput.\n");
>>> }
>>>
>>> #define flush_acquire_keyboard(device, valid_dik) flush_acquire_keyboard_(__LINE__, device, valid_dik)
>>>
>>> And then we can:
>>>
>>> hr = IDirectInputDevice_Acquire(keyboard);
>>> ok(SUCCEEDED(hr), "IDirectInputDevice_Acquire() failed: %08x\n", hr);
>>> flush_acquire_keyboard(keyboard, DIK_F);
>>>
>>
>> Why not even call Acquire inside the helper (and name it acquire_and_wait or
>> something like that)?
> 
> Doesn't really matter. I have a slight preference for slim helpers that
> do only one thing.
> 

Sure, but doing Acquire in the same helper still makes sense IMHO, as 
some kind of atomic "blocking acquire" operation that only returns when 
the device has really been acquired.
-- 
Rémi Bernon <rbernon at codeweavers.com>