[PATCH v4 0/1] MR11716: shell32/tests: Mark systray test as flaky on Windows

[Andrea Faulds (＠hikari_no_yume)]

It seems to fail on the Windows 10 runners for every MR that touches shell32 and thus triggers the test, but as far as I can tell there is nothing wrong with the test itself, and it does run fine in my Win10 VM.

Related: https://bugs.winehq.org//show_bug.cgi?id=55017

--
  v4: shell32/tests: Mark systray test as flaky on Windows
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11716

[PATCH v4 1/1] shell32/tests: Mark systray test as flaky on Windows

[Andrea Faulds]

From: Andrea Faulds <[e-mail]>

It seems to fail on the Windows 10 runners for every MR that touches
shell32 and thus triggers the test, but as far as I can tell there is
nothing wrong with the test itself, and it does run fine in my Win10 VM.

See: https://bugs.winehq.org//show_bug.cgi?id=55017
---
 dlls/shell32/tests/systray.c | 97 +++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 45 deletions(-)

diff --git a/dlls/shell32/tests/systray.c b/dlls/shell32/tests/systray.c
index 80621761240..d238ea8fce9 100644
--- a/dlls/shell32/tests/systray.c
+++ b/dlls/shell32/tests/systray.c
@@ -33,61 +33,68 @@ static void test_cbsize(void)
     NOTIFYICONDATAA nidA;
     BOOL ret;
 
-    if (pShell_NotifyIconW)
+    /* This test frequently fails with a non-specific E_FAIL on Windows in CI,
+     * but it does work fine when run standalone, and has no problems in WINE.
+     * See: https://bugs.winehq.org//show_bug.cgi?id=55017
+     */
+    flaky_if(!winetest_platform_is_wine)
     {
-        NOTIFYICONDATAW nidW;
-
-        ZeroMemory(&nidW, sizeof(nidW));
-        nidW.cbSize = NOTIFYICONDATAW_V1_SIZE;
-        nidW.hWnd = hMainWnd;
-        nidW.uID = 1;
-        nidW.uFlags = NIF_ICON|NIF_MESSAGE;
-        nidW.hIcon = LoadIconA(NULL, (LPSTR)IDI_APPLICATION);
-        nidW.uCallbackMessage = WM_USER+17;
+        if (pShell_NotifyIconW)
+        {
+            NOTIFYICONDATAW nidW;
+
+            ZeroMemory(&nidW, sizeof(nidW));
+            nidW.cbSize = NOTIFYICONDATAW_V1_SIZE;
+            nidW.hWnd = hMainWnd;
+            nidW.uID = 1;
+            nidW.uFlags = NIF_ICON|NIF_MESSAGE;
+            nidW.hIcon = LoadIconA(NULL, (LPSTR)IDI_APPLICATION);
+            nidW.uCallbackMessage = WM_USER+17;
+            SetLastError(0xdeadbeef);
+            ret = pShell_NotifyIconW(NIM_ADD, &nidW);
+            ok(ret, "NIM_ADD failed!\n");
+            ok(GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_NO_TOKEN,
+               "GetLastError() = %lu\n", GetLastError());
+            /* using an invalid cbSize does work */
+            nidW.cbSize = 3;
+            nidW.hWnd = hMainWnd;
+            nidW.uID = 1;
+            ret = pShell_NotifyIconW(NIM_DELETE, &nidW);
+            ok( ret || broken(!ret), /* nt4 */ "NIM_DELETE failed!\n");
+            /* as icon doesn't exist anymore - now there will be an error */
+            nidW.cbSize = sizeof(nidW);
+            SetLastError(0xdeadbeef);
+            ok(!pShell_NotifyIconW(NIM_DELETE, &nidW) != !ret, "The icon was not deleted\n");
+            ok(GetLastError() == E_FAIL || GetLastError() == ERROR_TIMEOUT,
+               "GetLastError() = %lu\n", GetLastError());
+        }
+
+        /* same for Shell_NotifyIconA */
+        ZeroMemory(&nidA, sizeof(nidA));
+        nidA.cbSize = NOTIFYICONDATAA_V1_SIZE;
+        nidA.hWnd = hMainWnd;
+        nidA.uID = 1;
+        nidA.uFlags = NIF_ICON|NIF_MESSAGE;
+        nidA.hIcon = LoadIconA(NULL, (LPSTR)IDI_APPLICATION);
+        nidA.uCallbackMessage = WM_USER+17;
         SetLastError(0xdeadbeef);
-        ret = pShell_NotifyIconW(NIM_ADD, &nidW);
-        ok(ret, "NIM_ADD failed!\n");
+        ok(Shell_NotifyIconA(NIM_ADD, &nidA), "NIM_ADD failed!\n");
         ok(GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_NO_TOKEN,
            "GetLastError() = %lu\n", GetLastError());
+
         /* using an invalid cbSize does work */
-        nidW.cbSize = 3;
-        nidW.hWnd = hMainWnd;
-        nidW.uID = 1;
-        ret = pShell_NotifyIconW(NIM_DELETE, &nidW);
-        ok( ret || broken(!ret), /* nt4 */ "NIM_DELETE failed!\n");
+        nidA.cbSize = 3;
+        nidA.hWnd = hMainWnd;
+        nidA.uID = 1;
+        ret = Shell_NotifyIconA(NIM_DELETE, &nidA);
+        ok(ret, "NIM_DELETE failed!\n");
         /* as icon doesn't exist anymore - now there will be an error */
-        nidW.cbSize = sizeof(nidW);
+        nidA.cbSize = sizeof(nidA);
         SetLastError(0xdeadbeef);
-        ok(!pShell_NotifyIconW(NIM_DELETE, &nidW) != !ret, "The icon was not deleted\n");
+        ok(!Shell_NotifyIconA(NIM_DELETE, &nidA) != !ret, "The icon was not deleted\n");
         ok(GetLastError() == E_FAIL || GetLastError() == ERROR_TIMEOUT,
            "GetLastError() = %lu\n", GetLastError());
     }
