Re: winspool/tests: fix broken test for GetPrinterDriver (updated)
"Detlef Riekenberg" <wine.dev(a)web.de> wrote:
else { - ok(GetLastError() == ERROR_INVALID_LEVEL, "wrong error %ld\n", GetLastError()); - ok(needed == (DWORD)-1,"not expected needed buffer size %ld\n", needed); - continue; - } + /* ERROR_OUTOFMEMORY from win9x go away, when needed was initiated with 0 */ + ok( !ret && + ((GetLastError() == ERROR_INVALID_LEVEL) || + (GetLastError() == ERROR_OUTOFMEMORY)), + "%d: returned %d with %ld (expected '0' with: " \ + "ERROR_INVALID_LEVEL or ERROR_OUTOFMEMORY)\n", + level, ret, GetLastError()); + /* needed is modified in win9x. testing for "needed == (DWORD)-1" will fail */
That's interesting, is there a pattern how win9x modifies it?
+ } + if(GetLastError() != ERROR_INSUFFICIENT_BUFFER) continue;
This line above is misplaced, better do not remove 'continue;' in the invalid level case. -- Dmitry.
Am Sonntag, den 30.04.2006, 11:57 +0900 schrieb Dmitry Timoshkov:
+ /* 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.
+ } + 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. Initializing "needed" with 0 is the easy way: We do not get an ERROR_OUTOFMEMORY from win9x and can remove the extra check, but you told us, that you do not want "needed=(DWORD)-1" to change. 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. Fixing your broken tests is really Time-consuming: test_DEVMODE failed, when the Printername has more TCHAR than CCHDEVICENAME (32) You can see the failing tests on XP in the WRT-Results. When the Default Printer is on a Server, the Limit is reached fast. NT3.51 has more failures: dmDriverExtra is 0 and dmDevicename is empty. (dmSpecVersion and dmDriverVersion are also 0) -- By By ... ... Detlef
"Detlef Riekenberg" <wine.dev(a)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.
Dmitry Timoshkov wrote:
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.
But only, when an Application need this undocumented behaviour. There is no way to test the modification of "needed" on win9x. IMHO Alexandre will not accept an NT-only test with GetVersion() here.
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.
dmDeviceName is documented here: http://msdn.microsoft.com/library/en-us/gdi/prntspol_8nle.asp failure overview for a still existing/relevant system: http://test.winehq.org/data/200604231000/#XP Example Results: http://test.winehq.org/data/200604231000/xp_hello/winspool.drv:info.txt http://test.winehq.org/data/200604231000/xp_PC-Top-Winkel3/winspool.drv:info... http://test.winehq.org/data/200604231000/xp_winxptest1-1/winspool.drv:info.t...
Frankly, I don't see much point in running and fixing them on that platforms,
IMHO, as long as wine support Apps, that are designed for win9x(95-ME), the tests should not fail on that platforms, if possible.
I'd consider that just an academic task.
I know, that NT3.51 is really old and incorrect in many places, but i have a License and I run the tests on this System as a "fun project". -- By By ... ... Detlef
participants (2)
-
Detlef Riekenberg -
Dmitry Timoshkov