[PATCH v3 0/1] MR772: tests: Allow marking unreliable tests as flaky.

[Francois Gouget (＠fgouget)]

--
  v3: tests: Allow marking unreliable tests as flaky.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/772

[PATCH v3 1/1] tests: Allow marking unreliable tests as flaky.

[Francois Gouget]

From: Francois Gouget <fgouget(a)codeweavers.com>

---
* The is_wine / is_windows variables seem controversial so I left them
  out of this version.
* No flaky_windows(): use plain flaky() instead.
* If a test if both flaky and todo it is only reported as flaky in the
  summary stats.
* The messages preserve the "Test failed" and "Test succeeded" prefix.
---
 include/wine/test.h | 81 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 71 insertions(+), 10 deletions(-)

diff --git a/include/wine/test.h b/include/wine/test.h
index 1690c5613d8..5d2d737f3a4 100644
--- a/include/wine/test.h
+++ b/include/wine/test.h
@@ -44,6 +44,9 @@ extern int winetest_time;
 /* running in interactive mode? */
 extern int winetest_interactive;
 
+/* always count flaky tests as successful (BOOL) */
+extern int winetest_allow_flaky;
+
 /* report successful tests (BOOL) */
 extern int winetest_report_success;
 
@@ -59,6 +62,9 @@ extern void winetest_ignore_exceptions( BOOL ignore );
 extern void winetest_start_todo( int is_todo );
 extern int winetest_loop_todo(void);
 extern void winetest_end_todo(void);
+extern void winetest_start_flaky( int is_flaky );
+extern int winetest_loop_flaky(void);
+extern void winetest_end_flaky(void);
 extern int winetest_get_mainargs( char*** pargv );
 extern LONG winetest_get_failures(void);
 extern void winetest_add_failures( LONG new_failures );
@@ -111,6 +117,13 @@ extern void winetest_pop_context(void);
 #define trace    trace_(__FILE__, __LINE__)
 #define wait_child_process wait_child_process_(__FILE__, __LINE__)
 
+#define flaky_if(is_flaky) for (winetest_start_flaky(is_flaky); \
+                                winetest_loop_flaky(); \
+                                winetest_end_flaky())
+#define flaky                   flaky_if(TRUE)
+#define flaky_wine              flaky_if(!strcmp(winetest_platform, "wine"))
+#define flaky_wine_if(is_flaky) flaky_if((is_flaky) && !strcmp(winetest_platform, "wine"))
+
 #define todo_if(is_todo) for (winetest_start_todo(is_todo); \
                               winetest_loop_todo(); \
                               winetest_end_todo())
@@ -194,6 +207,9 @@ int winetest_interactive = 0;
 /* current platform */
 const char *winetest_platform = "windows";
 
+/* always count flaky tests as successful (BOOL) */
+int winetest_allow_flaky;
+
 /* report successful tests (BOOL) */
 int winetest_report_success = 0;
 
@@ -211,6 +227,7 @@ static const struct test *current_test; /* test currently being run */
 
 static LONG successes;       /* number of successful tests */
 static LONG failures;        /* number of failures */
+static LONG flaky_failures;  /* number of failures inside flaky block */
 static LONG skipped;         /* number of skipped test chunks */
 static LONG todo_successes;  /* number of successful tests inside todo block */
 static LONG todo_failures;   /* number of failures inside todo block */
@@ -226,6 +243,8 @@ struct tls_data
 {
     const char* current_file;        /* file of current check */
     int current_line;                /* line of current check */
+    unsigned int flaky_level;        /* current flaky nesting level */
+    int flaky_do_loop;
     unsigned int todo_level;         /* current todo nesting level */
     int todo_do_loop;
     char *str_pos;                   /* position in debug buffer */
@@ -360,10 +379,19 @@ int winetest_vok( int condition, const char *msg, va_list args )
         if (condition)
         {
             if (winetest_color) printf( color_dark_red );
-            winetest_print_context( "Test succeeded inside todo block: " );
-            vprintf(msg, args);
+            if (data->flaky_level)
+            {
+                winetest_print_context( "Test succeeded inside flaky todo block: " );
+                vprintf(msg, args);
+                InterlockedIncrement(&flaky_failures);
+            }
+            else
+            {
+                winetest_print_context( "Test succeeded inside todo block: " );
+                vprintf(msg, args);
+                InterlockedIncrement(&todo_failures);
+            }
             if (winetest_color) printf( color_reset );
-            InterlockedIncrement(&todo_failures);
             return 0;
         }
         else
@@ -390,10 +418,19 @@ int winetest_vok( int condition, const char *msg, va_list args )
         if (!condition)
         {
             if (winetest_color) printf( color_bright_red );
-            winetest_print_context( "Test failed: " );
-            vprintf(msg, args);
+            if (data->flaky_level)
+            {
+                winetest_print_context( "Test marked flaky: " );
+                vprintf(msg, args);
+                InterlockedIncrement(&flaky_failures);
+            }
+            else
+            {
+                winetest_print_context( "Test failed: " );
+                vprintf(msg, args);
+                InterlockedIncrement(&failures);
+            }
             if (winetest_color) printf( color_reset );
-            InterlockedIncrement(&failures);
             return 0;
         }
         else
@@ -470,6 +507,27 @@ void winetest_win_skip( const char *msg, ... )
     va_end(valist);
 }
 
