Sunday, September 25, 2005, 5:44:36 PM, Detlef Riekenberg wrote:
> If the Patch is to Large (i already removed the Unicode-Tests)
> i can split the Testsuite, but this will reduce the amount of testing,
> compared to the current CVS.
No kidding it's so large! You can shrink it down to probably 10% of it's current
size.
> +
> +/* ############################
> + *
> + */
> +
Please remove these lines.
> + if(opt_verbose)
> + {
> + trace("-----\n");
> + }
> +
This is not really informative to print it at all.
> + buffer = HeapAlloc(GetProcessHeap(), 0, size);
> + lasterror=GetLastError();
> + if(opt_verbose)
> + {
> + trace("%p: HeapAlloc(%p, 0, 0x%lx/%ld) => (0x%lx/%ld)\n", buffer,
> + GetProcessHeap(), size, size, lasterror, lasterror);
> +
> + put_lasterror();
> + }
> + ok(buffer != NULL, "allocating 0x%ld/%ld Bytes for Buffer =>
> (0x%lx/%ld)\n",
> + size, size, lasterror, lasterror);
Will look better like this:
buffer = HeapAlloc(GetProcessHeap(), 0, size);
ok(buffer != NULL, "HeapAlloc(%ld) error=%lx\n", size, GetLastError());
> + if(pGetPrinterDriverDirectoryA == NULL)
> + {
> + return ;
> + }
> +
Looks much better this way:
if(pGetPrinterDriverDirectoryA == NULL) return;
> + }else
> + {
Wine uses this style please follow it:
if ()
{
}
else
{
}
> + is_result_ok(FALSE);
Is not helpfull really. Indicate what failed. Adding extra verbose traces makes
output too big and hard to find what really failed.
> +#define ENVIRONMENT_NULL 0
> +#define ENVIRONMENT_WIN40 1
> +#define ENVIRONMENT_W32X86 2
> +#define ENVIRONMENT_IA64 3
> +#define ENVIRONMENT_W32ALPHA 4
> +#define ENVIRONMENT_W32PPC 5
> +#define ENVIRONMENT_W32MIPS 6
> +#define ENVIRONMENT_MAX ENVIRONMENT_W32MIPS
Please look inside include/wine/test.h and other wine test files. And don't
invent platform detection. It's already done just use it. Or don't define
something that you don't need.
Most of the stuff that you added to dlls/winspool/tests/info.c is useless.
Vitaliy
