winspool/tests: fix broken test for GetPrinterDriver (updated)

[Dmitry Timoshkov]

"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.

-- 
Dmitry.