Re: gdi32: Add tests for GetICMProfile and SetICMMode.

[Detlef Riekenberg]

On Mo, 2008-01-21 at 16:33 +0100, Hans Leidekker wrote:
> +static void test_GetICMProfileA( HDC dc )

Please check GetLastError() as much as possible
(Add comments, when GetLastError() does not return something usable
> +    size = MAX_PATH;
> +    ret = GetICMProfileA( dc, &size, NULL );
> +    ok( !ret, "GetICMProfileA succeeded\n" );

This is different from GetICMProfileW and MSDN.
Please add a comment.
> +    size = 0;
> +    SetLastError(0xdeadbeef);
> +    ret = GetICMProfileA( dc, &size, filename );
> +    error = GetLastError();
> +    ok( !ret, "GetICMProfileA succeeded\n" );
> +    ok( size, "expected size > 0\n" );
> +    ok( error == ERROR_INSUFFICIENT_BUFFER, "got %d, expected
> ERROR_INSUFFICIENT_BUFFER\n", error );

what are the advantages compared to one test for all?

  ok ( !ret && (GetLastError() == ERROR_INSUFFICIENT_BUFFER) &&
       (size > 0), 
       "got %u with %u and size %d (expected '0' with "
       "ERROR_INSUFFICIENT_BUFFER and '> 0'\n",
       ret, GetLastError(), size);
> +    size = MAX_PATH;
> +    ret = GetICMProfileA( dc, &size, filename );
> +    ok( ret, "GetICMProfileA failed %d\n", GetLastError() );

Does the size include the terminating '\0'?
what about tests with:
size == needed_size -1
size == needed_size
size == needed_size +1



I'm unable to test your Patch now. will try to do it tomorrow.


-- 
 
By by ... Detlef

gdi32: Add tests for GetICMProfile and SetICMMode.

[James Hawkins]

On Jan 21, 2008 6:25 PM, Detlef Riekenberg <wine.dev(a)web.de> wrote:
> On Mo, 2008-01-21 at 16:33 +0100, Hans Leidekker wrote:
> > +static void test_GetICMProfileA( HDC dc )
>
> Please check GetLastError() as much as possible
> (Add comments, when GetLastError() does not return something usable
> > +    size = MAX_PATH;
> > +    ret = GetICMProfileA( dc, &size, NULL );
> > +    ok( !ret, "GetICMProfileA succeeded\n" );
>
> This is different from GetICMProfileW and MSDN.
> Please add a comment.
> > +    size = 0;
> > +    SetLastError(0xdeadbeef);
> > +    ret = GetICMProfileA( dc, &size, filename );
> > +    error = GetLastError();
> > +    ok( !ret, "GetICMProfileA succeeded\n" );
> > +    ok( size, "expected size > 0\n" );
> > +    ok( error == ERROR_INSUFFICIENT_BUFFER, "got %d, expected
> > ERROR_INSUFFICIENT_BUFFER\n", error );
>
> what are the advantages compared to one test for all?

So that you can see exactly what part is not conforming to the expected value.
> ok ( !ret && (GetLastError() == ERROR_INSUFFICIENT_BUFFER) &&
>        (size > 0),
>        "got %u with %u and size %d (expected '0' with "
>        "ERROR_INSUFFICIENT_BUFFER and '> 0'\n",
>        ret, GetLastError(), size);

This is very poor form for unit tests.  The key word is 'unit',
meaning one.  How can you read anything in that mess?

-- 
James Hawkins