[PATCH v3] shell32/tests: Avoid ever changing values in the shlexec failure messages.

[Zebediah Figura]

On 3/15/20 7:18 PM, Francois Gouget wrote:
> Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
> ---
> 
> v3: The derandomisation must be done in the ok*() functions, not before
>      to not break the comparisons. The TestBot now shows the expected
>      results: it still has failures but the TestBot will be able to see
>      that they are not new.
> 
>   dlls/shell32/tests/shlexec.c | 60 ++++++++++++++++++++++++++++--------
>   1 file changed, 48 insertions(+), 12 deletions(-)
> 
> diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c
> index 030ab54525a..3571b64cc9e 100644
> --- a/dlls/shell32/tests/shlexec.c
> +++ b/dlls/shell32/tests/shlexec.c
> @@ -350,6 +350,42 @@ static void dump_child_(const char* file, int line)
>    *
>    ***/
>   
> +static void substTmpPath(char* str, const char *path, char rep)
> +{
> +    char *p;
> +
> +    /* Only modify the last component of the path */
> +    p = strrchr(path, '\\');
> +    if (p) path = p + 1;
> +    p = strrchr(path, '/');
> +    if (p) path = p + 1;
> +
> +    p = strstr(str, path);
> +    while (p)
> +    {
> +        p += 2; /* skip the 'wt' prefix */
> +        /* and only replace the random part of the filename */
> +        while (*p && *p != '.')
> +            *p++ = rep;
> +        p = strstr(p, path);
> +    }
> +}
> +
> +static const char* derandomizeString(const char* str)
> +{
> +    static char buffer[2][2048];
> +    static int current_buffer = 0;
> +
> +    if (!str || strlen(str) >= sizeof(buffer[0]))
> +        return str;
> +
> +    current_buffer = current_buffer ? 0 : 1;
> +    strcpy(buffer[current_buffer], str);
> +    substTmpPath(buffer[current_buffer], tmpdir, 'D');
> +    substTmpPath(buffer[current_buffer], child_file, 'C');
> +    return buffer[current_buffer];
> +}
> +

This seems an awful lot of effort to put in just to avoid a variable 
string being printed. Is there really anything we get out of having 
those strings printed in the first place? Would it be better to just 
remove them?

>   static char shell_call[2048];
>   static void WINAPIV __WINE_PRINTF_ATTR(2,3) _okShell(int condition, const char *msg, ...)
>   {
> @@ -378,12 +414,12 @@ static void okChildString_(const char* file, int line, const char* key, const ch
>       result=getChildString("Child", key);
>       if (!result)
>       {
> -        okShell_(file, line)(FALSE, "%s expected '%s', but key not found or empty\n", key, expected);
> +        okShell_(file, line)(FALSE, "%s expected '%s', but key not found or empty\n", key, derandomizeString(expected));
>           return;
>       }
>       okShell_(file, line)(lstrcmpiA(result, expected) == 0 ||
>                            broken(lstrcmpiA(result, bad) == 0),
> -                         "%s expected '%s', got '%s'\n", key, expected, result);
> +                         "%s expected '%s', got '%s'\n", key, derandomizeString(expected), derandomizeString(result));
>   }
>   #define okChildString(key, expected) okChildString_(__FILE__, __LINE__, (key), (expected), (expected))
>   #define okChildStringBroken(key, expected, broken) okChildString_(__FILE__, __LINE__, (key), (expected), (broken))
> @@ -432,7 +468,7 @@ static void okChildPath_(const char* file, int line, const char* key, const char
>       result=getChildString("Child", key);
>       if (!result)
>       {
> -        okShell_(file,line)(FALSE, "%s expected '%s', but key not found or empty\n", key, expected);
> +        okShell_(file,line)(FALSE, "%s expected '%s', but key not found or empty\n", key, derandomizeString(expected));
>           return;
>       }
>       shortequal = FALSE;
> @@ -451,7 +487,7 @@ static void okChildPath_(const char* file, int line, const char* key, const char
>           }
>       }
>       okShell_(file,line)(equal || broken(shortequal) /* XP SP1 */,
> -                        "%s expected '%s', got '%s'\n", key, expected, result);
> +                        "%s expected '%s', got '%s'\n", key, derandomizeString(expected), derandomizeString(result));
>   }
>   #define okChildPath(key, expected) okChildPath_(__FILE__, __LINE__, (key), (expected))
>   
> @@ -507,9 +543,9 @@ static INT_PTR shell_execute_(const char* file, int line, LPCSTR verb, LPCSTR fi
>   
>       strcpy(shell_call, "ShellExecute(");
>       strcat_param(shell_call, "verb", verb);
> -    strcat_param(shell_call, "file", filename);
> -    strcat_param(shell_call, "params", parameters);
> -    strcat_param(shell_call, "dir", directory);
> +    strcat_param(shell_call, "file", derandomizeString(filename));
> +    strcat_param(shell_call, "params", derandomizeString(parameters));
> +    strcat_param(shell_call, "dir", derandomizeString(directory));
>       strcat(shell_call, ")");
>       strcat(shell_call, assoc_desc);
>       if (winetest_debug > 1)
> @@ -591,9 +627,9 @@ static INT_PTR shell_execute_ex_(const char* file, int line,
>       sprintf(smask, "0x%x", mask);
>       strcat_param(shell_call, "mask", smask);
>       strcat_param(shell_call, "verb", verb);
> -    strcat_param(shell_call, "file", filename);
> -    strcat_param(shell_call, "params", parameters);
> -    strcat_param(shell_call, "dir", directory);
> +    strcat_param(shell_call, "file", derandomizeString(filename));
> +    strcat_param(shell_call, "params", derandomizeString(parameters));
> +    strcat_param(shell_call, "dir", derandomizeString(directory));
>       strcat_param(shell_call, "class", class);
>       strcat(shell_call, ")");
>       strcat(shell_call, assoc_desc);
> @@ -832,7 +868,7 @@ static void create_test_verb_dde(const char* classname, const char* verb,
>       sprintf(shell, "%d", rawcmd);
>       strcat_param(assoc_desc, "rawcmd", shell);
>       strcat_param(assoc_desc, "cmdtail", cmdtail);
> -    strcat_param(assoc_desc, "ddeexec", ddeexec);
> +    strcat_param(assoc_desc, "ddeexec", derandomizeString(ddeexec));
>       strcat_param(assoc_desc, "app", application);
>       strcat_param(assoc_desc, "topic", topic);
>       strcat_param(assoc_desc, "ifexec", ifexec);
> @@ -2080,7 +2116,7 @@ static void test_find_executable(void)
>           if (rc > 32)
>               rc=33;
>           todo_wine_if(test->todo & 0x10)
> -            ok(rc==test->rc, "FindExecutable(%s) failed: rc=%ld\n", filename, rc);
> +            ok(rc==test->rc, "FindExecutable(%s) failed: rc=%ld\n", derandomizeString(filename), rc);
>           if (rc > 32)
>           {
>               BOOL equal;
>