Re: [PATCH v2 2/3] d2d1/tests: Add tests for effect inputs.

[Henri Verbeet]

On Sat, 31 Jul 2021 at 06:25, Ziqing Hui  wrote:
> @@ -9797,6 +9800,60 @@ static void test_effect(BOOL d3d11)
>                      max_inputs, test->max_inputs);
>          }
> -
>    todo_wine
> -
>    {
> -
>    input_count = ID2D1Effect_GetInputCount(effect);
> -
>    ok (input_count == 1 || input_count == 2, "Got unexpected input count %u.\n", input_count);
> -

What determines whether this returns 1 or 2?

As an aside, I'd much prefer putting "todo_wine" before individual
ok() calls over using block-todo_wine.
> -
>    input_count = (test->max_inputs < 16 ? test->max_inputs : 16);
> -
>    for (j = 0; j < input_count + off_limit_tests; ++j)
> -
>    {

What is the meaning of the "off_limit_tests" variable? It's not
obvious to me from the variable name.
> -
>        hr = ID2D1Effect_SetInputCount(effect, j);
> -
>        if (j < test->min_inputs || j > test->max_inputs)
> -
>            ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr);
> -
>        else
> -
>            ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
> -
>    }

If these were to fail, it would not be obvious from the failure
message for which input/"j" that was.
> -
>    ID2D1Effect_SetInput(effect, 0, (ID2D1Image *)bitmap, FALSE);
> -
>    for (j = 0; j < input_count + off_limit_tests; ++j)
> -
>    {
> -
>        image_a = (ID2D1Image *)0xdeadbeef;
> -
>        if (j >= input_count)
> -
>            ID2D1Effect_SetInput(effect, j, (ID2D1Image *)bitmap, FALSE);
> -
>        ID2D1Effect_GetInput(effect, j, &image_a);
> -
>        if (j == 0)
> -
>        {
> -
>            ok(image_a == (ID2D1Image *)bitmap, "Got unexpected image_a %p.\n", image_a);
> -
>            if (image_a == (ID2D1Image *)bitmap)
> -
>                ID2D1Image_Release(image_a);
> -
>        }
> -
>        else
> -
>        {
> -
>            ok(image_a == NULL, "Got unexpected image_a %p.\n", image_a);
> -
>        }
> -
>    }

It may end up being nicer to use two separate loops for this.