[PATCH v2 0/5] MR169: user32/tests: Cleanup some tests.
-- v2: user32/tests: Process messages while waiting in WindowFromPoint tests. user32/tests: Check for window class registration failure normally. user32/tests: Remove SetForegroundWindow success checks. user32/tests: Remove unnecessary test skipping checks. user32/tests: Fix indentation to silence a warning. https://gitlab.winehq.org/wine/wine/-/merge_requests/169
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/tests/msg.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 2705914d5e5..235724a5ff9 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -14089,9 +14089,11 @@ static void test_ShowWindow(void) ok(wp.ptMaxPosition.x == sw[i].wp_max.x && wp.ptMaxPosition.y == sw[i].wp_max.y, "expected %ld,%ld got %ld,%ld\n", sw[i].wp_max.x, sw[i].wp_max.y, wp.ptMaxPosition.x, wp.ptMaxPosition.y); -if (0) /* FIXME: Wine behaves completely different here */ - ok(EqualRect(&win_rc, &wp.rcNormalPosition), "expected %s got %s\n", - wine_dbgstr_rect(&win_rc), wine_dbgstr_rect(&wp.rcNormalPosition)); + if (0) /* FIXME: Wine behaves completely different here */ + { + ok(EqualRect(&win_rc, &wp.rcNormalPosition), "expected %s got %s\n", + wine_dbgstr_rect(&win_rc), wine_dbgstr_rect(&wp.rcNormalPosition)); + } } DestroyWindow(hwnd); flush_events(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/169
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115946 Your paranoid android. === w10pro64_ar (64 bit report) === user32: msg.c:12861: Test failed: message time not advanced: 1561a 1561a msg.c:12862: Test failed: coords not changed: (101 101) (101 101) msg.c:12879: Test failed: message time not advanced: 1561a 1561a msg.c:12880: Test failed: coords not changed: (101 101) (101 101)
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/tests/win.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 3727efb1b41..c6667cafeff 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -1453,10 +1453,6 @@ static void test_nonclient_area(HWND hwnd) GetWindowRect(hwnd, &rc_window); GetClientRect(hwnd, &rc_client); - /* avoid some cases when things go wrong */ - if (IsRectEmpty(&rc_window) || IsRectEmpty(&rc_client) || - rc_window.right > 32768 || rc_window.bottom > 32768) return; - rc = rc_client; MapWindowPoints(hwnd, 0, (LPPOINT)&rc, 2); FixedAdjustWindowRectEx(&rc, style, menu, exstyle); @@ -4314,7 +4310,7 @@ static void test_capture_4(void) WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, 0, 400, 200, NULL, NULL, hInstance, NULL); ok(hwnd != NULL, "CreateWindowEx failed with error %ld\n", GetLastError()); - if (!hwnd) return; + hmenu = CreatePopupMenu(); ret = AppendMenuA( hmenu, MF_STRING, 1, "winetest2"); @@ -4379,11 +4375,8 @@ static void test_keyboard_input(HWND hwnd) ok(GetFocus() == hwnd, "wrong focus window %p\n", GetFocus()); keybd_event(VK_SPACE, 0, 0, 0); - if (!peek_message(&msg)) - { - skip( "keybd_event didn't work, skipping keyboard test\n" ); - return; - } + ret = peek_message(&msg); + ok( ret, "message %04x available\n", msg.message); ok(msg.hwnd == hwnd && msg.message == WM_KEYDOWN, "hwnd %p message %04x\n", msg.hwnd, msg.message); ret = peek_message(&msg); ok( !ret, "message %04x available\n", msg.message); @@ -11289,12 +11282,6 @@ static void test_LockWindowUpdate(HWND parent) {parent, parent, 1, 1} }; - if (!child) - { - skip("CreateWindow failed, skipping LockWindowUpdate tests\n"); - return; - } - ShowWindow(parent, SW_SHOW); UpdateWindow(parent); flush_events(TRUE); @@ -12181,11 +12168,6 @@ static void test_destroy_quit(void) "destroy test main", WS_OVERLAPPED | WS_CAPTION, 100, 100, 100, 100, 0, 0, GetModuleHandleA(NULL), NULL); ok(destroy_data.main_wnd != NULL, "CreateWindowEx failed with error %ld\n", GetLastError()); - if (!destroy_data.main_wnd) - { - CloseHandle(destroy_data.evt); - return; - } thread1 = CreateThread(NULL, 0, destroy_thread1, 0, 0, NULL); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/169
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115947 Your paranoid android. === w10pro64_ja (64 bit report) === user32: win.c:4379: Test failed: message 0100 available win.c:4380: Test failed: hwnd 0000000000000000 message 0100 win.c:4409: Test failed: no message available win.c:4410: Test failed: hwnd 0000000000000000 message 0100 win.c:10800: Test failed: pos = 00fa00fa win.c:10804: Test failed: pos = 00fa00fa win.c:10808: Test failed: pos = 00fa00fa === w10pro64_zh_CN (64 bit report) === user32: win.c:4379: Test failed: message 0100 available win.c:4380: Test failed: hwnd 0000000000000000 message 0100 win.c:4409: Test failed: no message available win.c:4410: Test failed: hwnd 0000000000000000 message 0100 win.c:10800: Test failed: pos = 00fa00fa win.c:10804: Test failed: pos = 00fa00fa win.c:10808: Test failed: pos = 00fa00fa
From: Rémi Bernon <rbernon(a)codeweavers.com> It's not supposed to fail and skipping the tests simply creates false successful runs. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/tests/win.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index c6667cafeff..ee347fd3e70 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -3489,13 +3489,11 @@ static void check_z_order_debug(HWND hwnd, HWND next, HWND prev, HWND owner, static void test_popup_zorder(HWND hwnd_D, HWND hwnd_E, DWORD style) { HWND hwnd_A, hwnd_B, hwnd_C, hwnd_F; + BOOL ret; /* Give current thread foreground state otherwise the tests may fail. */ - if (!SetForegroundWindow(hwnd_D)) - { - skip("SetForegroundWindow not working\n"); - return; - } + ret = SetForegroundWindow(hwnd_D); + ok(ret, "SetForegroundWindow failed, error %lu\n", GetLastError()); SetWindowPos(hwnd_E, hwnd_D, 0,0,0,0, SWP_NOSIZE|SWP_NOMOVE|SWP_NOACTIVATE); @@ -3955,11 +3953,8 @@ static void test_SetForegroundWindow(HWND hwnd) check_wnd_state(hwnd, hwnd, hwnd, 0); ret = SetForegroundWindow(hwnd); - if (!ret) - { - skip( "SetForegroundWindow not working\n" ); - return; - } + ok(ret, "SetForegroundWindow failed, error %lu\n", GetLastError()); + check_wnd_state(hwnd, hwnd, hwnd, 0); SetLastError(0xdeadbeef); @@ -11599,19 +11594,15 @@ static void reset_window_state(HWND *state, int n) static void test_topmost(void) { HWND owner, hwnd, hwnd2, hwnd_child, hwnd_child2, hwnd_grandchild, state[6] = { 0 }; - BOOL is_wine = !strcmp(winetest_platform, "wine"); + BOOL ret, is_wine = !strcmp(winetest_platform, "wine"); owner = create_tool_window(WS_CAPTION | WS_SYSMENU | WS_MINIMIZEBOX | WS_MAXIMIZEBOX | WS_POPUP | WS_VISIBLE, 0); ok(owner != 0, "Failed to create owner window (%ld)\n", GetLastError()); /* Give current thread foreground state otherwise the tests may fail. */ - if (!SetForegroundWindow(owner)) - { - DestroyWindow(owner); - skip("SetForegroundWindow not working\n"); - return; - } + ret = SetForegroundWindow(owner); + ok(ret, "SetForegroundWindow failed, error %lu\n", GetLastError()); hwnd = create_tool_window(WS_CAPTION | WS_SYSMENU | WS_MINIMIZEBOX | WS_MAXIMIZEBOX | WS_POPUP | WS_VISIBLE, owner); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/169
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115948 Your paranoid android. === w864 (32 bit report) === user32: win.c:2684: Test failed: style 0x200000: expected !100 win.c:2684: Test failed: style 0x300000: expected !100 === w864 (64 bit report) === user32: win.c:2684: Test failed: style 0x200000: expected !100 win.c:2684: Test failed: style 0x300000: expected !100 === w10pro64_ja (64 bit report) === user32: win.c:4374: Test failed: message 0100 available win.c:4375: Test failed: hwnd 0000000000000000 message 0100 win.c:4404: Test failed: no message available win.c:4405: Test failed: hwnd 0000000000000000 message 0100 win.c:10795: Test failed: pos = 00fa00fa win.c:10799: Test failed: pos = 00fa00fa win.c:10803: Test failed: pos = 00fa00fa === w10pro64_zh_CN (64 bit report) === user32: win.c:4374: Test failed: message 0100 available win.c:4375: Test failed: hwnd 0000000000000000 message 0100 win.c:4404: Test failed: no message available win.c:4405: Test failed: hwnd 0000000000000000 message 0100 win.c:10795: Test failed: pos = 00fa00fa win.c:10799: Test failed: pos = 00fa00fa win.c:10803: Test failed: pos = 00fa00fa
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/tests/win.c | 48 ++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index ee347fd3e70..0f07e42ae0d 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -1161,10 +1161,11 @@ static LRESULT WINAPI tool_window_procA(HWND hwnd, UINT msg, WPARAM wparam, LPAR static const WCHAR mainclassW[] = {'M','a','i','n','W','i','n','d','o','w','C','l','a','s','s','W',0}; -static BOOL RegisterWindowClasses(void) +static void RegisterWindowClasses(void) { WNDCLASSW clsW; WNDCLASSA cls; + BOOL ret; cls.style = CS_DBLCLKS; cls.lpfnWndProc = main_window_procA; @@ -1177,7 +1178,8 @@ static BOOL RegisterWindowClasses(void) cls.lpszMenuName = NULL; cls.lpszClassName = "MainWindowClass"; - if(!RegisterClassA(&cls)) return FALSE; + ret = RegisterClassA(&cls); + ok(ret, "RegisterClassA failed, error %lu\n", GetLastError()); clsW.style = CS_DBLCLKS; clsW.lpfnWndProc = main_window_procW; @@ -1190,7 +1192,8 @@ static BOOL RegisterWindowClasses(void) clsW.lpszMenuName = NULL; clsW.lpszClassName = mainclassW; - if(!RegisterClassW(&clsW)) return FALSE; + ret = RegisterClassW(&clsW); + ok(ret, "RegisterClassW failed, error %lu\n", GetLastError()); cls.style = 0; cls.lpfnWndProc = tool_window_procA; @@ -1203,9 +1206,8 @@ static BOOL RegisterWindowClasses(void) cls.lpszMenuName = NULL; cls.lpszClassName = "ToolWindowClass"; - if(!RegisterClassA(&cls)) return FALSE; - - return TRUE; + ret = RegisterClassA(&cls); + ok(ret, "RegisterClassA failed, error %lu\n", GetLastError()); } static void verify_window_info(const char *hook, HWND hwnd, const WINDOWINFO *info) @@ -2499,9 +2501,10 @@ static LRESULT WINAPI mdi_main_wnd_procA(HWND hwnd, UINT msg, WPARAM wparam, LPA return DefFrameProcA(hwnd, mdi_client, msg, wparam, lparam); } -static BOOL mdi_RegisterWindowClasses(void) +static void mdi_RegisterWindowClasses(void) { WNDCLASSA cls; + BOOL ret; cls.style = 0; cls.lpfnWndProc = mdi_main_wnd_procA; @@ -2513,17 +2516,18 @@ static BOOL mdi_RegisterWindowClasses(void) cls.hbrBackground = GetStockObject(WHITE_BRUSH); cls.lpszMenuName = NULL; cls.lpszClassName = "MDI_parent_Class"; - if(!RegisterClassA(&cls)) return FALSE; + ret = RegisterClassA(&cls); + ok(ret, "RegisterClassA failed, error %lu\n", GetLastError()); cls.lpfnWndProc = mdi_child_wnd_proc_1; cls.lpszClassName = "MDI_child_Class_1"; - if(!RegisterClassA(&cls)) return FALSE; + ret = RegisterClassA(&cls); + ok(ret, "RegisterClassA failed, error %lu\n", GetLastError()); cls.lpfnWndProc = mdi_child_wnd_proc_2; cls.lpszClassName = "MDI_child_Class_2"; - if(!RegisterClassA(&cls)) return FALSE; - - return TRUE; + ret = RegisterClassA(&cls); + ok(ret, "RegisterClassA failed, error %lu\n", GetLastError()); } static void test_mdi(void) @@ -2536,7 +2540,7 @@ static void test_mdi(void) MSG msg; HMENU frame_menu, child_menu; - if (!mdi_RegisterWindowClasses()) assert(0); + mdi_RegisterWindowClasses(); mdi_hwndMain = CreateWindowExA(0, "MDI_parent_Class", "MDI parent window", WS_CAPTION | WS_SYSMENU | WS_MINIMIZEBOX | @@ -5994,9 +5998,10 @@ static void test_AWRwindow(LPCSTR class, LONG style, LONG exStyle, BOOL menu) DestroyWindow(hwnd); } -static BOOL AWR_init(void) +static void AWR_init(void) { WNDCLASSA class; + BOOL ret; class.style = CS_HREDRAW | CS_VREDRAW; class.lpfnWndProc = DefWindowProcA; @@ -6009,18 +6014,12 @@ static BOOL AWR_init(void) class.lpszMenuName = 0; class.lpszClassName = szAWRClass; - if (!RegisterClassA(&class)) { - ok(FALSE, "RegisterClass failed\n"); - return FALSE; - } + ret = RegisterClassA(&class); + ok(ret, "RegisterClassA failed, error %lu\n", GetLastError()); hmenu = CreateMenu(); - if (!hmenu) - return FALSE; ok(hmenu != 0, "Failed to create menu\n"); ok(AppendMenuA(hmenu, MF_STRING, 1, "Test!"), "Failed to create menu item\n"); - - return TRUE; } @@ -6087,8 +6086,7 @@ static void test_AWR_flags(void) static void test_AdjustWindowRect(void) { - if (!AWR_init()) - return; + AWR_init(); SHOWSYSMETRIC(SM_CYCAPTION); SHOWSYSMETRIC(SM_CYSMCAPTION); @@ -13017,7 +13015,7 @@ START_TEST(win) return; } - if (!RegisterWindowClasses()) assert(0); + RegisterWindowClasses(); hwndMain = CreateWindowExA(/*WS_EX_TOOLWINDOW*/ 0, "MainWindowClass", "Main window", WS_CAPTION | WS_SYSMENU | WS_MINIMIZEBOX | -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/169
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115949 Your paranoid android. === w10pro64_en_AE_u8 (64 bit report) === user32: win.c:4551: Test failed: hwnd 000000000002024C/000000000002024C message 0200 win.c:4556: Test failed: hwnd 000000000002024C/000000000002024C message 0203 win.c:4560: Test failed: message 0202 available === w10pro64_ja (64 bit report) === user32: win.c:4378: Test failed: message 0100 available win.c:4379: Test failed: hwnd 0000000000000000 message 0100 win.c:4408: Test failed: no message available win.c:4409: Test failed: hwnd 0000000000000000 message 0100 win.c:10793: Test failed: pos = 00fa00fa win.c:10797: Test failed: pos = 00fa00fa win.c:10801: Test failed: pos = 00fa00fa === w10pro64_zh_CN (64 bit report) === user32: win.c:4378: Test failed: message 0100 available win.c:4379: Test failed: hwnd 0000000000000000 message 0100 win.c:4408: Test failed: no message available win.c:4409: Test failed: hwnd 0000000000000000 message 0100 win.c:10793: Test failed: pos = 00fa00fa win.c:10797: Test failed: pos = 00fa00fa win.c:10801: Test failed: pos = 00fa00fa
From: Rémi Bernon <rbernon(a)codeweavers.com> The test is randomly failing, processing messages while waiting for events and cleaning up child windows in the other process should fix the issue. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/tests/win.c | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 0f07e42ae0d..2d9ff7cbc45 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -10211,21 +10211,29 @@ static void window_from_point_proc(HWND parent) flush_events(TRUE); SetEvent(start_event); - got_hittest = FALSE; got_click = FALSE; - while(!got_click && wait_for_message(&msg)) { - if(msg.message == WM_LBUTTONUP) { - ok(msg.hwnd == win, "msg.hwnd = %p, expected %p\n", msg.hwnd, win); - got_click = TRUE; + got_hittest = FALSE; + while ((ret = MsgWaitForMultipleObjects( 1, &end_event, FALSE, INFINITE, QS_ALLINPUT )) <= 1) + { + while (PeekMessageA( &msg, 0, 0, 0, PM_REMOVE )) + { + if (msg.message == WM_LBUTTONUP) + { + ok(msg.hwnd == win, "msg.hwnd = %p, expected %p\n", msg.hwnd, win); + got_click = TRUE; + } + TranslateMessage( &msg ); + DispatchMessageA( &msg ); } - DispatchMessageA(&msg); + if (ret == 0) break; } + ok(ret == 0, "MsgWaitForMultipleObjects returned %#lx\n", ret); ok(got_hittest, "transparent window didn't get WM_NCHITTEST message\n"); ok(got_click, "button under static window didn't get WM_LBUTTONUP\n"); - ret = WaitForSingleObject(end_event, 5000); - ok(ret == WAIT_OBJECT_0, "WaitForSingleObject returned %lx\n", ret); - + DestroyWindow(win); + DestroyWindow(child_button); + DestroyWindow(child_static); CloseHandle(start_event); CloseHandle(end_event); } @@ -10245,14 +10253,10 @@ static void test_window_from_point(const char *argv0) pt.x = pt.y = 150; win = WindowFromPoint(pt); + ok(win == hwnd, "WindowFromPoint returned %p, expected %p\n", win, hwnd); pt.x = 250; - if(win == hwnd) - win = WindowFromPoint(pt); - if(win != hwnd) { - skip("there's another window covering test window\n"); - DestroyWindow(hwnd); - return; - } + win = WindowFromPoint(pt); + ok(win == hwnd, "WindowFromPoint returned %p, expected %p\n", win, hwnd); child = CreateWindowExA(0, "static", "static", WS_CHILD | WS_VISIBLE, 0, 0, 100, 100, hwnd, 0, NULL, NULL); @@ -10279,7 +10283,7 @@ static void test_window_from_point(const char *argv0) startup.cb = sizeof(startup); ok(CreateProcessA(NULL, cmd, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info), "CreateProcess failed.\n"); - ok(wait_for_events(1, &start_event, 1000) == 0, "didn't get start_event\n"); + wait_for_events(1, &start_event, INFINITE); child = GetWindow(hwnd, GW_CHILD); win = WindowFromPoint(pt); @@ -10294,6 +10298,8 @@ static void test_window_from_point(const char *argv0) ok(win == child, "WindowFromPoint returned %p, expected %p\n", win, child); SetEvent(end_event); + wait_for_events(1, &info.hProcess, INFINITE); + wait_child_process(info.hProcess); CloseHandle(start_event); CloseHandle(end_event); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/169
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115950 Your paranoid android. === w864 (32 bit report) === user32: win.c:2688: Test failed: style 0x200000: expected !100 win.c:2688: Test failed: style 0x300000: expected !100 === w864 (64 bit report) === user32: win.c:2688: Test failed: style 0x200000: expected !100 win.c:2688: Test failed: style 0x300000: expected !100 === w10pro64_ja (64 bit report) === user32: win.c:4378: Test failed: message 0100 available win.c:4379: Test failed: hwnd 0000000000000000 message 0100 win.c:4408: Test failed: no message available win.c:4409: Test failed: hwnd 0000000000000000 message 0100 win.c:10799: Test failed: pos = 00fa00fa win.c:10803: Test failed: pos = 00fa00fa win.c:10807: Test failed: pos = 00fa00fa === w10pro64_zh_CN (64 bit report) === user32: win.c:4378: Test failed: message 0100 available win.c:4379: Test failed: hwnd 0000000000000000 message 0100 win.c:4408: Test failed: no message available win.c:4409: Test failed: hwnd 0000000000000000 message 0100 win.c:10799: Test failed: pos = 00fa00fa win.c:10803: Test failed: pos = 00fa00fa win.c:10807: Test failed: pos = 00fa00fa
v2: Check `SetForegroundWindow` return values. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/169#note_1469
Afaiu SetForegroundWindow can fail on Windows if the terminal that is running the tests wasn't the foreground window when the test was started. I.e. it is some sort of focus stealing prevention. In the d3d tests we have a few similar tests. Those are mostly motivated by Linux WMs refusing to give us focus, not so much on Windows. There is also the case of fvwm2 or other WMs with a focus-follows-mouse setting that will make SetForegroundWindow not do what you want it to do. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/169#note_1470
On Wed Jun 1 18:31:09 2022 +0000, Stefan Dösinger wrote:
Afaiu SetForegroundWindow can fail on Windows if the terminal that is running the tests wasn't the foreground window when the test was started. I.e. it is some sort of focus stealing prevention. In the d3d tests we have a few similar tests. Those are mostly motivated by Linux WMs refusing to give us focus, not so much on Windows. There is also the case of fvwm2 or other WMs with a focus-follows-mouse setting that will make SetForegroundWindow not do what you want it to do. Sure, `SetForegroundWindow` doesn't necessarily succeeds on windows, but in the case of the tests and how they are run on windows, it is not supposed to fail.
If it fails it means the tests will fail anyway. There have been cases like this in testbot runs, for instance when there's an unexpected topmost window (for instance the firewall warning), and it should be fixed by fixing the environment, not by skipping tests. On Linux there's some other issues, but they are mitigated by the `flush_events` calls which makes sure X11 events stabilize in the state we expect windows to be. It is not ideal, and Wine `user32` and `winex11` are still racy w.r.t focus and foreground, but that's something that also need to be fixed, not ignored. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/169#note_1471
On Wed Jun 1 18:31:09 2022 +0000, Rémi Bernon wrote:
Sure, `SetForegroundWindow` doesn't necessarily succeeds on windows, but in the case of the tests and how they are run on windows, it is not supposed to fail. If it fails it means the tests will fail anyway. There have been cases like this in testbot runs, for instance when there's an unexpected topmost window (for instance the firewall warning), and it should be fixed by fixing the environment, not by skipping tests. On Linux there's some other issues, but they are mitigated by the `flush_events` calls which makes sure X11 events stabilize in the state we expect windows to be. It is not ideal, and Wine `user32` and `winex11` are still racy w.r.t focus and foreground, but that's something that also need to be fixed, not ignored.
it should be fixed by fixing the environment, not by skipping tests.
I'm not sure I agree. The testbot isn't the only environment where people possibly run the tests, and someone new to the entire user32 thing might find environment-induced test failures a bug and not a feature. That said, it would be sensible to apply different rules to d3d and user32. In d3d, handling focus loss/gain is only a relatively small part, whereas it is pretty central to user32. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/169#note_1472
On Wed Jun 1 18:36:38 2022 +0000, Stefan Dösinger wrote:
it should be fixed by fixing the environment, not by skipping tests.
I'm not sure I agree. The testbot isn't the only environment where people possibly run the tests, and someone new to the entire user32 thing might find environment-induced test failures a bug and not a feature. That said, it would be sensible to apply different rules to d3d and user32. In d3d, handling focus loss/gain is only a relatively small part, whereas it is pretty central to user32. I honestly don't think you can expect tests to do anything meaningful if you are doing something else at the same time in the environment they run in. Maybe that can work for a few very well isolated things, but not for `user32`, and definitely not for `user32:win` / `user32:msg`.
Even if we consider it a feature we can only at best skip tests, and the unaware user would get a seemingly successful test result, and be happy with it when most tests would have been skipped. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/169#note_1473
On 6/1/22 13:43, Rémi Bernon (@rbernon) wrote:
On Wed Jun 1 18:36:38 2022 +0000, Stefan Dösinger wrote:
it should be fixed by fixing the environment, not by skipping tests.
I'm not sure I agree. The testbot isn't the only environment where people possibly run the tests, and someone new to the entire user32 thing might find environment-induced test failures a bug and not a feature. That said, it would be sensible to apply different rules to d3d and user32. In d3d, handling focus loss/gain is only a relatively small part, whereas it is pretty central to user32. I honestly don't think you can expect tests to do anything meaningful if you are doing something else at the same time in the environment they run in. Maybe that can work for a few very well isolated things, but not for `user32`, and definitely not for `user32:win` / `user32:msg`.
Even if we consider it a feature we can only at best skip tests, and the unaware user would get a seemingly successful test result, and be happy with it when most tests would have been skipped.
Is there a chance we can sidestep the issue by using a separate desktop or window station?
Or maybe rephrase the test failure message emphasising a possible environment problem or adding a comment that this particular check might fail if the window manager is overly aggressive with focus prevention. Anyway, I'll sign out of the discussion now. Since I'm not working on user32 much it is your choice. I just know I have a d3d focus test-that-can't-fail that's failing on AJ's machine that I need to fix :-( ... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/169#note_1475
participants (4)
-
Marvin -
Rémi Bernon -
Stefan Dösinger
-
Zebediah Figura