Re: [PATCH v2] msxml3/test: Add XMLParser Tests

[Nikolay Sivov]

On 02.08.2016 11:02, Alistair Leslie-Hughes wrote:
> With Signed-off.
>
> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>
> ---
>  dlls/msxml3/tests/xmlparser.c | 196 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 188 insertions(+), 8 deletions(-)
>
> diff --git a/dlls/msxml3/tests/xmlparser.c b/dlls/msxml3/tests/xmlparser.c
> index 32514e3..bf896e4 100644
> --- a/dlls/msxml3/tests/xmlparser.c
> +++ b/dlls/msxml3/tests/xmlparser.c
> @@ -26,8 +26,45 @@
>  #include "windows.h"
>  #include "ole2.h"
>  #include "xmlparser.h"
> +#include "rpcproxy.h"
>  #include "wine/test.h"

Why do you need this?
> +
> +static HRESULT hresult_NotifyEvent = S_OK;

Please move this global variable to factory instance.
> static HRESULT WINAPI nodefact_QueryInterface(IXMLNodeFactory *iface,
>          REFIID riid, void **ppvObject)
> @@ -54,34 +91,67 @@ static ULONG WINAPI nodefact_Release(IXMLNodeFactory *iface)
>  }
>
> static HRESULT WINAPI nodefact_NotifyEvent(IXMLNodeFactory *iface,
> -        IXMLNodeSource *pSource, XML_NODEFACTORY_EVENT iEvt)
> +        IXMLNodeSource *source, XML_NODEFACTORY_EVENT event)
>  {
> -    return E_NOTIMPL;
> +    switch(event)
> +    {
> +        case XMLNF_STARTDOCUMENT:
> +            CHECK_EXPECT(NotifyStartDocument);
> +            trace("NotifyEvent: XMLNF_STARTDOCUMENT\n");
> +            break;
> +        case XMLNF_ENDPROLOG:
> +            CHECK_EXPECT(NotifyEndProlog);
> +            trace("NotifyEvent: XMLNF_ENDPROLOG\n");
> +            break;
> +        case XMLNF_ENDDOCUMENT:
> +            CHECK_EXPECT(NotifyEndDocument);
> +            trace("NotifyEvent: XMLNF_ENDDOCUMENT\n");
> +            break;
> +        case XMLNF_DATAAVAILABLE:
> +            CHECK_EXPECT(NotifyDataAvailable);
> +            trace("NotifyEvent: XMLNF_DATAAVAILABLE\n");
> +            break;
> +        default:
> +            ok(0, "Unsupported NotifyEvent %p %d\n", source, event);
> +            return E_FAIL;
> +    }
> +
> +    return hresult_NotifyEvent;
>  }

