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

[Nikolay Sivov]

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.

> 
> -Alex
>