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

[Nikolay Sivov]

On 05.07.2016 20:04, Alex Henrie wrote:
> 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?

I usually add them, but I don't think it's a strict requirement.

> 
> -Alex
>