winspool: Add a test for DeviceCapabilities, fix some bugs found

[Detlef Riekenberg]

On Mi, 2007-04-04 at 11:06 +0900, Dmitry Timoshkov wrote:
> "Detlef Riekenberg" <wine.dev at web.de> wrote:
> 
> >> +    hComdlg32 = LoadLibrary("comdlg32.dll");
> >> +    assert(hComdlg32);
> >> +    pPrintDlg = GetProcAddress(hComdlg32, "PrintDlgA");
> >> +    assert(pPrintDlg);
> >
> > Why are you using assert() and kill the test?
> > Is skip not enough?
> 
> Skip is supposed to skip tests for optional functionality that doesn't
> present on some systems, that's not the case here.
> 
> >> +    ok(dm != NULL, "GlobalLock(prn_dlg.hDevMode) failed\n");
> >> +    trace("dmDeviceName \"%s\"\n", dm->dmDeviceName);
> >
> > You added a ok() for "dm =! NULL", but "dm" is accessed always and will
> > crash the test.
> >> +    ok(dn != NULL, "GlobalLock(prn_dlg.hDevNames) failed\n");
> >> +    ok(dn->wDriverOffset, "expected not 0 wDriverOffset\n");
> >
> > Same here for "dn".
> 
> That's the point of the test. dm and dn must not be NULL in all cases.




> >> +    ok(lstrcmp((const char *)dm->dmDeviceName, (const char *)dn + dn->wDeviceOffset) == 0, "device names not 
> >> match\n");
> >
> > You introduced the same bug, as you did in in your previous Patch!
> > (test_DocumentProperties)
> >
> > dm->dmDeviceName is limited to CCHDEVICENAME
> 
> I don't see a problem here.

You introduced the same bug, as you did in in your previous Patch for
"winspool.drv", which was commited on 15. April 2006 
(test_DocumentProperties and test_DEVMODE)

Example failure for "Microsoft Office Document Image Writer":
http://test.winehq.org/data/200605051000/xp_RAYEN/winspool.drv:info.txt

More Examples for failing tests:
http://test.winehq.org/data/200605051000/#XP
http://test.winehq.org/data/200604231000/#XP


> > info.c:2093:fields 7b13
> > info.c:2095: Test succeeded inside todo block: fields 7b13 != dmFields
> > 7b13
> >
> > I get the same "succeeded inside todo" for all printers
> 
> I have only 1 configured printer here: "Wine Postscript Driver", and the test
> fails for me.

I use wineps.drv (WINEPS Printer using CUPS), but the drivers have the
same name as the cups-printer (patch from Huw a while ago).
My current cups-Printers are "cups-pdf" and 2 Parallel-Printers.

> > Why is it so difficult to add the tests in alphabetic order?
> 
> Some tests may depend on the results of other tests, or cause side effects,

In this case, the test is broken and need to be fixed
(similar to the side-effects, that Paul fixed for advapi32)

> Feel free to send a patch and re-arrange the tests.
You are able to fix it: Please fix it. 

> > This Patch dump a lot of unneeded data.
> > Please remove the traces (or dump the data only in interactive mode).
> The traces are supposed to help understanding what's wrong with the test when
> somebody looks at the test results at test.winehq.org and trys to figure out
> how to fix a failing test.

Dump additional data, when a test failed is ok, 
but in your Patch, the data is always dumped.

> > You are testing "PrintDlgA" in the testsuite for "winspool.drv".
> > The correct location is: dlls/comdlg32/tests/printdlg.c

> The test exercises the functionality of winspool, but does that with the help
> of PrintDlgA to avoid code duplication. The test doesn't add a hard dependency
> on comdlg32.dll, so I don't see a problem.

msi/tests use DeleteFile, but you would never add tests for DeleteFile
in the testsuite for msi.

Please add the tests related to PrintDlgA in
dlls/comdlg32/tests/printdlg.c

Thanks

-- 
 
By by ... Detlef