winspool/tests: fix broken test for GetPrinterDriver (updated)
dmitry at codeweavers.com
Mon May 1 09:52:01 CDT 2006
"Detlef Riekenberg" <wine.dev at web.de> wrote:
>> > + /* needed is modified in win9x. testing for "needed == (DWORD)-1" will fail */
>> That's interesting, is there a pattern how win9x modifies it?
> + trace("(%d) returned %d with 0x%08lx/%ld and 0x%08lx/%ld\n", level,
> + ret, GetLastError(), GetLastError(), needed, needed);
> Nothing usable:
> IMHO, it's an address when level==-1 (ME: 0x0041afd4, 95:0x0051000c)
> On level==0 it's set to 0.
> In the other cases, the value depends on the Default Printer.
> I have seen 213, 257, 294, 297, 319, 401
> There is no Documentation, what the OS does with this Parameter when the
> Function failed.
An absent documentation should not stop us from testing such cases.
>> > + }
>> > + if(GetLastError() != ERROR_INSUFFICIENT_BUFFER) continue;
>> This line above is misplaced, better do not remove 'continue;' in the invalid
>> level case.
> This will not work.
> The first "continue" (ERROR_OUTOFMEMORY-check) leaves the if-case but
> goes on in the for-loop.
> Using a nested if is IMHO the ugly way.
The first 'continue;' which you have added with the following line
+ if(!ret && (GetLastError() == ERROR_INVALID_LEVEL)) continue;
catches a not supported level on older platforms. Btw, !ret check is redundant
since it was already done earlier. The second 'continue;' already has been there,
and it handled an invalid level case, there is no point falling through in that
case, therefore my comment.
> Initializing "needed" with 0 is the easy way: We do not get an
> ERROR_OUTOFMEMORY from win9x and can remove the extra check,
That doesn't matter here.
> but you
> told us, that you do not want "needed=(DWORD)-1" to change.
'needed' is initialized to a not possible value in order to catch all possible
modifications from the API side. Since needed == 0 is one of possible values,
we couldn't notice if the API changes something.
> It's not our Fault, that the App, that you want to get working, is so
> buggy, that it depends on such an undocumented crap.
Please don't blame the app you know nothing about, my test case has nothing
to do with it. I wrote that test in order to check if the API behaves correctly
just to [dis]prove my suspicions.
> Fixing your broken tests is really Time-consuming:
> test_DEVMODE failed, when the Printername has more
> TCHAR than CCHDEVICENAME (32)
My tests are not broken. The fact that they don't pass on some no more
existing/relevant systems doesn't invalidate the tests. Frankly, I don't
see much point in running and fixing them on that platforms, I'd consider
that just an academic task.
More information about the wine-devel