Thanks again for testing this for me, Paul. I have modified the tests so that they should pass and I've skipped tests on win9x that I suspect are causing the blue screens (personally, I doubt it's due to VMware).
I moved the new test functions to the top of the file, but I didn't remove the comments. I can shorten them if preferred, but I personally prefer to have functions well documented. So if you or anybody else could test this again for me, I would greatly appreciate it. Thanks! Daniel --- On Fri, 7/3/09, Paul Vriens <[email protected]> wrote: > From: Paul Vriens <[email protected]> > Subject: Re: user32: Windows test request (cursors/icons) > To: [email protected] > Cc: [email protected] > Date: Friday, July 3, 2009, 3:44 PM > Daniel Santos wrote: > > I don't have access to a working windows machine right > now and would appreciate if somebody can run these tests > against XP, vista and/or 9x. > > > > Thanks! > > Daniel > > > Hi Daniel, > > Tested on XP Professional SP3: > > cursoricon.c:1367: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:1369: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:1371: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:184: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon: 6 tests executed (0 marked as todo, 0 > failures), 0 skipped. > cursoricon: 553 tests executed (0 marked as todo, 4 > failures), 0 skipped. > > NT4 Server SP6a: > > cursoricon.c:1363: Test failed: DestroyCursor returned 1, > expected FALSE. > cursoricon.c:1363: Test failed: Last error is 3735928559 > (0xdeadbeef), expected 1402 (0x57a). > cursoricon.c:1367: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:1369: Test failed: DestroyIcon returned 1, > expected FALSE. > cursoricon.c:1369: Test failed: Last error is 3735928559 > (0xdeadbeef), expected 1414 (0x586). > cursoricon.c:1371: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:184: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon: 6 tests executed (0 marked as todo, 0 > failures), 0 skipped. > cursoricon: 553 tests executed (0 marked as todo, 7 > failures), 0 skipped. > > W2K Professional SP4: > > cursoricon.c:1367: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:1369: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:1371: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:184: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon: 6 tests executed (0 marked as todo, 0 > failures), 0 skipped. > cursoricon: 553 tests executed (0 marked as todo, 4 > failures), 0 skipped. > > W2K3 SE SP2: > > cursoricon.c:1367: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:1369: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:1371: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:184: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon: 6 tests executed (0 marked as todo, 0 > failures), 0 skipped. > cursoricon: 554 tests executed (0 marked as todo, 4 > failures), 0 skipped. > > Vista Ultimate - SP2: > > cursoricon.c:1367: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:1369: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:1371: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon.c:184: Test failed: Last error is 1402 (0x57a), > expected 1414 (0x586). > cursoricon: 11 tests executed (0 marked as todo, 0 > failures), 0 skipped. > cursoricon: 553 tests executed (0 marked as todo, 4 > failures), 0 skipped. > > I get a blue screen on Win95 and Win98 (VMware issue?) !! > > Some remarks on your patch: > > - Please move the new functions to the top so you don't > have to forward declare them. > - The level of comments for the 2 added functions is > something we usually expect for implementations, not for > tests ;) > > -- Cheers, > > Paul. >
From 8ec917a8657667f1d61fbd92b70b54b7153a065f Mon Sep 17 00:00:00 2001 From: Daniel Santos <[email protected]> Date: Sun, 5 Jul 2009 15:52:36 -0500 Subject: user32: Add more tests for SetCursor & DestroyCursor --- dlls/user32/tests/cursoricon.c | 270 +++++++++++++++++++++++++++++++++------- 1 files changed, 223 insertions(+), 47 deletions(-) diff --git a/dlls/user32/tests/cursoricon.c b/dlls/user32/tests/cursoricon.c index 3f8bd82..0487f46 100644 --- a/dlls/user32/tests/cursoricon.c +++ b/dlls/user32/tests/cursoricon.c @@ -18,6 +18,9 @@ * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, write to the Free Software * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + * + * FIXME: Add tests for CreateCursor and verify that width & hight cannot exceed + * SM_CXCURSOR & SM_CYCURSOR (because they can) */ #include <assert.h> @@ -64,6 +67,133 @@ static HANDLE child_process; #define PROC_INIT (WM_USER+1) + +/************************************************************************* + * test_SetCursor + * + * Encapsulates tests to SetCursor() function. + * + * PARAMS + * target [I] A handle to use when calling SetCursor + * shouldFail [I] Rather of not it should fail + * line [I] The line this function was called from. + * retValTodo [I] True if return value is broken in wine (wine's return value + * is broken in some, but not all cases) + * + * RETURNS + * The return value of the SetCursor() call. + */ +static HCURSOR test_SetCursor(HANDLE hNewCursor, BOOL shouldFail, int line, + BOOL retValTodo) +{ + HCURSOR cursor1, cursor2, cursor3; + DWORD error; + + /* GetCursor should have no errors */ + SetLastError(0xdeadbeef); + cursor1 = GetCursor(); + error = GetLastError(); + ok_(__FILE__, line)(error == 0xdeadbeef, + "GetCursor() changed last error: %u.\n", + error); + + /* Call SetCursor on supplied handle */ + SetLastError(0xdeadbeef); + cursor2 = SetCursor(hNewCursor); + error = GetLastError(); + cursor3 = GetCursor(); + + if (shouldFail) { + todo_wine ok_(__FILE__, line)(error == ERROR_INVALID_CURSOR_HANDLE, + "Last error is %u (0x%x), expected " + "ERROR_INVALID_CURSOR_HANDLE (%u).\n", + error, error, ERROR_INVALID_CURSOR_HANDLE); + + if(retValTodo) { + todo_wine ok_(__FILE__, line)(!cursor2, + "SetCursor() returned non-zero (%p).\n", + cursor2); + } else { + ok_(__FILE__, line)(!cursor2, + "SetCursor() returned non-zero (%p).\n", + cursor2); + } + + todo_wine ok_(__FILE__, line)(cursor1 == cursor3, + "SetCursor() changed cursor.\n"); + } else { + /* WinXP: For some reason, SetCursor() is returning NULL when hNewCursor + * is NULL. Is it this way on other versions? MSDN says it should + * return the old cursor. + */ + if(hNewCursor) { + ok_(__FILE__, line)(cursor1 == cursor2, + "SetCursor() did not return the previous cursor, " + "expected %p, got %p.\n", + cursor1, cursor2); + } else { + todo_wine ok_(__FILE__, line)(!cursor2, + "SetCursor() returned %p, expected NULL.\n", + cursor2); + } + + ok_(__FILE__, line)(cursor3 == hNewCursor, + "GetCursor() did not return the value we previously passed to " + "SetCursor(). Expected %p, got %p.\n", + hNewCursor, cursor3); + } + + return cursor2; +} + + +/************************************************************************* + * test_DestroyCursorIcon + * + * Encapsulates simple tests to DestroyCursor and DestroyIcon functions. + * + * PARAMS + * h [I] A handle to use when calling DestroyIcon/DestroyCursor + * isCursor [I] TRUE for DestroyCursor, FALSE for DestroyIcon + * expectedRet [I] The expected return value + * line [I] The line this function was called from. + * expectedError [I] The expected last error + * + * RETURNS + * The return value of the function call. + * + * NOTES + * Only use on very simple test cases where behavior is consistenit accross + * windows versions and there are no TODOs. + */ +static BOOL test_DestroyCursorIcon(HANDLE h, BOOL isCursor, BOOL expectedRet, + int line, DWORD expectedError) +{ + BOOL ret; + DWORD error; + const char* funcName = isCursor ? "DestroyCursor" : "DestroyIcon"; + + /* crashes on win9x */ + if ((GetVersion() & 0x80000000) && ( + h == 0 || h == (HANDLE)1 || h == INVALID_HANDLE_VALUE)) + return FALSE; + + SetLastError(0xdeadbeef); + ret = isCursor ? DestroyCursor(h) : DestroyIcon(h); + error = GetLastError(); + + /* broken on NT4 Server SP6a when handle == 1 */ + ok_(__FILE__, line)(ret == expectedRet || broken(h == (HANDLE)1), + "%s returned %d, expected %s.\n", + funcName, ret, expectedRet ? "TRUE" : "FALSE"); + ok_(__FILE__, line)(error == expectedError || broken(h == (HANDLE)1), + "Last error is %u (0x%x), expected %u (0x%x).\n", + error, error, expectedError, expectedError); + + return ret; +} + + static LRESULT CALLBACK callback_child(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) { BOOL ret; @@ -72,14 +202,15 @@ static LRESULT CALLBACK callback_child(HWND hwnd, UINT msg, WPARAM wParam, LPARA switch (msg) { /* Destroy the cursor. */ - case WM_USER+1: + case WM_USER + 1: SetLastError(0xdeadbeef); ret = DestroyCursor((HCURSOR) lParam); error = GetLastError(); - todo_wine ok(!ret || broken(ret) /* win9x */, "DestroyCursor on the active cursor succeeded.\n"); + todo_wine ok(!ret || broken(ret), /* win9x */ + "DestroyCursor on the active cursor of another process succeeded.\n"); ok(error == ERROR_DESTROY_OBJECT_OF_OTHER_THREAD || - error == 0xdeadbeef, /* vista */ - "Last error: %u\n", error); + error == 0xdeadbeef, /* vista */ + "Last error: %u\n", error); return TRUE; case WM_DESTROY: PostQuitMessage(0); @@ -169,6 +300,14 @@ static void do_parent(void) 0, 0, 200, 200, 0, 0, 0, NULL); ok(parent != 0, "CreateWindowA failed. Error: %u\n", GetLastError()); + /* make sure Destroy{Cursor,Icon} wont accept other user handle types */ + todo_wine { + test_DestroyCursorIcon(parent, TRUE, FALSE, __LINE__, + ERROR_INVALID_CURSOR_HANDLE); + test_DestroyCursorIcon(parent, FALSE, FALSE, __LINE__, + ERROR_INVALID_CURSOR_HANDLE); + } + /* Start child process. */ memset(&startup, 0, sizeof(startup)); startup.cb = sizeof(startup); @@ -217,7 +356,7 @@ static void test_child_process(void) cursor = CreateIconIndirect(&cursorInfo); ok(cursor != NULL, "CreateIconIndirect returned %p.\n", cursor); - SetCursor(cursor); + test_SetCursor(cursor, FALSE, __LINE__, FALSE); /* Destroy the cursor. */ SendMessage(child, WM_USER+1, 0, (LPARAM) cursor); @@ -520,12 +659,12 @@ static void test_CreateIcon(void) hIcon = CreateIcon(0, 16, 16, 1, 1, bmp_bits, bmp_bits); ok(hIcon != 0, "CreateIcon failed\n"); test_icon_info(hIcon, 16, 16, 1); - DestroyIcon(hIcon); + test_DestroyCursorIcon(hIcon, FALSE, TRUE, __LINE__, 0xdeadbeef); hIcon = CreateIcon(0, 16, 16, 1, display_bpp, bmp_bits, bmp_bits); ok(hIcon != 0, "CreateIcon failed\n"); test_icon_info(hIcon, 16, 16, display_bpp); - DestroyIcon(hIcon); + test_DestroyCursorIcon(hIcon, FALSE, TRUE, __LINE__, 0xdeadbeef); hbmMask = CreateBitmap(16, 16, 1, 1, bmp_bits); ok(hbmMask != 0, "CreateBitmap failed\n"); @@ -560,7 +699,7 @@ static void test_CreateIcon(void) hIcon = CreateIconIndirect(&info); ok(hIcon != 0, "CreateIconIndirect failed\n"); test_icon_info(hIcon, 16, 16, display_bpp); - DestroyIcon(hIcon); + test_DestroyCursorIcon(hIcon, FALSE, TRUE, __LINE__, 0xdeadbeef); DeleteObject(hbmMask); DeleteObject(hbmColor); @@ -577,7 +716,7 @@ static void test_CreateIcon(void) hIcon = CreateIconIndirect(&info); ok(hIcon != 0, "CreateIconIndirect failed\n"); test_icon_info(hIcon, 16, 16, 1); - DestroyIcon(hIcon); + test_DestroyCursorIcon(hIcon, FALSE, TRUE, __LINE__, 0xdeadbeef); DeleteObject(hbmMask); DeleteObject(hbmColor); @@ -610,7 +749,7 @@ static void test_CreateIcon(void) hIcon = CreateIconIndirect(&info); ok(hIcon != 0, "CreateIconIndirect failed\n"); test_icon_info(hIcon, 32, 32, 8); - DestroyIcon(hIcon); + test_DestroyCursorIcon(hIcon, FALSE, TRUE, __LINE__, 0xdeadbeef); DeleteObject(hbmColor); bmpinfo->bmiHeader.biBitCount = 16; @@ -628,7 +767,7 @@ static void test_CreateIcon(void) hIcon = CreateIconIndirect(&info); ok(hIcon != 0, "CreateIconIndirect failed\n"); test_icon_info(hIcon, 32, 32, 8); - DestroyIcon(hIcon); + test_DestroyCursorIcon(hIcon, FALSE, TRUE, __LINE__, 0xdeadbeef); DeleteObject(hbmColor); bmpinfo->bmiHeader.biBitCount = 32; @@ -646,7 +785,7 @@ static void test_CreateIcon(void) hIcon = CreateIconIndirect(&info); ok(hIcon != 0, "CreateIconIndirect failed\n"); test_icon_info(hIcon, 32, 32, 8); - DestroyIcon(hIcon); + test_DestroyCursorIcon(hIcon, FALSE, TRUE, __LINE__, 0xdeadbeef); DeleteObject(hbmMask); DeleteObject(hbmColor); @@ -740,7 +879,8 @@ static void test_LoadImageFile(const unsigned char * image_data, broken(error == 0xdeadbeef) || /* Win9x */ broken(error == ERROR_BAD_PATHNAME), /* Win98, WinMe */ "Last error: %u\n", error); - if (handle != NULL) DestroyCursor(handle); + if (handle != NULL) + test_DestroyCursorIcon(handle, TRUE, TRUE, __LINE__, 0xdeadbeef); /* Load as icon. For all tested formats, this should fail */ SetLastError(0xdeadbeef); @@ -751,7 +891,8 @@ static void test_LoadImageFile(const unsigned char * image_data, broken(error == 0xdeadbeef) || /* Win9x */ broken(error == ERROR_BAD_PATHNAME), /* Win98, WinMe */ "Last error: %u\n", error); - if (handle != NULL) DestroyIcon(handle); + if (handle != NULL) + test_DestroyCursorIcon(handle, FALSE, TRUE, __LINE__, 0xdeadbeef); /* Load as bitmap. Should succeed if bmp, fail for everything else */ SetLastError(0xdeadbeef); @@ -771,6 +912,7 @@ static void test_LoadImageFile(const unsigned char * image_data, static void test_LoadImage(void) { HANDLE handle; + HCURSOR cursor; BOOL ret; DWORD error, bytes_written; CURSORICONFILEDIR *icon_data; @@ -845,16 +987,22 @@ static void test_LoadImage(void) ok(icon_info.hbmMask != NULL, "No hbmMask!\n"); } + /* Test SetCursor with this icon */ + cursor = test_SetCursor(handle, FALSE, __LINE__, TRUE); + + /* Now change it back to the previous cursor */ + test_SetCursor(cursor, FALSE, __LINE__, FALSE); + /* Clean up. */ - SetLastError(0xdeadbeef); - ret = DestroyCursor(handle); - ok(ret, "DestroyCursor() failed.\n"); - error = GetLastError(); - ok(error == 0xdeadbeef, "Last error: %u\n", error); + ret = test_DestroyCursorIcon(handle, TRUE, TRUE, __LINE__, 0xdeadbeef); HeapFree(GetProcessHeap(), 0, icon_data); DeleteFileA("icon.ico"); + /* Test passing invalid handles to SetCursor. */ + test_SetCursor(handle, TRUE, __LINE__, FALSE); + test_SetCursor(GetCurrentThread(), TRUE, __LINE__, TRUE); + test_LoadImageFile(bmpimage, sizeof(bmpimage), "bmp", 1); test_LoadImageFile(gifimage, sizeof(gifimage), "gif", 0); test_LoadImageFile(gif4pixel, sizeof(gif4pixel), "gif", 0); @@ -917,11 +1065,7 @@ static void test_CreateIconFromResource(void) } /* Clean up. */ - SetLastError(0xdeadbeef); - ret = DestroyCursor(handle); - ok(ret, "DestroyCursor() failed.\n"); - error = GetLastError(); - ok(error == 0xdeadbeef, "Last error: %u\n", error); + ret = test_DestroyCursorIcon(handle, TRUE, TRUE, __LINE__, 0xdeadbeef); /* Test creating an icon. */ SetLastError(0xdeadbeef); @@ -947,11 +1091,7 @@ static void test_CreateIconFromResource(void) } /* Clean up. */ - SetLastError(0xdeadbeef); - ret = DestroyCursor(handle); - ok(ret, "DestroyCursor() failed.\n"); - error = GetLastError(); - ok(error == 0xdeadbeef, "Last error: %u\n", error); + ret = test_DestroyCursorIcon(handle, TRUE, TRUE, __LINE__, 0xdeadbeef); HeapFree(GetProcessHeap(), 0, hotspot); } @@ -1222,6 +1362,22 @@ static void test_DestroyCursor(void) UINT display_bpp; HDC hdc; + /* Try to destory a bunch of crap */ + todo_wine { + test_DestroyCursorIcon(0, TRUE, FALSE, __LINE__, + ERROR_INVALID_CURSOR_HANDLE); + test_DestroyCursorIcon((HANDLE)1, TRUE, FALSE, __LINE__, + ERROR_INVALID_CURSOR_HANDLE); + test_DestroyCursorIcon(INVALID_HANDLE_VALUE, TRUE, FALSE, __LINE__, + ERROR_INVALID_CURSOR_HANDLE); + test_DestroyCursorIcon(0, FALSE, FALSE, __LINE__, + ERROR_INVALID_CURSOR_HANDLE); + test_DestroyCursorIcon((HANDLE)1, FALSE, FALSE, __LINE__, + ERROR_INVALID_CURSOR_HANDLE); + test_DestroyCursorIcon(INVALID_HANDLE_VALUE, FALSE, FALSE, __LINE__, + ERROR_INVALID_CURSOR_HANDLE); + } + hdc = GetDC(0); display_bpp = GetDeviceCaps(hdc, BITSPIXEL); ReleaseDC(0, hdc); @@ -1237,29 +1393,53 @@ static void test_DestroyCursor(void) if(!cursor) { return; } - SetCursor(cursor); + test_SetCursor(cursor, FALSE, __LINE__, FALSE); + + /* When you attempt to destroy the current cursor, windows obliges you. + * On win9x, it returns TRUE (indicating success) and on NT it returns + * FALSE, but neither sets the last error. + */ SetLastError(0xdeadbeef); ret = DestroyCursor(cursor); - ok(!ret || broken(ret) /* succeeds on win9x */, "DestroyCursor on the active cursor succeeded\n"); error = GetLastError(); - ok(error == 0xdeadbeef, "Last error: %u\n", error); + ok(error == 0xdeadbeef, "Last error: 0x%x.\n", error); + ok(!ret || broken(ret) /* succeeds on win9x */, + "DestroyCursor on the active cursor returned TRUE.\n"); + + /* QUESTION: This if statment effectively skips these tests on win9x, do we + * want to do that? */ if (!ret) { cursor2 = GetCursor(); ok(cursor2 == cursor, "Active was set to %p when trying to destroy it\n", cursor2); - SetCursor(NULL); - /* Trying to destroy the cursor properly fails now with - * ERROR_INVALID_CURSOR_HANDLE. This happens because we called - * DestroyCursor() 2+ times after calling SetCursor(). The calls to - * GetCursor() and SetCursor(NULL) in between make no difference. */ + /* Make sure showing/hiding the invalid cursor still works. */ + SetLastError(0xdeadbeef); + ok(ShowCursor(TRUE) == 1, "Cursor count != 1\n"); + ok(GetLastError() == 0xdeadbeef, "Last error: 0x%x.\n", GetLastError()); + + SetLastError(0xdeadbeef); + ok(ShowCursor(FALSE) == 0, "Cursor count != 0\n"); + ok(GetLastError() == 0xdeadbeef, "Last error: 0x%x.\n", GetLastError()); + test_SetCursor(NULL, FALSE, __LINE__, FALSE); + + /* Trying to destroy the cursor again fails now with + * ERROR_INVALID_CURSOR_HANDLE. This happens because we've previously + * called DestroyCursor() on this handle. The calls to GetCursor() + * and SetCursor(NULL) in between make no difference. It would appear + * that windows doesn't mind having an invalid cursor handle as its + * current cursor much the way a comatose person doesn't mind a + * stranger in their hospital room. + */ + SetLastError(0xdeadbeef); ret = DestroyCursor(cursor); todo_wine { ok(!ret, "DestroyCursor succeeded.\n"); error = GetLastError(); - ok(error == ERROR_INVALID_CURSOR_HANDLE || error == 0xdeadbeef, /* vista */ - "Last error: 0x%08x\n", error); + ok(error == ERROR_INVALID_CURSOR_HANDLE + || broken(error == 0xdeadbeef), /* vista */ + "Last error: 0x%08x\n", error); } } @@ -1271,17 +1451,13 @@ static void test_DestroyCursor(void) SetLastError(0xdeadbeef); ret = DestroyCursor(cursor); - ok(ret || broken(!ret) /* fails on win9x */, "DestroyCursor on the active cursor failed.\n"); error = GetLastError(); - ok(error == 0xdeadbeef, "Last error: 0x%08x\n", error); + ok(error == 0xdeadbeef, "Last error: 0x%x.\n", error); + ok(ret || broken(!ret) /* fails on win9x */, + "DestroyCursor on OEM cursor failed.\n"); /* Try setting the cursor to a destroyed OEM cursor. */ - SetLastError(0xdeadbeef); - SetCursor(cursor); - error = GetLastError(); - todo_wine { - ok(error == 0xdeadbeef, "Last error: 0x%08x\n", error); - } + test_SetCursor(cursor, FALSE, __LINE__, FALSE); /* Check if LoadCursor() returns the same handle with the same icon. */ cursor2 = LoadCursor(NULL, IDC_ARROW); -- 1.6.3.3