-
-    /* same for Shell_NotifyIconA */
-    ZeroMemory(&nidA, sizeof(nidA));
-    nidA.cbSize = NOTIFYICONDATAA_V1_SIZE;
-    nidA.hWnd = hMainWnd;
-    nidA.uID = 1;
-    nidA.uFlags = NIF_ICON|NIF_MESSAGE;
-    nidA.hIcon = LoadIconA(NULL, (LPSTR)IDI_APPLICATION);
-    nidA.uCallbackMessage = WM_USER+17;
-    SetLastError(0xdeadbeef);
-    ok(Shell_NotifyIconA(NIM_ADD, &nidA), "NIM_ADD failed!\n");
-    ok(GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_NO_TOKEN,
-       "GetLastError() = %lu\n", GetLastError());
-
-    /* using an invalid cbSize does work */
-    nidA.cbSize = 3;
-    nidA.hWnd = hMainWnd;
-    nidA.uID = 1;
-    ret = Shell_NotifyIconA(NIM_DELETE, &nidA);
-    ok(ret, "NIM_DELETE failed!\n");
-    /* as icon doesn't exist anymore - now there will be an error */
-    nidA.cbSize = sizeof(nidA);
-    SetLastError(0xdeadbeef);
-    ok(!Shell_NotifyIconA(NIM_DELETE, &nidA) != !ret, "The icon was not deleted\n");
-    ok(GetLastError() == E_FAIL || GetLastError() == ERROR_TIMEOUT,
-       "GetLastError() = %lu\n", GetLastError());
 }
 
 START_TEST(systray)
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/11716

[Andrea Faulds (＠hikari_no_yume)]

Given this documentation comment, I don't know if marking this test as flaky is really appropriate:

```
/* If is_flaky is true, indicates that the next test may occasionally fail due
 * to unavoidable outside race conditions. Such failures will be flagged as
 * flaky so they can be ignored by automated testing tools.
 *
 * Remarks:
 * - This is not meant to paper over race conditions within the test itself.
 *   Those are bugs and should be fixed.
 * - This is not meant to be used for tests that normally succeed but
 *   systematically fail on a specific platform or locale.
 * - The failures should be rare to start with. If a test fails 25% of the time
 *   it is probably wrong.
 */
static inline void winetest_start_flaky( int is_flaky )
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11716#note_149393

[PATCH v4 0/1] MR11716: shell32/tests: Mark systray test as flaky on Windows - closed

[Andrea Faulds (＠hikari_no_yume)]

This merge request was closed by Andrea Faulds.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11716