On 17.03.2021 15:05, Rémi Bernon wrote:
On 3/17/21 2:52 PM, Jacek Caban wrote:
On 17.03.2021 14:33, Rémi Bernon wrote:
+ size = 0xdeadbeef; + hr = IVectorView_Gamepad_get_Size(gamepads, &size); + ok(SUCCEEDED(hr), "IVectorView_Gamepad_QueryInterface failed, hr %#x\n", hr); + todo_wine ok(size != 0xdeadbeef, "IVectorView_Gamepad_get_Size returned %u\n", size);
+ rc = IVectorView_Gamepad_Release(gamepads); + todo_wine ok(rc == 1, "IVectorView_Gamepad_Release returned unexpected refcount %d\n", rc);
There's a typo in the message here and the refcount test could probably be dropped as I dropped the others. Compared to windows.media.speechsynthesis tests, refcount for these classes isn't always the same on the testbot Windows VMs, so it didn't seem very relevant to keep the tests.
Yes, tests for exact ref count values are very rarely interesting and it would be better to drop them.
However, I've got more tests to send with GamepadAdded/Removed event handlers as well as RawGameController runtimeclass, so unless there's some other changes to make elsewhere I'd rather not resend this series just for that.
There are some things that I would find nice to change. I didn't want to reject speech patches just for that, but since it's becoming a template, it's worth mentioning:
- Testing for the exact (usually S_OK) value instead of SUCCEED() in
ok() macros is generally better. It's more strict and catches cases where we should return things like S_FALSE instead.
- I would call tests something like input.c instead. We will never
have more test files for most of those DLLs, so if we need some 'non-statics' tests in the future, it would be good if they fit the existing file.
Thanks,
Jacek
Right, works for me. If the widl and include changes are good, I can resend the last 4 patches later with the test changes (and clean the speech tests accordingly to make them consistent).
Sounds good to me.
Jacek