Re: [PATCH 1/2] d3drm/tests: Add tests for IDirect3DRM*::CreateObject (v2).

[Henri Verbeet]

On 11 April 2016 at 20:04, Aaryaman Vasishta  wrote:
> -
>    hr = IDirect3DRM_CreateObject(d3drm1, NULL, NULL, tests[i].iid, (void **)&unknown);
> -
>    todo_wine ok(hr == D3DRMERR_BADVALUE, "Test %u: expected hr == D3DRMERR_BADVALUE, got %#x.\n", i, hr);

Note that mentioning the expected value is mostly redundant here. It's
obvious as soon as you look at the test source, and test output isn't
generally useful without that.
> -
>    hr = IDirect3DRM_CreateObject(d3drm1, tests[i].clsid, NULL, tests[i].iid, (void **)&unknown);
> -
>    todo_wine ok(SUCCEEDED(hr), "Test %u: expected hr == D3DRM_OK, got %#x.\n", i, hr);
> -
>    if (SUCCEEDED(hr))
> -
>    {

I'll point out that if you had done a "continue" on failure instead,
you wouldn't need to change the indentation when you remove the
todo_wine.

[PATCH 1/2] d3drm/tests: Add tests for IDirect3DRM*::CreateObject (v2).

[Aaryaman Vasishta]

Thanks for the review! Will keep that in mind for the next patches.

Cheers,
Aaryaman

On Tue, Apr 12, 2016 at 6:47 PM, Henri Verbeet  wrote:
> On 11 April 2016 at 20:04, Aaryaman Vasishta 
> wrote:
> > -
> >    hr = IDirect3DRM_CreateObject(d3drm1, NULL, NULL, tests[i].iid,
>
> (void **)&unknown);
> > -
> >    todo_wine ok(hr == D3DRMERR_BADVALUE, "Test %u: expected hr ==
>
> D3DRMERR_BADVALUE, got %#x.\n", i, hr);
> Note that mentioning the expected value is mostly redundant here. It's
> obvious as soon as you look at the test source, and test output isn't
> generally useful without that.
> > -
> >    hr = IDirect3DRM_CreateObject(d3drm1, tests[i].clsid, NULL,
>
> tests[i].iid, (void **)&unknown);
> > -
> >    todo_wine ok(SUCCEEDED(hr), "Test %u: expected hr == D3DRM_OK,
>
> got %#x.\n", i, hr);
> > -
> >    if (SUCCEEDED(hr))
> > -
> >    {
>
> I'll point out that if you had done a "continue" on failure instead,
> you wouldn't need to change the indentation when you remove the
> todo_wine.