mmdevapi/tests: Add format tests.

Andrew Eikum aeikum at codeweavers.com
Fri Oct 7 09:16:02 CDT 2011


Hi Jörg,

Thanks for adding these tests. It would have been nicer to split the
patch up into discrete chunks. That would make the patch much easier
to review. Overall it looks pretty good. Some remarks follow...

> BTW, I've not touched
> render.c:223: Test failed: IsFormatSupported(0xffffffff) call returns 8889000e
> capture.ok has a similar random failure.  I believe that test is bogus.  It
> should avoid ending up calling the exclusive mode "audio endpoint" and hope/show
> that the shared mode mixer aka. "audio engine" reports E_INVALIDARG.  Perhaps
> using ~AUDCLNT_SHAREMODE_EXCLUSIVE is a means to enforce this, even though
> that's actually an enum, not a flag.

Yeah, that's a pretty uninteresting test. If it's flaky, I don't see
any compelling reason to keep it around.

> +    unk = (void*)0xDEADF00D;
>      hr = IAudioClient_GetService(ac, &IID_IAudioStreamVolume, (void**)&unk);
>      ok(hr == AUDCLNT_E_NOT_INITIALIZED, "Uninitialized GetService call returns %08x\n", hr);
> +    todo_wine ok(broken(unk != NULL), "GetService %08x &out pointer %p\n", hr, unk);

This test is strange. broken() basically means "should fail on
Windows, should not fail in Wine." It's used when some particular
version of Windows does something unexpected which we are uninterested
in duplicating. Do you expect unk to always be non-NULL? If so, then
just test for that without the broken() macro.

>  
> +static void test_formats(AUDCLNT_SHAREMODE mode)

The code in this function is very difficult to follow. I wonder if it
couldn't be written more clearly with explicit conditions and a bit
more code duplication.

> +        ok((hr == S_FALSE)^(pwfx2 == NULL), "hr %x<->suggest %p\n", hr, pwfx2);
> +        if (pwfx2 == (WAVEFORMATEX*)0xDEADF00D) /* bug in Wine */
> +            pwfx2 = NULL;

Would be nicer to both fix the bug in Wine and have a test showing the
correct behavior.

> @@ -1378,7 +1535,7 @@ START_TEST(render)
>      hr = CoCreateInstance(&CLSID_MMDeviceEnumerator, NULL, CLSCTX_INPROC_SERVER, &IID_IMMDeviceEnumerator, (void**)&mme);
>      if (FAILED(hr))
>      {
> -        skip("mmdevapi not available: 0x%08x\n", hr);
> +        win_skip("mmdevapi not available: 0x%08x\n", hr);

If the user has an invalid driver, CoCreateInstance() will fail to
create the enumerator object. I think that's valid behavior, so we
shouldn't fail.

Andrew



More information about the wine-devel mailing list