Set/WindowLong problem in user/win test
This test uses {Get,Set}WindowLongW which of course does not work on Win9x, thus causing the test to fail. I guess part of the goal is to check that mixing Ansi calls (CreatewindowExA) with Unicode calls works. So I wrote a function test_set_window_long function that calls SetWindowLongW and then SetWindowLongA and checks that their results match but that causes the test to fail (at least on NT4 and Wine). More precisely even if all this function does is the following, then the test fails: { LONG rc=SetWindowLongA(hwnd,nIndex,dwNewLong); SetWindowLongA(hwnd,nIndex,dwNewLong); return rc; } Comment out the second SetWindowLongA and it works. Why would calling SetWindowLong a second time make any difference? Here's the full patch, just change '#if 0' into '#if 1' to make the test fail. Index: dlls/user/tests/win.c =================================================================== RCS file: /home/wine/wine/dlls/user/tests/win.c,v retrieving revision 1.5 diff -u -r1.5 win.c --- dlls/user/tests/win.c 15 Nov 2002 00:02:51 -0000 1.5 +++ dlls/user/tests/win.c 24 Dec 2002 03:16:36 -0000 @@ -45,6 +45,47 @@ static HWND hwndMain, hwndMain2; +static LONG test_set_window_long(HWND hwnd, int nIndex, LONG dwNewLong) +{ + LONG rc; + + SetLastError(0); + rc=SetWindowLongW(hwnd,nIndex,dwNewLong); + if (rc==0 && GetLastError()==ERROR_CALL_NOT_IMPLEMENTED) { + rc=SetWindowLongA(hwnd,nIndex,dwNewLong); + } +#if 0 + else { + LONG set,get; + get=GetWindowLongW(hwnd,nIndex); + set=SetWindowLongA(hwnd,nIndex,dwNewLong); + ok(set==get, + "SetWindowLongA/W(%ld) failed: got %ld, expected %ld\n", + dwNewLong,set,get); + } +#endif + return rc; +} + +static LONG test_get_window_long(HWND hwnd, int nIndex) +{ + LONG rc; + + SetLastError(0); + rc=GetWindowLongW(hwnd,nIndex); + if (rc==0 && GetLastError()==ERROR_CALL_NOT_IMPLEMENTED) { + rc=GetWindowLongA(hwnd,nIndex); + } else { + LONG rcA; + SetLastError(0); + rcA=GetWindowLongA(hwnd,nIndex); + ok(rc==rcA, + "GetWindowLongW failed: got %ld, expected %ld\n", + rc,rcA); + } + return rc; +} + /* check the values returned by the various parent/owner functions on a given window */ static void check_parents( HWND hwnd, HWND ga_parent, HWND gwl_parent, HWND get_parent, HWND gw_owner, HWND ga_root, HWND ga_root_owner ) @@ -56,7 +97,7 @@ res = pGetAncestor( hwnd, GA_PARENT ); ok( res == ga_parent, "Wrong result for GA_PARENT %p expected %p", res, ga_parent ); } - res = (HWND)GetWindowLongW( hwnd, GWL_HWNDPARENT ); + res = (HWND)test_get_window_long( hwnd, GWL_HWNDPARENT ); ok( res == gwl_parent, "Wrong result for GWL_HWNDPARENT %p expected %p", res, gwl_parent ); res = GetParent( hwnd ); ok( res == get_parent, "Wrong result for GetParent %p expected %p", res, get_parent ); @@ -97,31 +138,31 @@ /* desktop window */ check_parents( desktop, 0, 0, 0, 0, 0, 0 ); - style = GetWindowLongW( desktop, GWL_STYLE ); - ok( !SetWindowLongW( desktop, GWL_STYLE, WS_POPUP ), "Set GWL_STYLE on desktop succeeded" ); - ok( !SetWindowLongW( desktop, GWL_STYLE, 0 ), "Set GWL_STYLE on desktop succeeded" ); - ok( GetWindowLongW( desktop, GWL_STYLE ) == style, "Desktop style changed" ); + style = test_get_window_long( desktop, GWL_STYLE ); + ok( !SetWindowLongA( desktop, GWL_STYLE, WS_POPUP ), "Set GWL_STYLE on desktop succeeded" ); + ok( !SetWindowLongA( desktop, GWL_STYLE, 0 ), "Set GWL_STYLE on desktop succeeded" ); + ok( test_get_window_long( desktop, GWL_STYLE ) == style, "Desktop style changed" ); /* normal child window */ test = create_tool_window( WS_CHILD, hwndMain ); trace( "created child %p\n", test ); check_parents( test, hwndMain, hwndMain, hwndMain, 0, hwndMain, hwndMain ); - SetWindowLongW( test, GWL_STYLE, 0 ); + test_set_window_long( test, GWL_STYLE, 0 ); check_parents( test, hwndMain, hwndMain, 0, 0, hwndMain, test ); - SetWindowLongW( test, GWL_STYLE, WS_POPUP ); + test_set_window_long( test, GWL_STYLE, WS_POPUP ); check_parents( test, hwndMain, hwndMain, 0, 0, hwndMain, test ); - SetWindowLongW( test, GWL_STYLE, WS_POPUP|WS_CHILD ); + test_set_window_long( test, GWL_STYLE, WS_POPUP|WS_CHILD ); check_parents( test, hwndMain, hwndMain, 0, 0, hwndMain, test ); - SetWindowLongW( test, GWL_STYLE, WS_CHILD ); + test_set_window_long( test, GWL_STYLE, WS_CHILD ); DestroyWindow( test ); /* child of desktop */ test = create_tool_window( WS_CHILD, desktop ); trace( "created child of desktop %p\n", test ); check_parents( test, desktop, 0, desktop, 0, test, desktop ); - SetWindowLongW( test, GWL_STYLE, WS_POPUP ); + test_set_window_long( test, GWL_STYLE, WS_POPUP ); check_parents( test, desktop, 0, 0, 0, test, test ); - SetWindowLongW( test, GWL_STYLE, 0 ); + test_set_window_long( test, GWL_STYLE, 0 ); check_parents( test, desktop, 0, 0, 0, test, test ); DestroyWindow( test ); @@ -129,9 +170,9 @@ test = create_tool_window( WS_CHILD, child ); trace( "created child of child %p\n", test ); check_parents( test, child, child, child, 0, hwndMain, hwndMain ); - SetWindowLongW( test, GWL_STYLE, 0 ); + test_set_window_long( test, GWL_STYLE, 0 ); check_parents( test, child, child, 0, 0, hwndMain, test ); - SetWindowLongW( test, GWL_STYLE, WS_POPUP ); + test_set_window_long( test, GWL_STYLE, WS_POPUP ); check_parents( test, child, child, 0, 0, hwndMain, test ); DestroyWindow( test ); @@ -139,9 +180,9 @@ test = create_tool_window( 0, 0 ); trace( "created top-level %p\n", test ); check_parents( test, desktop, 0, 0, 0, test, test ); - SetWindowLongW( test, GWL_STYLE, WS_POPUP ); + test_set_window_long( test, GWL_STYLE, WS_POPUP ); check_parents( test, desktop, 0, 0, 0, test, test ); - SetWindowLongW( test, GWL_STYLE, WS_CHILD ); + test_set_window_long( test, GWL_STYLE, WS_CHILD ); check_parents( test, desktop, 0, desktop, 0, test, desktop ); DestroyWindow( test ); @@ -149,9 +190,9 @@ test = create_tool_window( 0, hwndMain ); trace( "created owned top-level %p\n", test ); check_parents( test, desktop, hwndMain, 0, hwndMain, test, test ); - SetWindowLongW( test, GWL_STYLE, WS_POPUP ); + test_set_window_long( test, GWL_STYLE, WS_POPUP ); check_parents( test, desktop, hwndMain, hwndMain, hwndMain, test, hwndMain ); - SetWindowLongW( test, GWL_STYLE, WS_CHILD ); + test_set_window_long( test, GWL_STYLE, WS_CHILD ); check_parents( test, desktop, hwndMain, desktop, hwndMain, test, desktop ); DestroyWindow( test ); @@ -159,9 +200,9 @@ test = create_tool_window( WS_POPUP, 0 ); trace( "created popup %p\n", test ); check_parents( test, desktop, 0, 0, 0, test, test ); - SetWindowLongW( test, GWL_STYLE, WS_CHILD ); + test_set_window_long( test, GWL_STYLE, WS_CHILD ); check_parents( test, desktop, 0, desktop, 0, test, desktop ); - SetWindowLongW( test, GWL_STYLE, 0 ); + test_set_window_long( test, GWL_STYLE, 0 ); check_parents( test, desktop, 0, 0, 0, test, test ); DestroyWindow( test ); @@ -169,9 +210,9 @@ test = create_tool_window( WS_POPUP, hwndMain ); trace( "created owned popup %p\n", test ); check_parents( test, desktop, hwndMain, hwndMain, hwndMain, test, hwndMain ); - SetWindowLongW( test, GWL_STYLE, WS_CHILD ); + test_set_window_long( test, GWL_STYLE, WS_CHILD ); check_parents( test, desktop, hwndMain, desktop, hwndMain, test, desktop ); - SetWindowLongW( test, GWL_STYLE, 0 ); + test_set_window_long( test, GWL_STYLE, 0 ); check_parents( test, desktop, hwndMain, 0, hwndMain, test, test ); DestroyWindow( test ); @@ -210,7 +251,7 @@ /* desktop window */ check_parents( desktop, 0, 0, 0, 0, 0, 0 ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, (LONG_PTR)hwndMain2 ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, (LONG_PTR)hwndMain2 ); ok( !ret, "Set GWL_HWNDPARENT succeeded on desktop" ); check_parents( desktop, 0, 0, 0, 0, 0, 0 ); ok( !SetParent( desktop, hwndMain ), "SetParent succeeded on desktop" ); @@ -220,24 +261,24 @@ test = create_tool_window( WS_CHILD, hwndMain ); trace( "created child %p\n", test ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, (LONG_PTR)hwndMain2 ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, (LONG_PTR)hwndMain2 ); ok( ret == hwndMain, "GWL_HWNDPARENT return value %p expected %p", ret, hwndMain ); check_parents( test, hwndMain2, hwndMain2, hwndMain2, 0, hwndMain2, hwndMain2 ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, (LONG_PTR)child ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, (LONG_PTR)child ); ok( ret == hwndMain2, "GWL_HWNDPARENT return value %p expected %p", ret, hwndMain2 ); check_parents( test, child, child, child, 0, hwndMain, hwndMain ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, (LONG_PTR)desktop ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, (LONG_PTR)desktop ); ok( ret == child, "GWL_HWNDPARENT return value %p expected %p", ret, child ); check_parents( test, desktop, 0, desktop, 0, test, desktop ); /* window is now child of desktop so GWL_HWNDPARENT changes owner from now on */ - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, (LONG_PTR)child ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, (LONG_PTR)child ); ok( ret == 0, "GWL_HWNDPARENT return value %p expected 0", ret ); check_parents( test, desktop, child, desktop, child, test, desktop ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, 0 ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, 0 ); ok( ret == child, "GWL_HWNDPARENT return value %p expected %p", ret, child ); check_parents( test, desktop, 0, desktop, 0, test, desktop ); DestroyWindow( test ); @@ -246,15 +287,15 @@ test = create_tool_window( 0, 0 ); trace( "created top-level %p\n", test ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, (LONG_PTR)hwndMain2 ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, (LONG_PTR)hwndMain2 ); ok( ret == 0, "GWL_HWNDPARENT return value %p expected 0", ret ); check_parents( test, desktop, hwndMain2, 0, hwndMain2, test, test ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, (LONG_PTR)child ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, (LONG_PTR)child ); ok( ret == hwndMain2, "GWL_HWNDPARENT return value %p expected %p", ret, hwndMain2 ); check_parents( test, desktop, child, 0, child, test, test ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, 0 ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, 0 ); ok( ret == child, "GWL_HWNDPARENT return value %p expected %p", ret, child ); check_parents( test, desktop, 0, 0, 0, test, test ); DestroyWindow( test ); @@ -263,15 +304,15 @@ test = create_tool_window( WS_POPUP, 0 ); trace( "created popup %p\n", test ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, (LONG_PTR)hwndMain2 ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, (LONG_PTR)hwndMain2 ); ok( ret == 0, "GWL_HWNDPARENT return value %p expected 0", ret ); check_parents( test, desktop, hwndMain2, hwndMain2, hwndMain2, test, hwndMain2 ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, (LONG_PTR)child ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, (LONG_PTR)child ); ok( ret == hwndMain2, "GWL_HWNDPARENT return value %p expected %p", ret, hwndMain2 ); check_parents( test, desktop, child, child, child, test, hwndMain ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, 0 ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, 0 ); ok( ret == child, "GWL_HWNDPARENT return value %p expected %p", ret, child ); check_parents( test, desktop, 0, 0, 0, test, test ); DestroyWindow( test ); @@ -310,7 +351,7 @@ ok( ret == desktop, "SetParent return value %p expected %p", ret, desktop ); check_parents( test, child, child, hwndMain2, hwndMain2, hwndMain, hwndMain2 ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, (ULONG_PTR)hwndMain ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, (ULONG_PTR)hwndMain ); ok( ret == child, "GWL_HWNDPARENT return value %p expected %p", ret, child ); check_parents( test, hwndMain, hwndMain, hwndMain2, hwndMain2, hwndMain, hwndMain2 ); DestroyWindow( test ); @@ -343,7 +384,7 @@ owner = create_tool_window( WS_CHILD, hwndMain2 ); test = create_tool_window( WS_POPUP, 0 ); trace( "created owner %p and popup %p\n", owner, test ); - ret = (HWND)SetWindowLongW( test, GWL_HWNDPARENT, (ULONG_PTR)owner ); + ret = (HWND)test_set_window_long( test, GWL_HWNDPARENT, (ULONG_PTR)owner ); ok( ret == 0, "GWL_HWNDPARENT return value %p expected 0", ret ); check_parents( test, desktop, owner, owner, owner, test, hwndMain2 ); DestroyWindow( owner ); -- Francois Gouget fgouget(a)free.fr http://fgouget.free.fr/ Before you criticize someone, walk a mile in his shoes. That way, if he gets angry, he'll be a mile away - and barefoot.
On Tuesday 24 December 2002 03:23 pm, Francois Gouget wrote:
This test uses {Get,Set}WindowLongW which of course does not work on Win9x, thus causing the test to fail.
This type of scenario seems to come up a lot lately... perhaps there should be a standard way to detect windows 95 (or another platform) during a test and skip things as may be appropriate on that platform? What do you think? -gmt "It does not take a majority to prevail ... but rather an irate, tireless minority, keen on setting brushfires of freedom in the minds of men." --Samuel Adams, Patriot
On Tue, 24 Dec 2002, Greg Turner wrote:
On Tuesday 24 December 2002 03:23 pm, Francois Gouget wrote:
This test uses {Get,Set}WindowLongW which of course does not work on Win9x, thus causing the test to fail.
This type of scenario seems to come up a lot lately... perhaps there should be a standard way to detect windows 95 (or another platform) during a test and skip things as may be appropriate on that platform? What do you think?
Tests should *not* check whether they are running on Windows 95, 98 or other. Just like individual autoconf-like functionality tests are better than relying on __LINUX__ or other macros, tests should check for the existence of the functions they want to test. So the best way to handle A/W issues is to just call the function and check for failure with the ERROR_CALL_NOT_IMPLEMENTED error. If that happens then the check(s) should be skipped. Similarly, the way to deal with APIs that only exist on some platforms is to use LoadLibrary and GetProcAddress. If the resulting pointer is NULL, then skip the test(s). It may even make sense to do so in cases where the whole library may be missing (kind of depends on the impact of missing libraries on automated runs: does it stop the batch file with a messagebox?). The real issue is that up until now all tests have been written using Wine and never tested in Windows. Or if they were actually tested in Windows they were only tested with one version of Windows and one quickly finds whether that was Win9x or an NT-something. But now that it's easy to compile and run these tests on Windows this problem should pretty much disappear (or rather tests will be fixed as they come rather than accumulate). It has been some time since I last updated the zip file that contains the windows test executables as I tried to focus on actually fixing the tests. Now that some progress has been made I'll try to put something together in the comming days. -- Francois Gouget fgouget(a)free.fr http://fgouget.free.fr/ May your Tongue stick to the Roof of your Mouth with the Force of a Thousand Caramels.
On Tuesday 24 December 2002 08:23 pm, Francois Gouget wrote:
On Tue, 24 Dec 2002, Greg Turner wrote:
This type of scenario seems to come up a lot lately... perhaps there should be a standard way to detect windows 95 (or another platform) during a test and skip things as may be appropriate on that platform? What do you think?
Tests should *not* check whether they are running on Windows 95, 98 or other. Just like individual autoconf-like functionality tests are better than relying on __LINUX__ or other macros, tests should check for the existence of the functions they want to test.
Why not, it seems reasonable enough to me? I'm talking about run-time checks here -- I don't know the API's, but surely there is some Microsoft-sanctioned way to determine the OS you are running on...? And as the wine developers, surely we can decide if we are running under wine... so why is this really so bad, assuming it's implemented well, in an easy-to-re-use manner.
So the best way to handle A/W issues is to just call the function and check for failure with the ERROR_CALL_NOT_IMPLEMENTED error. If that happens then the check(s) should be skipped. Similarly, the way to deal with APIs that only exist on some platforms is to use LoadLibrary and GetProcAddress. If the resulting pointer is NULL, then skip the test(s). It may even make sense to do so in cases where the whole library may be missing (kind of depends on the impact of missing libraries on automated runs: does it stop the batch file with a messagebox?).
The real issue is that up until now all tests have been written using Wine and never tested in Windows. Or if they were actually tested in Windows they were only tested with one version of Windows and one quickly finds whether that was Win9x or an NT-something.
Yes, the concept of what is being tested seems to have broadened a bit, from "test the guts of wine" to "test the guts of wine in a windows-compatible way," which seems like a good thing to me, btw. It hasn't really been an issue, and perhaps it need never be, but, as you point out, certainly one could imagine scenarios where it's not so simple as just checking for ERROR_CALL_NOT_IMPLEMENTED: perhaps there is no such API to call on some platform, or perhaps the user needs permission to write to some resource that might depend on the user's permission on the particular machine... who knows? Testing a big system like wine which goes pretty deep into the architectural roots, and all the way up to big honkin' "assemblies" (or what-have-you's) like the java virtual machine or directx... there could be a lot of circumstances where the outcome might not be reliable but we don't want to drop the test out of the default testrun... this is precisely why it seems that being able to rule out 90% of such cases with a simple "what environment do I run under?" test seems like a win... but perhaps you have thought this through more carefully than I? Am I missing something? Merry Christmas, and thanks for all the great code and hard work, -- gmt "It does not take a majority to prevail ... but rather an irate, tireless minority, keen on setting brushfires of freedom in the minds of men." --Samuel Adams, Patriot
On Tue, 24 Dec 2002, Greg Turner wrote:
On Tuesday 24 December 2002 08:23 pm, Francois Gouget wrote:
On Tue, 24 Dec 2002, Greg Turner wrote:
This type of scenario seems to come up a lot lately... perhaps there should be a standard way to detect windows 95 (or another platform) during a test and skip things as may be appropriate on that platform? What do you think?
Tests should *not* check whether they are running on Windows 95, 98 or other. Just like individual autoconf-like functionality tests are better than relying on __LINUX__ or other macros, tests should check for the existence of the functions they want to test.
Why not, it seems reasonable enough to me? I'm talking about run-time checks here -- I don't know the API's, but surely there is some Microsoft-sanctioned way to determine the OS you are running on...? And as the wine developers, surely we can decide if we are running under wine... so why is this really so bad, assuming it's implemented well, in an easy-to-re-use manner.
Because it means that the test would have to know that API X is not available on Win95, Win98 and NT4 but that you do have it on Win98 SE, 2000, XP, Win XP 2005 and maybe even on NT4 with SP6. And for API Y it's completely different. And for API Z, it may very well depend on whether you installed IE 5 (and thus upgraded half of the system dlls), or maybe Office 2000 (which upgrades another half of the system dlls). Not to mention what would happen if you install Office 11. Etc, etc, etc. In other words, it would be unreliable and unmaintainable. [...]
Yes, the concept of what is being tested seems to have broadened a bit, from "test the guts of wine" to "test the guts of wine in a windows-compatible way," which seems like a good thing to me, btw.
I think the intention did not really change from the beginning (at least not for me) but the tests old denomination 'regression tests' was clearly a bad name and may have caused some confusion. Not that the goal is not to test Wine in a 'windows-compatible' way. It's to make sure that Wine _is_ windows compatible, i.e. that its behavior conforms to that of at least one of the Windows platforms. Hence the current 'conformance tests' name.
It hasn't really been an issue, and perhaps it need never be, but, as you point out, certainly one could imagine scenarios where it's not so simple as just checking for ERROR_CALL_NOT_IMPLEMENTED: perhaps there is no such API to call on some platform, or perhaps the user needs permission to write to some resource that might depend on the user's permission on the particular machine... who knows?
Yes some things may not always be testable. Creating users in a domain for instance. Even something as simple as 'testing sounds' cannot be tested if there is no soundcard. However I doubt these will represent any significant portion of the conformance tests. And the current sound tests show how to deal with such cases: they enumerate the soundcards and if there's none, well then no further test is performed (note that the enumeration is still tested). Merry Christmas -- Francois Gouget fgouget(a)free.fr http://fgouget.free.fr/ "Utilisateur" (nom commun) : Mot utilisé par les informaticiens en lieu et place d'"idiot".
Francois Gouget <fgouget(a)free.fr> writes:
This test uses {Get,Set}WindowLongW which of course does not work on Win9x, thus causing the test to fail. I guess part of the goal is to check that mixing Ansi calls (CreatewindowExA) with Unicode calls works.
No, there's no real reason to use the W call since we only set parent and style. We might as well replace all calls by the A version.
So I wrote a function test_set_window_long function that calls SetWindowLongW and then SetWindowLongA and checks that their results match but that causes the test to fail (at least on NT4 and Wine). More precisely even if all this function does is the following, then the test fails:
{ LONG rc=SetWindowLongA(hwnd,nIndex,dwNewLong); SetWindowLongA(hwnd,nIndex,dwNewLong); return rc; }
Comment out the second SetWindowLongA and it works. Why would calling SetWindowLong a second time make any difference?
The behavior of SetWindowLong(GWL_HWNDPARENT) depends on the current parent, so calling it a second time with the same arguments is not necessarily a nop. -- Alexandre Julliard julliard(a)winehq.com
participants (3)
-
Alexandre Julliard -
Francois Gouget -
Greg Turner