Re: [PATCH 1/3] user32/tests: Sleep enough time to make sure all input message have been processed.
On Mon, Jul 15, 2013 at 7:02 PM, Alexandre Julliard wrote: > This looks like a better approach, yes Sorry, this approach doesn't work as I expected, further investigation show that posted messages would be processed before input messages, msdn confirms it: --- quote --- If no filter is specified, messages are processed in the following order: Sent messages Posted messages Input (hardware) messages and system internal events Sent messages (again) WM_PAINT messages WM_TIMER messages --- quote --- msdn also said: To retrieve input messages before posted messages, use the wMsgFilterMin and wMsgFilterMaxparameters. However, even we can change the filter in our test, native Popup WndProc still receive posted messages before input message because it has its own message loop and we can't (and of cause we shouldn't) change it. This situation run out of my head, I can't find any easy way to fix it, I doubt if there is any way better than the original patch (patch 97161, pending). I'm open to any suggestion, thanks! If no better solution I hope patch 97161 is acceptable. -- Regards, Qian Hong - http://www.codeweavers.com
Re: [PATCH 1/3] user32/tests: Sleep enough time to make sure all input message have been processed.
Qian Hong writes: > On Fri, Jul 12, 2013 at 3:07 AM, Alexandre Julliard > wrote: >> Adding sleeps to work around timing issues is usually suspicious, > > Hello, how about this one? This looks like a better approach, yes (make sure to fix your tab size before submitting though, the indentation is messed up). -- Alexandre Julliard julli...@winehq.org
Re: [PATCH 1/3] user32/tests: Sleep enough time to make sure all input message have been processed.
On Fri, Jul 12, 2013 at 3:07 AM, Alexandre Julliard wrote:
> Adding sleeps to work around timing issues is usually suspicious,
Hello, how about this one?
--
Regards,
Qian Hong
-
http://www.codeweavers.com
diff --git a/dlls/user32/tests/menu.c b/dlls/user32/tests/menu.c
index 3e4cf93..fab7539 100644
--- a/dlls/user32/tests/menu.c
+++ b/dlls/user32/tests/menu.c
@@ -120,8 +120,11 @@ typedef struct
static BOOL bMenuVisible;
static INT popmenu;
static BOOL got_input;
+static BOOL got_last_input;
static HMENU hMenus[4];
+#define WM_LAST_INPUT (WM_USER + 1)
+
#define MOD_SIZE 10
#define MOD_NRMENUS 8
@@ -2113,6 +2116,7 @@ static DWORD WINAPI test_menu_input_thread(LPVOID
lpParameter)
int ret = TRUE, elapsed = 0;
got_input = i && menu_tests[i-1].bMenuVisible;
+ got_last_input = FALSE;
if (menu_tests[i].type == INPUT_KEYBOARD)
for (j = 0; menu_tests[i].wVk[j] != 0; j++)
@@ -2121,19 +2125,19 @@ static DWORD WINAPI test_menu_input_thread(LPVOID
lpParameter)
for (j = 0; menu_tests[i].menu_item_pairs[j].uMenu != 0; j++)
if (!(ret = click_menu(hWnd,
&menu_tests[i].menu_item_pairs[j]))) break;
+ PostMessage( hWnd, WM_LAST_INPUT, 0, 0);
if (!ret)
{
skip( "test %u: failed to send input\n", i );
PostMessage( hWnd, WM_CANCELMODE, 0, 0 );
return 0;
}
-while (menu_tests[i].bMenuVisible != bMenuVisible)
-{
+do {
if (elapsed > 200)
break;
elapsed += 20;
Sleep(20);
-}
+} while (!got_last_input);
if (!got_input)
{
@@ -2166,6 +2170,9 @@ static LRESULT CALLBACK WndProc(HWND hWnd, UINT msg,
WPARAM wParam,
case WM_EXITMENULOOP:
case WM_MENUSELECT:
break;
+ case WM_LAST_INPUT:
+ got_last_input = TRUE;
+ break;
case WM_KEYDOWN:
case WM_SYSKEYDOWN:
Re: [PATCH 1/3] user32/tests: Sleep enough time to make sure all input message have been processed.
On Fri, Jul 12, 2013 at 7:11 PM, Qian Hong wrote:
> do {
> sleeping;
> } while (sent_msg match received_msg or slept too long)
typo, should be:
while (sent_msg does not match received_msg && haven't sleep too long)
--
Regards,
Qian Hong
-
http://www.codeweavers.com
Re: [PATCH 1/3] user32/tests: Sleep enough time to make sure all input message have been processed.
Hello,
Thanks for the feedback.
On Fri, Jul 12, 2013 at 3:07 AM, Alexandre Julliard 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)
{
Re: [PATCH 1/3] user32/tests: Sleep enough time to make sure all input message have been processed.
Qian Hong writes: > Hello, > > This patch is marked as pending, is there anything I can improve? Did > the test results in [1] and [2] help? 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. -- Alexandre Julliard julli...@winehq.org
Re: [PATCH 1/3] user32/tests: Sleep enough time to make sure all input message have been processed.
Hello, This patch is marked as pending, is there anything I can improve? Did the test results in [1] and [2] help? Thanks for any feedback! [1] https://testbot.winehq.org/JobDetails.pl?Key=26191 [2] https://testbot.winehq.org/JobDetails.pl?Key=26190 On Wed, Jul 3, 2013 at 2:45 AM, Qian Hong wrote: > Hello, > > Current menu test code may hide some failure tests [1], with the attached > patch, all failure tests could be detected [2]. > > > [1] https://testbot.winehq.org/JobDetails.pl?Key=26191 > [2] https://testbot.winehq.org/JobDetails.pl?Key=26190 > > > --- > dlls/user32/tests/menu.c |1 + > 1 file changed, 1 insertion(+) > > > > > -- Regards, Qian Hong - http://www.codeweavers.com
