[2/2] d3dx9_36/tests: Add tests for D3DXCreatePolygon. (try 3)

[Sebastian Lackner]

I am also fine with fixing these issues right away, especially because of the ugly typo - the error check for D3DXCreatePolygon() was copied from existing tests, I assumed there is a good reason why D3DXCreate*() could fail on some machines/testbots.
Would you mind taking another look please? Thanks for your feedback!

Regards,
Sebastian

On 05.11.2014 18:03, Matteo Bruni wrote:
> 2014-11-04 19:57 GMT+01:00 Sebastian Lackner <sebastian at fds-team.de>:
>> Based on a patch by David Adam.
>> ---
>>  dlls/d3dx9_36/tests/mesh.c | 163 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 163 insertions(+)
> 
> Hi Sebastian,
> 
> first of all, let me say I'm okay with these patches going in as they are.
> 
> IMHO there is still room for improvement though:
> 
> +static void test_polygon(IDirect3DDevice9 *device, float length,
> unsigned int sides)
> +{
> +    HRESULT hr;
> +    ID3DXMesh *polygon;
> +    struct mesh mesh;
> +    char name[256];
> 
> No need to use such a big array there.
> 
> +    hr = D3DXCreatePolygon(device, length, sides, &polygon, NULL);
> +    ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK)\n", hr);
> +    if (hr != D3D_OK)
> +    {
> +        skip("Couldn't create box\n");
> 
> Copy-paste typo. FWIW, we usually prefer a '.' at the end of trace messages.
> 
> +    adjacency = NULL;
> +    hr = D3DXCreatePolygon(device, 3.0f, 11, &polygon, &adjacency);
> +    ok(hr == D3D_OK, "Expected D3D_OK, received %#x\n", hr);
> +
> +    if (FAILED(hr))
> +    {
> +        skip("D3DXCreatePolygon failed\n");
> +        goto end;
> +    }
> 
> That D3DXCreatePolygon() call is not expected to ever fail, so no need
> for the "if".
> 
> +    if (adjacency)
> +        ID3DXBuffer_Release(adjacency);
> 
> If you remove the previous "if" you won't need the "end" label and
> this one becomes always true. Then I'd put this Release() right after
> the loop checking the adjacency.
> 
> Again, I don't care about those details, this is still good enough for
> me. It's mostly about any future patch you'd want to send in.
>