Re: [1/2] ole32/tests: Add a test for OleSave() called for a IOleCache storage.

[Huw Davies]

On Thu, Oct 19, 2017 at 01:06:50PM +0800, Dmitry Timoshkov wrote:
> Signed-off-by: Dmitry Timoshkov
>
> dlls/ole32/tests/ole2.c | 214 +++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 202 insertions(+), 12 deletions(-)

This looks ok, but could you please call IPersistStorage_Save()
directly rather than using the OleSave() helper?  Also fix
the function name to reflect that.

Thanks,
Huw.

[1/2] ole32/tests: Add a test for OleSave() called for a IOleCache storage.

[Dmitry Timoshkov]

Huw Davies  wrote:
> This looks ok, but could you please call IPersistStorage_Save()
> directly rather than using the OleSave() helper?  Also fix
> the function name to reflect that.

I was considering that, but using OleSave instead of IPersistStorage_Save
simplifies the test, saves quite a bit of typing and helps the test avoid
unnecessary details. I'd really prefer keeping to use OleSave() in these
tests.

[1/2] ole32/tests: Add a test for OleSave() called for a IOleCache storage.

[Huw Davies]

On Thu, Oct 19, 2017 at 05:53:27PM +0800, Dmitry Timoshkov wrote:
> Huw Davies  wrote:
> > This looks ok, but could you please call IPersistStorage_Save()
> > directly rather than using the OleSave() helper?  Also fix
> > the function name to reflect that.
>
> I was considering that, but using OleSave instead of IPersistStorage_Save
> simplifies the test, saves quite a bit of typing and helps the test avoid
> unnecessary details. I'd really prefer keeping to use OleSave() in these
> tests.

How's that?  The Storage_Commit() hunk could go for one.

The point is though, that this should really test the data
cache's IPersitStorge_Save() implementation, not what some
higher level helper is doing.

Huw.