Re: [ddraw/tests] Fix some test failures on Vista+

[Stefan Dösinger]

Am 10.12.2009 um 13:21 schrieb Paul Vriens:
> <0004-Fix-some-test-failures-on-Vista.patch>
> const char *name;
>         DDPIXELFORMAT fmt;
>         BOOL getdc_capable;
> +        HRESULT alt_result;
>      } testdata[] = {

You are setting the alt_result only for the 4 failing tests. I think C sets the others to 0 because the initializer is static data I think its better to explicitly set it in all other test formats to avoid confusion

[ddraw/tests] Fix some test failures on Vista+

[Paul Vriens]

On 12/10/2009 04:04 PM, Stefan Dösinger wrote:
> Am 10.12.2009 um 13:21 schrieb Paul Vriens:
> > <0004-Fix-some-test-failures-on-Vista.patch>
> > const char *name;
> >          DDPIXELFORMAT fmt;
> >          BOOL getdc_capable;
> > +        HRESULT alt_result;
> >       } testdata[] = {
>
> You are setting the alt_result only for the 4 failing tests. I think C sets the others to 0 because the initializer is static data I think its better to explicitly set it in all other test formats to avoid confusion

Yeah, but that's why I added the extra check for alt_result:

(testdata[i].alt_result && hr == testdata[i].alt_result)

It will look a bit strange adding S_OK (or the likes) for the alternate 
result.

I'm not saying it's common practice in the code but we have multiple of 
these structs where the last one(s) is not set.

-- 
Cheers,

Paul.

[ddraw/tests] Fix some test failures on Vista+

[Stefan Dösinger]

Am 10.12.2009 um 16:19 schrieb Paul Vriens:
> I'm not saying it's common practice in the code but we have multiple of these structs where the last one(s) is not set.
> Ok

[ddraw/tests] Fix some test failures on Vista+

[Paul Vriens]

On 12/10/2009 05:08 PM, Stefan Dösinger wrote:
> Am 10.12.2009 um 16:19 schrieb Paul Vriens:
> > I'm not saying it's common practice in the code but we have multiple of these structs where the last one(s) is not set.
> > Ok

Well, the patch has been committed but the ok() message looks a bit 
strange now. Do you think it makes sense to change getdc_capable to a 
HRESULT and do something like the following instead:

ok(hr == testdata[i].result ||
    testdata[i].alt_result && hr == testdata[i].alt_result,
    "GetDC on a %s surface returned 0x%08\n",
    testdata[i].name, hr);

Default result should be S_OK when capable and DDERR_CANTCREATEDC when not.

One thing I saw by running the above is that we return E_INVALIDARG 
instead of DDERR_CANTCREATEDC when GetDC fails btw.

-- 
Cheers,

Paul.

[ddraw/tests] Fix some test failures on Vista+

[Stefan Dösinger]

Am 10.12.2009 um 17:22 schrieb Paul Vriens:
> Well, the patch has been committed but the ok() message looks a bit strange now. Do you think it makes sense to change getdc_capable to a HRESULT and do something like the following instead:
>
> ok(hr == testdata[i].result ||
>   testdata[i].alt_result && hr == testdata[i].alt_result,
>   "GetDC on a %s surface returned 0x%08\n",
>   testdata[i].name, hr);
> That code makes it a bit more obvious than the last one, but those tests are of pretty little use now. The test essentially says "it can work or it cannot work". We also have a few other tests testing and documenting the return values in specific situations(though I think not in the situation where the format cannot be used with GetDC.

I have no problems if we leave the tests as they are right now. If you want, you could disable the tests entirely, revert the alternative ok() value, and add a separate test that tests what the return value is if a format can't be used with GetDC(but use a format that fails on all windows versions, like DCT1)