[1/2] kernel32/tests: add tests for GetDllDirectoryA

[Thomas Faber]

On 2011-11-06 18:04, Vitaliy Margolen wrote:
> On 11/06/2011 09:14 AM, Thomas Faber wrote:
>>
> 
>> +        length = strlen(dll_directories[i]);
>> +        assert(sizeof(buffer) > length + 1);
> Please don't do this. Tests shouldn't assert because programmer failed to pick the right buffer size
> of hard-coded strings.

Check.

>> +        ok(GetLastError() == 0xdeadbeef, "Error is %u\n", GetLastError());
> You should get value of GetLastError() before the ok check here. See all places that check values of
> GetLastError().

Check.

>> +        ok(buffer[length + 1] == 'A', "i=%d, Buffer overflow\n", i);
>> +        ok(buffer[ret] == 0, "i=%d, Wrong length or not null terminated\n", i);
>> +        ok(strcmp(buffer, dll_directories[i]) == 0, "i=%d, Wrong path returned: '%s'\n", i, buffer);
> You can do all these with one memcmp call.

Indeed. That's rather a leftover from the other testcases.

>> +        ok(buffer[length + 1] == 'A', "i=%d, Buffer overflow\n", i);
>> +        ok(buffer[ret] == 0, "i=%d, Wrong length or not null terminated\n", i);
>> +        for (j = 0; buffer[j]; j++)
>> +            if (buffer[j] != dll_directories[i][j])
>> +            {
>> +                ok(0, "i=%d, Wrong path returned: %s\n", i, wine_dbgstr_w(buffer));
>> +                break;
>> +            }
> Same for this.

This one's rather tricky, as it's a comparison between an ansi and a unicode string
(I didn't want to have the directories listed twice).
I'll think of something nicer.

>> +        ok(buffer[0] == 0 || /* XP, 2003 */
>> +           buffer[0] == 'A', "i=%d, Buffer overflow\n", i);
> Please mark bad case with broke() to check for correct Wine behavior.

Hmm, which one is broken though?
Wine currently has XP's behavior, that is, put a null in there, even if the buffer
size is zero.
This is consistent with the A version of the function, but does seem less sensible.

> Also please combine A & W tests. They do mostly the same things.

Agreed.


Thanks for the review! I'll make an updated patch.

-Tom