[PATCH 5/5] adsldp/tests: Add some tests for LDAPNamespace.

[Dmitry Timoshkov]

Signed-off-by: Dmitry Timoshkov 
---
 dlls/adsldp/tests/Makefile.in |  1 +
 dlls/adsldp/tests/ldap.c      | 76 +++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)
 create mode 100644 dlls/adsldp/tests/ldap.c

diff --git a/dlls/adsldp/tests/Makefile.in b/dlls/adsldp/tests/Makefile.in
index 777d2aa9de..52141139d5 100644
--- a/dlls/adsldp/tests/Makefile.in
+++ b/dlls/adsldp/tests/Makefile.in
@@ -2,4 +2,5 @@ TESTDLL   = adsldp.dll
 IMPORTS   = ole32 oleaut32 secur32 advapi32 uuid

C_SRCS = \
+	ldap.c \
    sysinfo.c
diff --git a/dlls/adsldp/tests/ldap.c b/dlls/adsldp/tests/ldap.c
new file mode 100644
index 0000000000..9e4dc2c1eb
--- /dev/null
+++ b/dlls/adsldp/tests/ldap.c
@@ -0,0 +1,76 @@
+/*
+ * LDAPNamespace Tests
+ *
+ * Copyright 2019 Dmitry Timoshkov
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#include <stdarg.h>
+#include <stdio.h>
+
+#define COBJMACROS
+
+#include "windef.h"
+#include "winbase.h"
+#include "objbase.h"
+#include "iads.h"
+
+#include "wine/test.h"
+
+#include "initguid.h"
+DEFINE_GUID(CLSID_LDAPNamespace,0x228d9a82,0xc302,0x11cf,0x9a,0xa4,0x00,0xaa,0x00,0x4a,0x56,0x91);
+
+static void test_LDAP(void)
+{
+    HRESULT hr;
+    IUnknown *unk;
+    IADs *ads;
+    IADsOpenDSObject *ads_open;
+    IDispatch *disp;
+
+    hr = CoCreateInstance(&CLSID_LDAPNamespace, 0, CLSCTX_INPROC_SERVER, &IID_IADs, (void **)&ads);
+    ok(hr == S_OK, "got %#x\n", hr);
+    IADs_Release(ads);
+
+    hr = CoCreateInstance(&CLSID_LDAPNamespace, 0, CLSCTX_INPROC_SERVER, &IID_IUnknown, (void **)&unk);
+    ok(hr == S_OK, "got %#x\n", hr);
+
+    hr = IUnknown_QueryInterface(unk, &IID_IDispatch, (void **)&disp);
+    ok(hr == S_OK, "got %#x\n", hr);
+    IDispatch_Release(disp);
+
+    hr = IUnknown_QueryInterface(unk, &IID_IADsOpenDSObject, (void **)&ads_open);
+    ok(hr == S_OK, "got %#x\n", hr);
+    IADsOpenDSObject_Release(ads_open);
+
+    hr = IADsOpenDSObject_OpenDSObject(ads_open, (BSTR)L"LDAP:", NULL, NULL, ADS_SECURE_AUTHENTICATION, &disp);
+todo_wine
+    ok(hr == S_OK, "got %#x\n", hr);
+if (hr == S_OK)
+    IDispatch_Release(disp);
+
+    IUnknown_Release(unk);
+}
+
+START_TEST(ldap)
+{
+    HRESULT hr;
+
+    hr = CoInitialize(NULL);
+    ok(hr == S_OK, "got %#x\n", hr);
+
+    test_LDAP();
+}

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=62210

Your paranoid android.

=== debian10 (32 bit report) ===

adsldp:
ldap.c:45: Test failed: got 0x80040154
Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x004015c0).

Report errors:
adsldp:ldap crashed (c0000005)

=== debian10 (32 bit French report) ===

adsldp:
ldap.c:45: Test failed: got 0x80040154
Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x004015c0).

Report errors:
adsldp:ldap crashed (c0000005)

=== debian10 (32 bit Japanese:Japan report) ===

adsldp:
ldap.c:45: Test failed: got 0x80040154
Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x004015c0).

Report errors:
adsldp:ldap crashed (c0000005)

=== debian10 (32 bit Chinese:China report) ===

adsldp:
ldap.c:45: Test failed: got 0x80040154
Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x004015c0).

Report errors:
adsldp:ldap crashed (c0000005)

=== debian10 (32 bit WoW report) ===

adsldp:
ldap.c:45: Test failed: got 0x80040154
Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x004015c0).

Report errors:
adsldp:ldap crashed (c0000005)

=== debian10 (64 bit WoW report) ===

adsldp:
ldap.c:45: Test failed: got 0x80040154
Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x004015c0).

Report errors:
adsldp:ldap crashed (c0000005)

[Dmitry Timoshkov]

Marvin  wrote:
> === debian10 (32 bit report) ===
>
> adsldp:
> ldap.c:45: Test failed: got 0x80040154
> Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x004015c0).
>
> Report errors:
> adsldp:ldap crashed (c0000005)

testbot is broken and doesn't update Wine prefix.

[Francois Gouget]

On Mon, 16 Dec 2019, Dmitry Timoshkov wrote:
> Marvin  wrote:
> > === debian10 (32 bit report) ===
> >
> > adsldp:
> > ldap.c:45: Test failed: got 0x80040154
> > Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x004015c0).
> >
> > Report errors:
> > adsldp:ldap crashed (c0000005)
>
> testbot is broken and doesn't update Wine prefix.

The TestBot recreates the WinePrefixes from scratch with the daily Wine 
commits:

https://source.winehq.org/git/tools.git/blob/HEAD:/testbot/bin/build/WineRec...

But it does not recreate the wineprefixes from scratch with each patch. 
For those it just runs Wine which is supposed to update the wineprefix 
automatically.

Not updating the wineprefix automatically would force the users to 
delete and recreate their wineprefixes, including reinstalling 
applications, every time they upgrade Wine, which I would argue is a 
bug.

[Zebediah Figura]

On 12/16/19 6:10 AM, Francois Gouget wrote:
> On Mon, 16 Dec 2019, Dmitry Timoshkov wrote:
> > Marvin  wrote:
> > > === debian10 (32 bit report) ===
> > >
> > > adsldp:
> > > ldap.c:45: Test failed: got 0x80040154
> > > Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x004015c0).
> > >
> > > Report errors:
> > > adsldp:ldap crashed (c0000005)
> >
> > testbot is broken and doesn't update Wine prefix.
>
> The TestBot recreates the WinePrefixes from scratch with the daily Wine 
> commits:
>
> https://source.winehq.org/git/tools.git/blob/HEAD:/testbot/bin/build/WineRec...
>
> But it does not recreate the wineprefixes from scratch with each patch. 
> For those it just runs Wine which is supposed to update the wineprefix 
> automatically.
>
> Not updating the wineprefix automatically would force the users to 
> delete and recreate their wineprefixes, including reinstalling 
> applications, every time they upgrade Wine, which I would argue is a 
> bug.

I'd agree. Unfortunately, we don't have a way of automatically detecting
when a new WINE_REGISTRY resource is added to an existing DLL. I'm not
sure what would be a good solution there (manually updating wine.inf.in?)

[Alexandre Julliard]

Zebediah Figura  writes:
> On 12/16/19 6:10 AM, Francois Gouget wrote:
> > On Mon, 16 Dec 2019, Dmitry Timoshkov wrote:
> > > Marvin  wrote:
> > > > === debian10 (32 bit report) ===
> > > >
> > > > adsldp:
> > > > ldap.c:45: Test failed: got 0x80040154
> > > > Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x004015c0).
> > > >
> > > > Report errors:
> > > > adsldp:ldap crashed (c0000005)
> > >
> > > testbot is broken and doesn't update Wine prefix.
> >
> > The TestBot recreates the WinePrefixes from scratch with the daily Wine 
> > commits:
> >
> > https://source.winehq.org/git/tools.git/blob/HEAD:/testbot/bin/build/WineRec...
> >
> > But it does not recreate the wineprefixes from scratch with each patch. 
> > For those it just runs Wine which is supposed to update the wineprefix 
> > automatically.
> >
> > Not updating the wineprefix automatically would force the users to 
> > delete and recreate their wineprefixes, including reinstalling 
> > applications, every time they upgrade Wine, which I would argue is a 
> > bug.
>
> I'd agree. Unfortunately, we don't have a way of automatically detecting
> when a new WINE_REGISTRY resource is added to an existing DLL. I'm not
> sure what would be a good solution there (manually updating wine.inf.in?)

wine.inf is updated on every release, so it's not an issue for users.
Developers are expected to be able to figure out such issues and update
manually when necessary (which happens rarely in practice). The testbot
could probably update on every patch, or update and retry when a test
failed.

[Sven Baars]

On 16-12-2019 02:42, Dmitry Timoshkov wrote:
> -
>
> +START_TEST(ldap)
> +{
> - HRESULT hr;
> -
> - hr = CoInitialize(NULL);
> - ok(hr == S_OK, "got %#x\n", hr);
> -
> - test_LDAP();
>
> +}

I think you forgot to call CoUninitialize here.

[Dmitry Timoshkov]

Sven Baars  wrote:
> > +START_TEST(ldap)
> > +{
> > - HRESULT hr;
> > -
> > - hr = CoInitialize(NULL);
> > - ok(hr == S_OK, "got %#x\n", hr);
> > -
> > - test_LDAP();
> >
> > +}
>
> I think you forgot to call CoUninitialize here.

The process is going to terminate anyway, so it's a waste of time.

[Sven Baars]

On 16-12-2019 15:05, Dmitry Timoshkov wrote:
> Sven Baars  wrote:
> > > +START_TEST(ldap)
> > > +{
> > > - HRESULT hr;
> > > -
> > > - hr = CoInitialize(NULL);
> > > - ok(hr == S_OK, "got %#x\n", hr);
> > > -
> > > - test_LDAP();
> > >
> > > +}
> >
> > I think you forgot to call CoUninitialize here.
>
> The process is going to terminate anyway, so it's a waste of time.

In that case, you might as well remove all *_Release calls from your
tests...

[Dmitry Timoshkov]

Sven Baars  wrote:
> On 16-12-2019 15:05, Dmitry Timoshkov wrote:
> > Sven Baars  wrote:
> > > > +START_TEST(ldap)
> > > > +{
> > > > - HRESULT hr;
> > > > -
> > > > - hr = CoInitialize(NULL);
> > > > - ok(hr == S_OK, "got %#x\n", hr);
> > > > -
> > > > - test_LDAP();
> > > >
> > > > +}
> > >
> > > I think you forgot to call CoUninitialize here.
> >
> > The process is going to terminate anyway, so it's a waste of time.
>
> In that case, you might as well remove all *_Release calls from your
> tests...

*_Release calls in the test help to ensure that reference counting
works correctly, and I do check that when looking at the logs. However
calling CoUninitialize() is meaningless for the test results.