Re: [PATCH v2 3/3] d2d1/tests: Add tests for standard effect properties.

[Ziqing Hui]

On 7/22/21 8:49 PM, Henri Verbeet wrote:
> On Thu, 22 Jul 2021 at 02:45, Ziqing Hui  wrote:
> > -
> >    todo_wine
> > -
> >    {
> > -
> >    hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_CLSID,
> > -
> >            D2D1_PROPERTY_TYPE_CLSID, (BYTE *)&clsid, sizeof(clsid));
> > -
> >    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
> > -
> >    if (hr == S_OK)
> > -
> >        ok(IsEqualGUID(&clsid, test->clsid), "Got unexpected clsid %s, expected %s.\n",
> > -
> >                debugstr_guid(&clsid), debugstr_guid(test->clsid));
> > -
> > -
> >    hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_CACHED,
> > -
> >            D2D1_PROPERTY_TYPE_BOOL, (BYTE *)&cached, sizeof(cached));
> > -
> >    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
> > -
> >    if (hr == S_OK)
> > -
> >        ok(cached == FALSE, "Got unexpected cached %d.\n", cached);
> > -
> > -
> >    hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_PRECISION,
> > -
> >            D2D1_PROPERTY_TYPE_ENUM, (BYTE *)&precision, sizeof(precision));
> > -
> >    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
> > -
> >    if (hr == S_OK)
> > -
> >        ok(precision == D2D1_BUFFER_PRECISION_UNKNOWN, "Got unexpected precision %u.\n", precision);
> > -
> > -
> >    hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_MIN_INPUTS,
> > -
> >            D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&min_inputs, sizeof(min_inputs));
> > -
> >    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
> > -
> >    if (hr == S_OK)
> > -
> >        ok(min_inputs == test->min_inputs, "Got unexpected min inputs %u, expected %u.\n",
> > -
> >                min_inputs, test->min_inputs);
> > -
> > -
> >    hr = ID2D1Effect_GetValue(effect, D2D1_PROPERTY_MAX_INPUTS,
> > -
> >            D2D1_PROPERTY_TYPE_UINT32, (BYTE *)&max_inputs, sizeof(max_inputs));
> > -
> >    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
> > -
> >    if (hr == S_OK)
> > -
> >        ok(max_inputs == test->max_inputs, "Got unexpected max inputs %u, expected %u.\n",
> > -
> >                max_inputs, test->max_inputs);
> > -
> >    }
>
> This has come up before, but if you're essentially asserting that a
> particular variable has a particular value using ok() (i.e., "ok(hr ==
> S_OK, ...);"), there's then no point in attempting to handle cases
> where that variable would have a different value.

The reason I check the "hr" is that we have a todo_wine here, which means that

the "hr" would have a different value when running the tests on wine.

Once we implemented it in wine, then we can remove the "todo_wine", also remove

the "if (hr == S_OK)".

What do you think, Henri? Should we remove the "if (hr == S_OK)"

in this patch right now or remove them after implementing  ID2D1Effect_GetValue()?