winspool/tests: New Testsuite (coding style)
Detlef Riekenberg
wine.dev at web.de
Mon Sep 26 14:40:57 CDT 2005
Am Sonntag, den 25.09.2005, 19:17 -0600 schrieb Vitaliy Margolen:
> No kidding it's so large! You can shrink it down to probably 10% of it's current
> size.
We will see, what's left after rework.
> > +/* ############################
A Separator makes the source more readable but i try to reduce that.
> > + if(opt_verbose)
> > + {
> > + trace("-----\n");
> > + }
> This is not really informative to print it at all.
Accepted.
> > + 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());
In this case, GetlastError() inside ok() works, but most times
i must take care, that NtCurrentTeb()->LastErrorValue is never
changed by any function called during a trace() / ok().
For this Reasons, i decided to cache the Result of GetLastError() in my
code.
My goal was to have the same style for every test,
but I change it to be more "K.I.S.S".
Since i want to test as much as possible, I test for:
- the returncode,
- then lasterror
- and then the returned data.
This is needed, because we have many bugs in all areas.
Example:
- Define a printer in linux.
- Run wine with an app that uses winspool.drv
- Delete your Linux-Printer
- winspool.drv in wine will report now:
* EnumPrinters => 0: correct,
* GetDefaultPrinter => your deleted printer: this is wrong
> Looks much better this way:
> if(pGetPrinterDriverDirectoryA == NULL) return;
> Wine uses this style please follow it:
> if ()
> {
> }
> else
> {
> }
Really?
cvs from yesterday:
grep -R "\}.*else" * | wc -l
4402
grep -R " * else" * | grep -v "\}.*else" | wc -l
10672
I can change that, but then we have:
a: if (check) function;
b: if (check) {
function1;
function2;
}
c: if (check)
{
function_true;
}
else
{
function_false;
}
No common Style.
I learned the Style i used in my Patch from the Editor "jfe", because
jfe did Autoident and other things with that Style.
I try to change that in my Patches, but "not fall back" to my old Style
is difficult .
> > + 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.
I will do a deep view to this..
> > +#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.
The Printing-Environments are defined my Microsoft.
"%SystemRoot%\system32\spool\drivers" and
"HKLM\System\CurrentControlSet\Control\Print\Environments"
are your friends :-)
PS: Our Registry-Entries for the driver is incomplete and wrong
> And don't invent platform detection. It's already done just use it.
Did you mean "sys_isNT" with this?
When a ok() is going to fail, we must make sure that this is really a
Failure on the Tested System.
Example: GetDefaultPrinterW is present, but not implemented
This is a failure on NT but ok on win9x.
A simple if() that find an unimplemented "GetDefaultPrinterW" could just
decide to return from the test_function but my way find more errors.
(I knew, that this is not perfect: With "Services for Unicode" on win9x,
no failure is allowed).
> Most of the stuff that you added to dlls/winspool/tests/info.c is useless.
I do not want to duplicate Code in the Testfile, so i concentrate the
common Helper-Functions in one File.
Since every Sourcefile must include the Filename as test name and
"info.c" was present before, i used that.
I removed many tests in different Files and the "not yet needed"
Helper-Functions from "info.c" to reduce the size of the Patch.
--
By By ...
... Detlef
More information about the wine-devel
mailing list