[PATCH v2 3/5] shell32/tests: Add tests for FolderItems.

[Alex Henrie]

2016-07-05 10:59 GMT-06:00 Nikolay Sivov <bunglehead at gmail.com>:
> On 05.07.2016 19:56, Alex Henrie wrote:
>> 2016-07-05 10:54 GMT-06:00 Nikolay Sivov <bunglehead at gmail.com>:
>>> On 05.07.2016 18:46, Alex Henrie wrote:
>>>> 2016-07-05 0:26 GMT-06:00 Nikolay Sivov <bunglehead at gmail.com>:
>>>>> On 05.07.2016 8:47, Alex Henrie wrote:
>>>>>> +    if (0) /* crashes on all versions of Windows */
>>>>>> +    {
>>>>>> +        r = FolderItems_get_Count(items, NULL);
>>>>>> +todo_wine
>>>>>> +        ok(r == E_INVALIDARG, "expected E_INVALIDARG, got %08x\n", r);
>>>>>> +    }
>>>>>
>>>>> Why do you want to handle this case if it crashes consistently on
>>>>> Windows? And todo_wine is not going to do anything in commented block
>>>>> obviously.
>>>>
>>>> Good point. Would you like me to remove this code altogether, or just
>>>> the todo_wine and ok statement? Also, would you like me to change the
>>>> other if (0) blocks to if (GetVersion() > MAKELONG(5,1)) to avoid
>>>> having todo_wine's in if (0) blocks?
>>>
>>> If it crashes on some windows versions just put it in 'if (0)' with
>>> appropriate comment for documentation purposes, and a NULL check could
>>> be implemented. Version 5,1 means XP, right? if it does not crash since
>>> XP just make it a regular test, enabled unconditionally.
>>
>> It crashes on XP, and I'm pretty sure that it's not OK to have tests
>> that crash the XP testbot. I'll remove the todo_wine's though.
>
> Ah, right, it was a greater-than check. So yes, if it only crashes on
> XP, and works on everything newer we should implement newer behavior and
> have disabled test that documents all that. No need for todo_wine.

And you don't want any if (0) blocks that crash on all versions of
Windows, right?

-Alex