Re: [6/6] comdlg32: Add a basic test for IShellView2_CreateViewWindow2.

[Reece Dunn]

2008/10/14 Henri Verbeet <hverbeet(a)codeweavers.com>:
> +            IShellView2_DestroyViewWindow(shell_view2);
> +            ok(SUCCEEDED(hr), "DestroyViewWindow returned %#x\n", hr);

You are missing a "hr = " here, otherwise the ok check is not checking
the return value from DestroyViewWindow.
> +            IShellView2_GetCurrentInfo(shell_view2, &folder_settings);
> +            ok(folder_settings.ViewMode == FVM_LIST, "view mode is %d, expected %d\n", folder_settings.ViewMode, FVM_LIST);

Shouldn't this also be checking the HRESULT return value from
GetCurrentInfo as per the other calls?

General observation: other tests in Wine explicitly check for specific
hr values, is there a reason you have used the generic SUCCEEDED here
instead of that approach?

- Reece

[6/6] comdlg32: Add a basic test for IShellView2_CreateViewWindow2.

[Henri Verbeet]

2008/10/14 Reece Dunn <msclrhd(a)googlemail.com>:
> 2008/10/14 Henri Verbeet <hverbeet(a)codeweavers.com>:
> > +            IShellView2_DestroyViewWindow(shell_view2);
> > +            ok(SUCCEEDED(hr), "DestroyViewWindow returned %#x\n", hr);
>
> You are missing a "hr = " here, otherwise the ok check is not checking
> the return value from DestroyViewWindow.
> > +            IShellView2_GetCurrentInfo(shell_view2, &folder_settings);
> > +            ok(folder_settings.ViewMode == FVM_LIST, "view mode is %d, expected %d\n", folder_settings.ViewMode, FVM_LIST);
>
> Shouldn't this also be checking the HRESULT return value from
> GetCurrentInfo as per the other calls?

You're right, I've sent an updated patch.
> General observation: other tests in Wine explicitly check for specific
> hr values, is there a reason you have used the generic SUCCEEDED here
> instead of that approach?

I don't care about specific return values here.

[6/6] comdlg32: Add a basic test for IShellView2_CreateViewWindow2.

[Juan Lang]

> General observation: other tests in Wine explicitly check for specific
> hr values, is there a reason you have used the generic SUCCEEDED here
> instead of that approach?

In general we don't care what the return value on success is.
Applications sometimes sometimes do check for particular failure
conditions, but particular success conditions aren't checked nearly as
often.
--Juan