Re: shell32/tests: Fix incorrect usage of broken() function.

[Nikolay Sivov]

On 08.09.2016 12:23, Sebastian Lackner wrote:
> Signed-off-by: Sebastian Lackner
>
> dlls/shell32/tests/shelldispatch.c  |    2 +-
>  dlls/shell32/tests/shfldr_special.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/shell32/tests/shelldispatch.c b/dlls/shell32/tests/shelldispatch.c
> index 9a47c67..70146cb 100644
> --- a/dlls/shell32/tests/shelldispatch.c
> +++ b/dlls/shell32/tests/shelldispatch.c
> @@ -346,7 +346,7 @@ static void test_items(void)
>      ok(r == S_OK, "Folder::Items failed: %08x\n", r);
>      ok(!!items, "items is null\n");
>      r = FolderItems_QueryInterface(items, &IID_FolderItems2, (void**)&items2);
> - ok(r == S_OK || broken(E_NOINTERFACE) /* xp and later */, "FolderItems::QueryInterface failed: %08x\n", r);
> - ok(r == S_OK || broken(r == E_NOINTERFACE) /* xp and later */, "FolderItems::QueryInterface failed: %08x\n", r);

Does it really fail anywhere? I don't see that on test results page.
>  ok(!!items2 || broken(!items2) /* xp and later */, "items2 is null\n");
>  r = FolderItems_QueryInterface(items, &IID_FolderItems3, (void**)&items3);
>  ok(r == S_OK, "FolderItems::QueryInterface failed: %08x\n", r);
>
> diff --git a/dlls/shell32/tests/shfldr_special.c b/dlls/shell32/tests/shfldr_special.c
> index 6d61220..dd4cc04 100644
> --- a/dlls/shell32/tests/shfldr_special.c
> +++ b/dlls/shell32/tests/shfldr_special.c
> @@ -166,7 +166,7 @@ if (0)
>  details.str.u.pOleStr = NULL;
>  hr = IShellFolder2_GetDetailsOf(folder, NULL, 0, &details);
> - ok(hr == S_OK || broken(E_NOTIMPL) /* W2K */, "got 0x%08x\n", hr);
> - ok(hr == S_OK || broken(hr == E_NOTIMPL) /* W2K */, "got 0x%08x\n", hr);
> if (SHELL_OsIsUnicode()) SHFree(details.str.u.pOleStr);

Win2K case could as well be removed.
>  /* test every column if method is implemented */

shell32/tests: Fix incorrect usage of broken() function.

[Sebastian Lackner]

On 08.09.2016 12:19, Nikolay Sivov wrote:
> On 08.09.2016 12:23, Sebastian Lackner wrote:
> > Signed-off-by: Sebastian Lackner
> >
> > dlls/shell32/tests/shelldispatch.c  |    2 +-
> >  dlls/shell32/tests/shfldr_special.c |    2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/dlls/shell32/tests/shelldispatch.c b/dlls/shell32/tests/shelldispatch.c
> > index 9a47c67..70146cb 100644
> > --- a/dlls/shell32/tests/shelldispatch.c
> > +++ b/dlls/shell32/tests/shelldispatch.c
> > @@ -346,7 +346,7 @@ static void test_items(void)
> >      ok(r == S_OK, "Folder::Items failed: %08x\n", r);
> >      ok(!!items, "items is null\n");
> >      r = FolderItems_QueryInterface(items, &IID_FolderItems2, (void**)&items2);
> > - ok(r == S_OK || broken(E_NOINTERFACE) /* xp and later */, "FolderItems::QueryInterface failed: %08x\n", r);
> > - ok(r == S_OK || broken(r == E_NOINTERFACE) /* xp and later */, "FolderItems::QueryInterface failed: %08x\n", r);
>
> Does it really fail anywhere? I don't see that on test results page.

Without the broken() it fails basically everywhere:
https://newtestbot.winehq.org/JobDetails.pl?Key=25676

