[PATCH] shell32/tests: add tests for the parser of SHELLEXECUTEINFO.lpFile (resend)

[Paul Vriens]

On 02/11/2010 03:37 PM, Ilya Basin wrote:
> Hi! This should expose this bug:
> http://bugs.winehq.org/show_bug.cgi?id=19385 ( the 'wine start'
> launcher does not open MS Office documents that have spaces in their
> path ). Although the title is misleading, many other tickets are
> marked as duplicates of it.
> It's not about spaces. It's about sometimes unneeded parsing of
> SHELLEXECUTEINFO.lpFile, trying to extract arguments from it. Why?
> Or maybe some other part of wine relies on this behavior?
> Tested on 98 se, 2k&  xp.
>
> ...obvious mistakes become visible only after you hit the "send"
> button. So I apologize beforehand.

Hi Ilya,

As Wine doesn't do the right thing (according to your tests), all 
failing Wine tests should be marked as such. We have a todo_wine 
construct for that.

>
> ---
>   dlls/shell32/tests/shlexec.c |  106 ++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 106 insertions(+), 0 deletions(-)
>
> diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c
> index c673f0a..fb5e6b7 100644
> --- a/dlls/shell32/tests/shlexec.c
> +++ b/dlls/shell32/tests/shlexec.c
> @@ -797,6 +797,10 @@ static const char* testfiles[]=
>       "%s\\test file.sde",
>       "%s\\test file.exe",
>       "%s\\test2.exe",
> +    "%s\\simple.shlexec",
> +    "%s\\drawback_file.noassoc",
> +    "%s\\drawback_file.noassoc foo.shlexec",
> +    "%s\\drawback_nonexist.noassoc foo.shlexec",
>       NULL
>   };
>
> @@ -852,6 +856,107 @@ static filename_tests_t noquotes_tests[]=
>       {NULL, NULL, 0}
>   };
>
> +static void test_lpFile_parsed(void)
> +{
> +    /* basename tmpdir */
> +    const char* shorttmpdir;
> +
> +    const char *testfile;
> +    char fileA[MAX_PATH];
> +
> +    int rc;
> +    int expected;
> +
> +    BOOL iswin9x = FALSE;
> +    BOOL isreallyNT = FALSE;
> +
> +    /* detect win ver */
> +    {
> +        DWORD dwVersion = GetVersion();
> +           DWORD dwWindowsMajorVersion =  (DWORD)(LOBYTE(LOWORD(dwVersion)));
> +        DWORD dwMinorVersion = (DWORD)(HIBYTE(LOWORD(dwVersion)));
> +           DWORD dwBuild;
> +
> +           if (dwVersion<  0x80000000)              // Windows NT/2000, Whistler

No C++ comments please.

> +                   dwBuild = (DWORD)(HIWORD(dwVersion));
> +           else if (dwWindowsMajorVersion<  4)      // Win32s
> +                   dwBuild = (DWORD)(HIWORD(dwVersion)&  ~0x8000);
> +           else                                     // Windows 95/98/Me
> +           {
> +                   dwBuild =  0;
> +            iswin9x = TRUE;
> +            isreallyNT = NULL != GetModuleHandle("ntdll");
> +           }
> +    }

We generally do not want any real version checking in the tests. Tests 
should decide based on behavior on what platform they are.

> +
> +    GetTempPathA(sizeof(fileA), fileA);
> +    shorttmpdir = tmpdir + strlen(fileA);
> +
> +    /* ensure tmpdir is in %TEMP% */
> +    SetEnvironmentVariableA("TEMP", fileA);

Why is this necessary?

> +
> +    #define TEST_LPFILE_PARSED_OK_() (rc==expected || rc>32&&  expected>32)
> +    #define TEST_LPFILE_PARSED_OK() ok(TEST_LPFILE_PARSED_OK_(), "expected %s (%d), got %s (%d), lpFile: %s \n", expected==33 ? "success" : "failure", expected, rc>  32 ? "success" : "failure", rc, fileA)
> +
> +    /* existing "drawback_file.noassoc" prevents finding "drawback_file.noassoc foo.shlexec" on wine */
> +    testfile = "%s\\drawback_file.noassoc foo.shlexec";
> +    expected = 33; sprintf(fileA, testfile, tmpdir);
> +    rc=shell_execute(NULL, fileA, NULL, NULL);
> +    TEST_LPFILE_PARSED_OK();
> +
> +    /* if quoted, existing "drawback_file.noassoc" not prevents finding "drawback_file.noassoc foo.shlexec" on wine */
> +    testfile = "\"%s\\drawback_file.noassoc foo.shlexec\"";
> +    expected = 33; sprintf(fileA, testfile, tmpdir);
> +    rc=shell_execute(NULL, fileA, NULL, NULL);
> +    TEST_LPFILE_PARSED_OK();
> +
> +    /* error should be 2, not 31 */
> +    testfile = "\"%s\\drawback_file.noassoc\" foo.shlexec";
> +    expected = 2; sprintf(fileA, testfile, tmpdir);
> +    rc=shell_execute(NULL, fileA, NULL, NULL);
> +    TEST_LPFILE_PARSED_OK();
> +
> +    /* ""command"" not works on wine (and real win9x and w2k) */
> +    testfile = "\"\"%s\\drawback_file.noassoc foo.shlexec\"\"";
> +    expected = iswin9x&&  !isreallyNT || dllver.dwMajorVersion<= 5 ? 2 : 33; sprintf(fileA, testfile, tmpdir);
> +    rc=shell_execute(NULL, fileA, NULL, NULL);
> +    TEST_LPFILE_PARSED_OK();
> +
> +    /* nonexisting "drawback_nonexist.noassoc" not prevents finding "drawback_nonexist.noassoc foo.shlexec" on wine */
> +    testfile = "%s\\drawback_nonexist.noassoc foo.shlexec";
> +    expected = 33; sprintf(fileA, testfile, tmpdir);
> +    rc=shell_execute(NULL, fileA, NULL, NULL);
> +    TEST_LPFILE_PARSED_OK();
> +
> +    /* is SEE_MASK_DOENVSUBST default flag? Should only be when XP emulates 9x */
> +    {

Any particular reason for this indentation?


-- 
Cheers,

Paul.