Arguably trace()'s are not really useful in this context, and will
appear on every parsing test.
> static HRESULT WINAPI nodefact_BeginChildren(IXMLNodeFactory *iface,
>          IXMLNodeSource *pSource, XML_NODE_INFO *pNodeInfo)
>  {
> -    return E_NOTIMPL;
> +    trace("nodefact_BeginChildren %p %p %d\n", pSource, pNodeInfo,
> +                pNodeInfo ? pNodeInfo->dwType : 0);
> +    return S_OK;
>  }
>
> static HRESULT WINAPI nodefact_EndChildren(IXMLNodeFactory *iface,
>          IXMLNodeSource *pSource, BOOL fEmpty, XML_NODE_INFO *pNodeInfo)
>  {
> -    return E_NOTIMPL;
> +    trace("nodefact_EndChildren %p %d %p %d\n", pSource, fEmpty, pNodeInfo,
> +                pNodeInfo ? pNodeInfo->dwType : 0);
> +    return S_OK;
>  }
>
> static HRESULT WINAPI nodefact_Error(IXMLNodeFactory *iface,
>          IXMLNodeSource *pSource, HRESULT hrErrorCode, USHORT cNumRecs,
>          XML_NODE_INFO **ppNodeInfo)
>  {
> -    return E_NOTIMPL;
> +    CHECK_EXPECT(FactoryError);
> +    trace("nodefact_Error %p %08x %d %p\n", pSource, hrErrorCode, cNumRecs, ppNodeInfo);
> +    return S_OK;
>  }
>
> -static HRESULT WINAPI nodefact_CreateNode(IXMLNodeFactory *iface, IXMLNodeSource *pSource,
> -        PVOID pNodeParent, USHORT cNumRecs, XML_NODE_INFO **ppNodeInfo)
> +static HRESULT WINAPI nodefact_CreateNode(IXMLNodeFactory *iface, IXMLNodeSource *source,
> +        void *parent, USHORT recs, XML_NODE_INFO **nodeinfo)
>  {
> -    return E_NOTIMPL;
> +    trace("nodefact_CreateNode %p %p %d %p\n", source, parent, recs, nodeinfo);
> +
> +    *nodeinfo = NULL;
> +
> +    return S_OK;
>  }
>
> static const IXMLNodeFactoryVtbl nodefactoryVtbl =
> @@ -150,6 +220,114 @@ static void create_test(void)
>      IXMLParser_Release(parser);
>  }
>
> +static void parse_documnet(void)
> +{
> +    HRESULT hr;
> +    IXMLParser *parser;
> +    IStream* stream = NULL;
> +    HGLOBAL hMem;
> +    LPBYTE bytes;
> +
> +    hr = CoCreateInstance(&CLSID_XMLParser30, NULL, CLSCTX_INPROC_SERVER, &IID_IXMLParser, (void**)&parser);
> +    if (FAILED(hr))
> +    {
> +        win_skip("IXMLParser is not available (0x%08x)\n", hr);
> +        return;
> +    }
> +
> +    hMem = GlobalAlloc(GMEM_MOVEABLE, strlen(empty_document));
> +    bytes = GlobalLock(hMem);
> +    memcpy(bytes, empty_document, strlen(empty_document));
> +    GlobalUnlock(hMem);
> +
> +    CreateStreamOnHGlobal(hMem, TRUE, &stream);
> +
> +    hr = IXMLParser_SetFactory(parser, &thenodefactory);
> +    ok(hr == S_OK, "Expected S_OK got 0x%08x\n", hr);
> +
> +    hr = IXMLParser_SetInput(parser, (IUnknown *)stream);
> +    ok(hr == S_OK, "Expected S_OK got 0x%08x\n", hr);
> +
> +    hr = IXMLParser_GetParserState(parser);
> +    ok(hr == XMLPARSER_IDLE, "got 0x%08x\n", hr);
> +
> +    SET_EXPECT(NotifyStartDocument);
> +    SET_EXPECT(NotifyEndDocument);
> +    SET_EXPECT(NotifyEndProlog);
> +    SET_EXPECT(NotifyDataAvailable);
> +    hr = IXMLParser_Run(parser, -1);
> +    todo_wine ok(hr == S_OK, "Expected S_OK got 0x%08x\n", hr);
> +    todo_wine CHECK_CALLED(NotifyStartDocument);
> +    todo_wine CHECK_CALLED(NotifyEndDocument);
> +    todo_wine CHECK_CALLED(NotifyEndProlog);
> +    todo_wine CHECK_CALLED(NotifyDataAvailable);

Main problem is that this does not test actual callback order - actual
ordering is

- start document;
- create node;
- end children;
- end prolog;
- data available;
- end document;

Please use test sequences facility saxreader.c is using - add_call(),
ok_sequence(), etc.
> START_TEST(xmlparser)
>  {
>      HRESULT hr;
> @@ -160,6 +338,8 @@ START_TEST(xmlparser)
>          return;
>
> create_test();
> +    parse_documnet();
> +    parse_documnet_error();

I guess it was meant to be 'document'.
> CoUninitialize();
>  }