Please note that I'm not adding the broken(...), it was already there before
but used incorrectly and hiding this test failure. If you have a different
solution in mind, feel free to propose an alternative patch.
> >  ok(!!items2 || broken(!items2) /* xp and later */, "items2 is null\n");
> >  r = FolderItems_QueryInterface(items, &IID_FolderItems3, (void**)&items3);
> >  ok(r == S_OK, "FolderItems::QueryInterface failed: %08x\n", r);
> >
> > diff --git a/dlls/shell32/tests/shfldr_special.c b/dlls/shell32/tests/shfldr_special.c
> > index 6d61220..dd4cc04 100644
> > --- a/dlls/shell32/tests/shfldr_special.c
> > +++ b/dlls/shell32/tests/shfldr_special.c
> > @@ -166,7 +166,7 @@ if (0)
> >  details.str.u.pOleStr = NULL;
> >  hr = IShellFolder2_GetDetailsOf(folder, NULL, 0, &details);
> > - ok(hr == S_OK || broken(E_NOTIMPL) /* W2K */, "got 0x%08x\n", hr);
> > - ok(hr == S_OK || broken(hr == E_NOTIMPL) /* W2K */, "got 0x%08x\n", hr);
> > if (SHELL_OsIsUnicode()) SHFree(details.str.u.pOleStr);
>
> Win2K case could as well be removed.
> >  /* test every column if method is implemented */

shell32/tests: Fix incorrect usage of broken() function.

[Nikolay Sivov]

On 08.09.2016 13:27, Sebastian Lackner wrote:
> On 08.09.2016 12:19, Nikolay Sivov wrote:
> > On 08.09.2016 12:23, Sebastian Lackner wrote:
> > > Signed-off-by: Sebastian Lackner
> > >
> > > dlls/shell32/tests/shelldispatch.c  |    2 +-
> > >  dlls/shell32/tests/shfldr_special.c |    2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/dlls/shell32/tests/shelldispatch.c b/dlls/shell32/tests/shelldispatch.c
> > > index 9a47c67..70146cb 100644
> > > --- a/dlls/shell32/tests/shelldispatch.c
> > > +++ b/dlls/shell32/tests/shelldispatch.c
> > > @@ -346,7 +346,7 @@ static void test_items(void)
> > >      ok(r == S_OK, "Folder::Items failed: %08x\n", r);
> > >      ok(!!items, "items is null\n");
> > >      r = FolderItems_QueryInterface(items, &IID_FolderItems2, (void**)&items2);
> > > - ok(r == S_OK || broken(E_NOINTERFACE) /* xp and later */, "FolderItems::QueryInterface failed: %08x\n", r);
> > > - ok(r == S_OK || broken(r == E_NOINTERFACE) /* xp and later */, "FolderItems::QueryInterface failed: %08x\n", r);
> >
> > Does it really fail anywhere? I don't see that on test results page.
>
> Without the broken() it fails basically everywhere:
> https://newtestbot.winehq.org/JobDetails.pl?Key=25676
>
> Please note that I'm not adding the broken(...), it was already there before
> but used incorrectly and hiding this test failure. If you have a different
> solution in mind, feel free to propose an alternative patch.

No, you're right, broken() was an equivalent of '|| 1' before your
change, so this test could never fail on windows.
> > >  ok(!!items2 || broken(!items2) /* xp and later */, "items2 is null\n");
> > >  r = FolderItems_QueryInterface(items, &IID_FolderItems3, (void**)&items3);
> > >  ok(r == S_OK, "FolderItems::QueryInterface failed: %08x\n", r);
> > >
> > > diff --git a/dlls/shell32/tests/shfldr_special.c b/dlls/shell32/tests/shfldr_special.c
> > > index 6d61220..dd4cc04 100644
> > > --- a/dlls/shell32/tests/shfldr_special.c
> > > +++ b/dlls/shell32/tests/shfldr_special.c
> > > @@ -166,7 +166,7 @@ if (0)
> > >  details.str.u.pOleStr = NULL;
> > >  hr = IShellFolder2_GetDetailsOf(folder, NULL, 0, &details);
> > > - ok(hr == S_OK || broken(E_NOTIMPL) /* W2K */, "got 0x%08x\n", hr);
> > > - ok(hr == S_OK || broken(hr == E_NOTIMPL) /* W2K */, "got 0x%08x\n", hr);
> > > if (SHELL_OsIsUnicode()) SHFree(details.str.u.pOleStr);
> >
> > Win2K case could as well be removed.
> > >  /* test every column if method is implemented */