+void winetest_start_flaky( int is_flaky )
+{
+    struct tls_data *data = get_tls_data();
+    data->flaky_level = (data->flaky_level << 1) | (is_flaky != 0);
+    data->flaky_do_loop = 1;
+}
+
+int winetest_loop_flaky(void)
+{
+    struct tls_data *data = get_tls_data();
+    int do_flaky = data->flaky_do_loop;
+    data->flaky_do_loop = 0;
+    return do_flaky;
+}
+
+void winetest_end_flaky(void)
+{
+    struct tls_data *data = get_tls_data();
+    data->flaky_level >>= 1;
+}
+
 void winetest_start_todo( int is_todo )
 {
     struct tls_data *data = get_tls_data();
@@ -615,14 +673,16 @@ static int run_test( const char *name )
             printf( "%04x:%s:%s Silenced %d todos, %d skips and %d traces.\n",
                     (UINT)GetCurrentProcessId(), test->name, winetest_elapsed(),
                     (UINT)muted_todo_successes, (UINT)muted_skipped, (UINT)muted_traces);
-        printf( "%04x:%s:%s %d tests executed (%d marked as todo, %d %s), %d skipped.\n",
+        printf( "%04x:%s:%s %d tests executed (%d marked as todo, %d as flaky, %d %s), %d skipped.\n",
                 (UINT)GetCurrentProcessId(), test->name, winetest_elapsed(),
-                (UINT)(successes + failures + todo_successes + todo_failures),
-                (UINT)todo_successes, (UINT)(failures + todo_failures),
+                (UINT)(successes + failures + flaky_failures + todo_successes + todo_failures),
+                (UINT)todo_successes, (UINT)flaky_failures, (UINT)(failures + todo_failures),
                 (failures + todo_failures != 1) ? "failures" : "failure",
                 (UINT)skipped );
     }
-    status = (failures + todo_failures < 255) ? failures + todo_failures : 255;
+    status = failures + todo_failures;
+    if (!winetest_allow_flaky) status += flaky_failures;
+    if (status > 255) status = 255;
     return status;
 }
 
@@ -681,6 +741,7 @@ int main( int argc, char **argv )
         winetest_color = !strcasecmp(p, "auto") ? isatty(fileno(stdout)) : atoi(p);
     if (GetEnvironmentVariableA( "WINETEST_DEBUG", p, sizeof(p) )) winetest_debug = atoi(p);
     if (GetEnvironmentVariableA( "WINETEST_INTERACTIVE", p, sizeof(p) )) winetest_interactive = atoi(p);
+    if (GetEnvironmentVariableA( "WINETEST_ALLOW_FLAKY", p, sizeof(p) )) winetest_allow_flaky = atoi(p);
     if (GetEnvironmentVariableA( "WINETEST_REPORT_SUCCESS", p, sizeof(p) )) winetest_report_success = atoi(p);
     if (GetEnvironmentVariableA( "WINETEST_TIME", p, sizeof(p) )) winetest_time = atoi(p);
     winetest_last_time = winetest_start_time = GetTickCount();
-- 
GitLab

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

[Francois Gouget (＠fgouget)]

On Fri Sep  2 13:43:34 2022 +0000, Francois Gouget wrote:
> changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/772/diffs?diff_id=9610&start_sha=1c3abe52331d65d9eba9e4b8a33213862143ccc3#169268741a20a06ceffa2ae5c94b3ce00caed94e_514_514)
> Done (was copy/paste from winetest_*_todo()).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/772#note_7563

[Alexandre Julliard (＠julliard)]

Alexandre Julliard (@julliard) commented about include/wine/test.h:
> {
>              if (winetest_color) printf( color_bright_red );
> -            winetest_print_context( "Test failed: " );
> -            vprintf(msg, args);
> +            if (data->flaky_level)
> +            {
> +                winetest_print_context( "Test marked flaky: " );
> +                vprintf(msg, args);
> +                InterlockedIncrement(&flaky_failures);
> +            }
> +            else
> +            {
> +                winetest_print_context( "Test failed: " );
> +                vprintf(msg, args);
> +                InterlockedIncrement(&failures);
> +            }

Flaky failures should probably use a different color, for instance the color used for todos, so that real errors stand out.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/772#note_7568