Hello, Thanks for the feedback.
On Fri, Jul 12, 2013 at 3:07 AM, Alexandre Julliard <julli...@winehq.org> wrote: > Adding sleeps to work around timing issues is usually suspicious, > particularly since the code is already supposed to wait. You'd have to > explain why that's not sufficient. Trying to explain why the sleeping in the while loop is not sufficient: The attached patch do two things: 1. - { INPUT_KEYBOARD, {{0}}, {VK_F10, 0}, FALSE, FALSE }, + { INPUT_KEYBOARD, {{0}}, {VK_F10, 0}, TRUE, FALSE }, Change the expected bMenuVisible from FALSE to TRUE in a passed test ( test 19 ) If the original test is correct, then after I change FALSE to TRUE, it should fail now. 2. Added some trace on sleeping time. The test result is here: https://testbot.winehq.org/JobDetails.pl?Key=26260 Looking at the test result, some times test 19 failed as expected, some times test 19 passed, such as https://testbot.winehq.org/JobDetails.pl?Key=26260&log_207=1#k207 --- snip --- test 19: Are we going to sleep? test 19: total sleeped time: 0 WARNING! sleeped time is zero! --- snip --- The reason that total sleeping time equal to zero is, (menu_tests[19].bMenuVisible == bMenuVisible) is true at the beginning, so the while loop is totally skipped, and we get a fake passed test result. Due to the above reason, a better while loop is as below: do { if (elapsed > 200) break; elapsed += 20; Sleep(20); } while (menu_tests[i].bMenuVisible != bMenuVisible) However, even that is not sufficient. For some complex test, (menu_tests[19].bMenuVisible == bMenuVisible) could become true after receiving the first input message but before receiving the last input message, so we get a fake passed test result as well. That is why I add a Sleep(100) after the while loop. Does my explaining make sense? I have another idea: 1. counting the amount of the received messages in the WndProc callback: case WM_KEYDOWN: case WM_KEYDOWN: case WM_SYSKEYDOWN: /* case WM_MOUSEMOVE: */ // Don't count WM_MOUSEMOVE case WM_LBUTTONDOWN: case XXX: case YYY: ... received_msg++; 2. and counting the amount of sent messages in send_key and click_menu, such like: static void send_key(WORD wVk) { .... sent_msg++; } 3. finally change the while loop to: do { sleeping; } while (sent_msg match received_msg or slept too long) Would you want me submit this version? Thanks for reviewing. -- Regards, Qian Hong - http://www.codeweavers.com
diff --git a/dlls/user32/tests/menu.c b/dlls/user32/tests/menu.c index 3e4cf93..91abfad 100644 --- a/dlls/user32/tests/menu.c +++ b/dlls/user32/tests/menu.c @@ -2046,7 +2046,7 @@ static struct menu_mouse_tests_s { { INPUT_KEYBOARD, {{0}}, {VK_MENU, 'M', 'P', 0}, TRUE, FALSE }, { INPUT_KEYBOARD, {{0}}, {'E', 0}, FALSE, FALSE }, { INPUT_KEYBOARD, {{0}}, {VK_F10, 0}, TRUE, FALSE }, - { INPUT_KEYBOARD, {{0}}, {VK_F10, 0}, FALSE, FALSE }, + { INPUT_KEYBOARD, {{0}}, {VK_F10, 0}, TRUE, FALSE }, { INPUT_MOUSE, {{1, 2}, {0}}, {0}, TRUE, TRUE }, /* test 20 */ { INPUT_MOUSE, {{1, 1}, {0}}, {0}, FALSE, FALSE }, @@ -2127,13 +2127,18 @@ static DWORD WINAPI test_menu_input_thread(LPVOID lpParameter) PostMessage( hWnd, WM_CANCELMODE, 0, 0 ); return 0; } + + printf("\ntest %d: Are we going to sleep?\n", i); while (menu_tests[i].bMenuVisible != bMenuVisible) { + printf("test %d: Yes, sleeping...\n", i); if (elapsed > 200) break; elapsed += 20; Sleep(20); } + printf("test %d: total sleeped time: %d\n", i, elapsed); + if (elapsed == 0) printf("WARNING! sleeped time is zero!\n\n"); if (!got_input) {