[PATCH v6 2/4] shell32/tests: Add tests for IShellItemImageFactory.
Nikolay Sivov
nsivov at codeweavers.com
Tue Apr 26 07:34:09 CDT 2022
On 4/25/22 18:21, Jinoh Kang wrote:
> + if (!pSHCreateShellItem)
> + {
> + win_skip("SHCreateShellItem isn't available\n");
> + return;
> + }
You probably don't need to check that for new tests. I don't see skips
from it on win7+ test results.
> + ret = SHGetSpecialFolderLocation(NULL, CSIDL_DESKTOP, &pidl_desktop);
> + ok(ret == S_OK, "Got 0x%08lx\n", ret);
> + if (SUCCEEDED(ret))
> + {
> + ret = pSHCreateShellItem(NULL, NULL, pidl_desktop, &shellitem);
> + ok(SUCCEEDED(ret), "SHCreateShellItem returned %lx\n", ret);
> + if (SUCCEEDED(ret))
> + {
> + IShellItemImageFactory *siif;
> + ret = IShellItem_QueryInterface(shellitem, &IID_IShellItemImageFactory, (void **)&siif);
> + todo_wine
> + ok(ret == S_OK, "QueryInterface returned %lx\n", ret);
> + if (SUCCEEDED(ret))
> + {
> + HBITMAP hbm = NULL;
> + SIZE size = {32, 32};
> +
> + ok(!GetModuleHandleA("windowscodecs.dll"), "WIC should not have already been loaded\n");
> +
> + ret = IShellItemImageFactory_GetImage(siif, size, SIIGBF_BIGGERSIZEOK, &hbm);
> + todo_wine
> + ok(ret == S_OK || broken(ret == E_PENDING /* win7 */), "GetImage returned %lx\n", ret);
> + ok(FAILED(ret) == !hbm, "result = %lx but bitmap = %p\n", ret, hbm);
What's going on with win7? Have you tried different arguments or running
on some temp folder, instead of the desktop one? I would be great to
have it pass. Maybe size/flags problem?
> +
> + todo_wine
> + ok(!!GetModuleHandleA("windowscodecs.dll"), "WIC should have been loaded\n");
I don't think windowscodecs check needs to be here. I understand you
used it to verify that windowscodecs is loaded, but now that you know
it, you don't have to put as a requirement. Also this has a potential to
break, because of the test order.
> +
> + if (SUCCEEDED(ret) && hbm)
> + {
> + DWORD objtype = GetObjectType(hbm);
> + ok(objtype == OBJ_BITMAP, "Expected type OBJ_BITMAP, got %lu\n", objtype);
> + DeleteObject(hbm);
> + }
I don't think this is necessary. Method explicitly claims to return
HBITMAP, and we can assume it is such handle if it's not 0.
> + IShellItemImageFactory_Release(siif);
> + }
> + IShellItem_Release(shellitem);
> + }
> + ILFree(pidl_desktop);
> + }
I suggest removing some of that error handling, leaving only what's
needed because of todo.
More information about the wine-devel
mailing list