[PATCH] shell32/tests: Drop shell folder test workarounds for Windows <= 2000

Nikolay Sivov nsivov at codeweavers.com
Thu Jan 11 01:45:30 CST 2018


On 1/11/2018 10:15 AM, Alex Henrie wrote:
>      hmod = GetModuleHandleA("kernel32.dll");
>      pIsWow64Process = (void*)GetProcAddress(hmod, "IsWow64Process");
> -    pGetSystemWow64DirectoryW = (void*)GetProcAddress(hmod, "GetSystemWow64DirectoryW");
Will this work on XP?
>  
> @@ -278,10 +237,10 @@ static void test_ParseDisplayName(void)
>      }
>  
>      MultiByteToWideChar(CP_ACP, 0, cNonExistDir1A, -1, cTestDirW, MAX_PATH);
> -    hr = IShellFolder_ParseDisplayName(IDesktopFolder, 
> +    hr = IShellFolder_ParseDisplayName(IDesktopFolder,
>          NULL, NULL, cTestDirW, NULL, &newPIDL, 0);
> -    ok((hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)) || (hr == E_FAIL), 
> -        "ParseDisplayName returned %08x, expected 80070002 or E_FAIL\n", hr);
> +    ok(hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND),
> +        "ParseDisplayName returned %08x, expected 0x80070002\n", hr);
>  
>      res = GetFileAttributesA(cNonExistDir2A);
>      if(res != INVALID_FILE_ATTRIBUTES)
> @@ -291,21 +250,15 @@ static void test_ParseDisplayName(void)
>      }
>  
>      MultiByteToWideChar(CP_ACP, 0, cNonExistDir2A, -1, cTestDirW, MAX_PATH);
> -    hr = IShellFolder_ParseDisplayName(IDesktopFolder, 
> +    hr = IShellFolder_ParseDisplayName(IDesktopFolder,
>          NULL, NULL, cTestDirW, NULL, &newPIDL, 0);
> -    ok((hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)) || (hr == E_FAIL) || (hr == E_INVALIDARG), 
> -        "ParseDisplayName returned %08x, expected 80070002, E_FAIL or E_INVALIDARG\n", hr);
> +    todo_wine ok(hr == E_INVALIDARG, "ParseDisplayName returned %08x, expected E_INVALIDARG\n", hr);
Maybe it's better to use FAILED() in such cases.
>      IShellFolder_Release(psfSystemDir);
>  
> @@ -586,23 +527,19 @@ if (0)
>              {
>                  IPersist *pp;
>                  hr = IShellFolder_QueryInterface(psfChild, &IID_IPersist, (void**)&pp);
> -                ok(hr == S_OK ||
> -                   broken(hr == E_NOINTERFACE), /* Win9x, NT4, W2K */
> -                   "Got 0x%08x\n", hr);
> +                ok(hr == S_OK, "Got 0x%08x\n", hr);
>                  if(SUCCEEDED(hr))
>                  {
>                      CLSID id;
>                      hr = IPersist_GetClassID(pp, &id);
>                      ok(hr == S_OK, "Got 0x%08x\n", hr);
> -                    /* CLSID_ShellFSFolder on some w2k systems */
> -                    ok(IsEqualIID(&id, &CLSID_ShellDocObjView) || broken(IsEqualIID(&id, &CLSID_ShellFSFolder)),
> -                        "Unexpected classid %s\n", wine_dbgstr_guid(&id));
> +                    ok(IsEqualIID(&id, &CLSID_ShellDocObjView), "Unexpected classid %s\n", wine_dbgstr_guid(&id));
>                      IPersist_Release(pp);
>                  }
Please make this and similar cases more linear, we don't need
conditional block anymore.
>  
>                  IShellFolder_Release(psfChild);
>              }
> -            pILFree(pidl);
> +            ILFree(pidl);
>          }
>          DeleteFileA(pathA);
>      }
> @@ -623,12 +560,10 @@ if (0)
>          {
>              hr = IShellFolder_BindToObject(psfDesktop, pidl, NULL, &IID_IShellFolder, (void**)&psfChild);
>              ok(hr == E_FAIL || /* Vista+ */
> -               hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND) || /* XP, W2K3 */
> -               hr == E_INVALIDARG || /* W2K item in top dir */
> -               broken(hr == S_OK), /* Win9x, NT4, W2K */
> +               hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), /* XP, W2K3 */
>                 "Got 0x%08x\n", hr);
Same here, FAILED() might be better.
>
>
> -  
>      /* Some tests for IShellFolder::SetNameOf */
> -    if (pSHGetFolderPathAndSubDirA)
> +    hr = SHBindToParent(pidlTestFile, &IID_IShellFolder, (VOID**)&psfPersonal, &pidlLast);
Here and later on, since you're changing the line anyway, please use
actual type plus a space like (void **), same for other pointer types.
>      /* SHBindToParent fails, if called with a NULL PIDL. */
> -    hr = pSHBindToParent(NULL, &IID_IShellFolder, (VOID**)&psfPersonal, &pidlLast);
> +    hr = SHBindToParent(NULL, &IID_IShellFolder, (void**)&psfPersonal, &pidlLast);
>      ok (hr != S_OK, "SHBindToParent(NULL) should fail!\n");
That one should actually be FAILED().

Overall I think it's good, but too large for a single change. I would
start with getting rid of dynamic imports as a first patch, leaving
broken() cleanup and special version dependent cases for later.



More information about the wine-devel mailing list