dlls/oleaut32: add tests for OleLoadPictureFile, OleLoadPictureFileEx, and OleLoadPicturePath

Michael Karcher wine at mkarcher.dialup.fu-berlin.de
Wed Nov 19 02:26:54 CST 2008


Am Dienstag, den 18.11.2008, 23:04 -0800 schrieb Jeremy Drake:

> +static void test_VerifyPictureType(IDispatch *pdisp, short expectedtype, int caller_line)
> +{
> +    IPicture *pict;
> +    HRESULT hr;
> +    short type;
> +    pict = (void*)0xdeadbeef;
> +    hr = IDispatch_QueryInterface(pdisp, &IID_IPicture, (LPVOID*)&pict);
> +    ok(hr == S_OK, "hr %08x\n", hr);
> +    ok(pict != (void*)0xdeadbeef, "pict ptr, caller line %d\n", caller_line);
What if pict == NULL at this place?

> +    if (pict != (void*)0xdeadbeef)
In the testcases, you only need to test for errors that are expected to
happen on some platform (e.g. Wine, because something is TODO). Error
handling just because there *could* be an error, but you can't imagine
any way how that call should fail makes the source code hard to read,
but do not provide any benefit in real-world test running.

And if you want to test, just test SUCCEEDED(hr) here. You would
otherwise have to test pict != (void*)0xdeadbeef && pict != NULL.

[...]
> +        tmp2 = (void*)0xdeadbeef;
> +        hr = VarBstrCat(V_BSTR(&varFilename), tmp, &tmp2);
> +        ok(hr == S_OK, "hr %08x\n", hr);
> +        ok(tmp2 != (void*)0xdeadbeef, "VarBstrCat\n");
> +        if (tmp2 != (void*)0xdeadbeef)
> +        {
> +            SysFreeString(V_BSTR(&varFilename));
> +            V_BSTR(&varFilename) = tmp2;
Here we definitely can assume that VarBstrCat succeeds. Don't worry
about out-of-memory. There are enough other testcases that also break in
that case.

> +    file = CreateFileA(filename, GENERIC_READ|GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, 0);
> +    ok(file != INVALID_HANDLE_VALUE, "file creation\n");
You could add an "GetLastError()" here to print.

> +                hr = OleLoadPicturePath(bstrname, NULL, 0, 0, &IID_IDispatch, (LPVOID*)&disp);
> +                ok(hr == INET_E_RESOURCE_NOT_FOUND /*>=XP*/ || hr == E_FAIL /*<=2k*/, "hr %08x\n", hr);
This is an excessively long line. Break it at the "||". Indent in a way,
that the "hr ==" line up.

Regards,
  Michael Karcher




More information about the wine-patches mailing list