[PATCH 5/5] windows.media.speech/tests: Add basic tests for activation factory ifaces.

[Rémi Bernon]

Hi Bernhard,


On 3/5/22 20:54, Bernhard Kölbl wrote:
> To test, if the ifaces are static and if each activatable class
> has it's own activation factory.
> 
> Signed-off-by: Bernhard Kölbl <besentv at gmail.com>
> ---
>   dlls/windows.media.speech/tests/speech.c | 120 +++++++++++++++++++++++
>   1 file changed, 120 insertions(+)
> 
> diff --git a/dlls/windows.media.speech/tests/speech.c b/dlls/windows.media.speech/tests/speech.c
> index 38fd5c90dd0..0d9564d4a65 100644
> --- a/dlls/windows.media.speech/tests/speech.c
> +++ b/dlls/windows.media.speech/tests/speech.c
> @@ -29,6 +29,8 @@
>   #define WIDL_using_Windows_Foundation
>   #define WIDL_using_Windows_Foundation_Collections
>   #include "windows.foundation.h"
> +#define WIDL_using_Windows_Media_SpeechRecognition
> +#include "windows.media.speechrecognition.h"
>   #define WIDL_using_Windows_Media_SpeechSynthesis
>   #include "windows.media.speechsynthesis.h"
>   
> @@ -36,6 +38,123 @@
>   
>   HRESULT WINAPI (*pDllGetActivationFactory)(HSTRING, IActivationFactory **);
>   
> +static void test_ActivationFactory(void)
> +{
> +    static const WCHAR *synthesizer_name = L"Windows.Media.SpeechSynthesis.SpeechSynthesizer";
> +    static const WCHAR *recognizer_name = L"Windows.Media.SpeechRecognition.SpeechRecognizer";
> +    IInspectable *inspectable = NULL, *inspectable2 = NULL;
> +    IAgileObject *agile_object = NULL, *agile_object2 = NULL;
> +    IActivationFactory *afactory = NULL, *afactory2 = NULL, *afactory3 = NULL;
> +    ISpeechRecognizerFactory *sr_factory = NULL;
> +    ISpeechRecognizerStatics *sr_statics = NULL;
> +    ISpeechRecognizerStatics2 *sr_statics2 = NULL;


Regarding names, it doesn't matter too much but I'd rename "afactoryX" 
to "factoryX" instead. The "sr_" prefix is also a little bit cryptic, 
and "recognizer_" looks short enough and more readable.


> +    IInstalledVoicesStatic *installed_voices = NULL;
> +    HSTRING str, str2;
> +    HRESULT hr;
> +    ULONG ref;
> +
> +    hr = RoInitialize(RO_INIT_MULTITHREADED);
> +    ok(hr == S_OK, "RoInitialize failed, hr %#x.\n", hr);
> +
> +    hr = WindowsCreateString(synthesizer_name, wcslen(synthesizer_name), &str);
> +    ok(hr == S_OK, "WindowsCreateString failed, hr %#x.\n", hr);
> +
> +    hr = WindowsCreateString(recognizer_name, wcslen(recognizer_name), &str2);
> +    ok(hr == S_OK, "WindowsCreateString failed, hr %#x.\n", hr);
> +
> +    hr = RoGetActivationFactory(str, &IID_IActivationFactory, (void **)&afactory);
> +    ok(hr == S_OK, "RoGetActivationFactory failed, hr %#x.\n", hr);
> +


Unless you need the strings for some reason, I think it's shorter to 
just do WindowsCreateString, RoGetActivationFactory, WindowsDeleteString 
in sequence, you will only need one HSTRING variable.


> +    ref = IActivationFactory_AddRef(afactory);
> +    ok(ref == 3, "Got unexpected refcount: %u.\n", ref);
> +
> +    ref = IActivationFactory_Release(afactory);
> +    ok(ref == 2, "Got unexpected refcount: %u.\n", ref);
> +
> +    hr = IActivationFactory_QueryInterface(afactory, &IID_ISpeechRecognizerFactory, (void**)&sr_factory);
> +    ok(hr == E_NOINTERFACE, "IActivationFactory_QueryInterface failed, hr %#x.\n", hr);
> +
> +    hr = IActivationFactory_QueryInterface(afactory, &IID_IInspectable, (void**)&inspectable);
> +    ok(hr == S_OK, "IActivationFactory_QueryInterface failed, hr %#x.\n", hr);
> +
> +    hr = IActivationFactory_QueryInterface(afactory, &IID_IAgileObject, (void**)&agile_object);
> +    ok(hr == S_OK, "IActivationFactory_QueryInterface failed, hr %#x.\n", hr);
> +
> +    hr = IActivationFactory_QueryInterface(afactory, &IID_IInstalledVoicesStatic, (void**)&installed_voices);
> +    ok(hr == S_OK, "IActivationFactory_QueryInterface failed, hr %#x.\n", hr);
> +
> +    ref = IInstalledVoicesStatic_Release(installed_voices);
> +    ok(ref == 4, "Got unexpected refcount: %u.\n", ref);
> +
> +    ref = IAgileObject_Release(agile_object);
> +    ok(ref == 3, "Got unexpected refcount: %u.\n", ref);
> +
> +    ref = IInspectable_Release(inspectable);
> +    ok(ref == 2, "Got unexpected refcount: %u.\n", ref);
> +


I'm not sure checking the refcount extensively is very useful, I know I 
probably did it for the stubs, to check how things were supposed to 
work, but maybe now we have a better idea it's not necessary to do it 
for every factory.

To check the available interfaces it's shorter and convenient to use the 
check_interface test helper that's copied around, just copy whichever 
version of if you like best there too and you can then check the 
interfaces you want easily.

If you really want to check refcount, there's also an "EXPECT_REF" or 
"check_ref" helpers lying around as well, which may do it in a shorter way.


> +    hr = RoGetActivationFactory(str, &IID_IActivationFactory, (void **)&afactory2);
> +    ok(hr == S_OK, "RoGetActivationFactory failed, hr %#x.\n", hr);
> +    ok(afactory == afactory2, "Factories pointed at afactory %p afactory2 %p.\n", afactory, afactory2);
> +
> +    hr = RoGetActivationFactory(str2, &IID_IActivationFactory, (void **)&afactory3);
> +    todo_wine ok(hr == S_OK || broken(hr == REGDB_E_CLASSNOTREG), "RoGetActivationFactory failed, hr %#x.\n", hr);
> +
> +    if(hr == S_OK) /* Win10+ only */
> +    {


Instead of a big if like this, I'd suggest to have a done label and 
"goto done" here. Anything that doesn't require this factory can be 
tested before. If it makes things more complicated, maybe a dedicated 
test function for this factory is better.

Freeing the str before checking hr, as suggested above also saves you 
the need of handling the remaining HSTRING at the bottom.


> +        ok(afactory != afactory3, "Factories pointed at afactory %p afactory3 %p.\n", afactory, afactory3);
> +
> +        hr = IActivationFactory_QueryInterface(afactory3, &IID_IInspectable, (void**)&inspectable2);
> +        ok(hr == S_OK, "IActivationFactory_QueryInterface failed, hr %#x.\n", hr);
> +
> +        hr = IActivationFactory_QueryInterface(afactory3, &IID_IAgileObject, (void**)&agile_object2);
> +        ok(hr == S_OK, "IActivationFactory_QueryInterface failed, hr %#x.\n", hr);
> +
> +        hr = IActivationFactory_QueryInterface(afactory3, &IID_ISpeechRecognizerFactory, (void**)&sr_factory);
> +        ok(hr == S_OK, "IActivationFactory_QueryInterface failed, hr %#x.\n", hr);
> +
> +        hr = IActivationFactory_QueryInterface(afactory3, &IID_ISpeechRecognizerStatics, (void**)&sr_statics);
> +        ok(hr == S_OK, "IActivationFactory_QueryInterface failed, hr %#x.\n", hr);
> +
> +        ref = ISpeechRecognizerStatics_Release(sr_statics);
> +        ok(ref == 5, "Got unexpected refcount: %u.\n", ref);
> +
> +        ref = ISpeechRecognizerFactory_Release(sr_factory);
> +        ok(ref == 4, "Got unexpected refcount: %u.\n", ref);
> +
> +        ref = IAgileObject_Release(agile_object2);
> +        ok(ref == 3, "Got unexpected refcount: %u.\n", ref);
> +
> +        ref = IInspectable_Release(inspectable2);
> +        ok(ref == 2, "Got unexpected refcount: %u.\n", ref);
> +
> +        hr = IActivationFactory_QueryInterface(afactory3, &IID_ISpeechRecognizerStatics2, (void**)&sr_statics2);
> +        ok(hr == S_OK || broken(hr == E_NOINTERFACE), "IActivationFactory_QueryInterface failed, hr %#x.\n", hr);
> +
> +        if(hr == S_OK) /* ISpeechRecognizerStatics2 not available in Win10 1507 */
> +        {
> +            ref = ISpeechRecognizerStatics2_Release(sr_statics2);
> +            ok(ref == 2, "Got unexpected refcount: %u.\n", ref);
> +        }
> +


Again, I think checking refs everywhere makes this a little bit too 
verbose. If you want to assert there's no leak, doing it only when it's 
expected to reach 0 could be interesting and better. For static objects 
I don't think it's really useful.


> +        hr = IActivationFactory_QueryInterface(afactory3, &IID_IInstalledVoicesStatic, (void**)&installed_voices);
> +        ok(hr == E_NOINTERFACE, "IActivationFactory_QueryInterface failed, hr %#x.\n", hr);
> +
> +        ref = IActivationFactory_Release(afactory3);
> +        ok(ref == 1, "Got unexpected refcount: %u.\n", ref);
> +    }
> +
> +    ref = IActivationFactory_Release(afactory2);
> +    ok(ref == 2, "Got unexpected refcount: %u.\n", ref);
> +
> +    ref = IActivationFactory_Release(afactory);
> +    ok(ref == 1, "Got unexpected refcount: %u.\n", ref);
> +
> +    WindowsDeleteString(str);
> +    WindowsDeleteString(str2);
> +
> +    RoUninitialize();
> +}
> +
>   static void test_SpeechSynthesizer(void)
>   {
>       static const WCHAR *speech_synthesizer_name = L"Windows.Media.SpeechSynthesis.SpeechSynthesizer";
> @@ -203,6 +322,7 @@ static void test_VoiceInformation(void)
>   
>   START_TEST(speech)
>   {
> +    test_ActivationFactory();
>       test_SpeechSynthesizer();
>       test_VoiceInformation();
>   }


Cheers,
-- 
Rémi Bernon <rbernon at codeweavers.com>