[PATCH 1/3] user32/tests: Sleep enough time to make sure all input message have been processed.

[Qian Hong]

Hello,

Thanks for the feedback.

On Fri, Jul 12, 2013 at 3:07 AM, Alexandre Julliard <julliard at 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
-------------- next part --------------
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)
         {