[PATCH 2/4] shdocvw: Implement IWebBrowser_ExecWB.

[Jacek Caban]

On 1/20/11 6:40 PM, Erich Hoover wrote:
> On Thu, Jan 20, 2011 at 4:00 AM, Jacek Caban<jacek at codeweavers.com>  wrote:
>> ...
>> +    ok(status&   success_flag, "OLECMDID_STOP not enabled/supported: %08x\n", status);
>> You could test the exact value here: ok(status == ...)
> I was trying to avoid adding some conditional todo_wine calls

Hiding differences between Wine and Windows in tests is the worst thing 
you can do. When one is trying to find a bug, tested parts of 
implementation are much less suspicious than not or poorly tested. If 
you know about more such things in the patch, please fix them. You don't 
write the test to pass on your implementation. You write it to know the 
right implementation.

>   (for some reason native does not respond with "supported", even though it
> does in the mshtml tests), please see if you like the attached better.

Or it proves that it's not a simple forward to mshtml?

>> test_CommandTargetPassthru(TRUE) could go to existing test_WebBrowser.
> Would it not be better to just keep these all in one place, since for
> the tests without the custom target it's necessary to create and
> initialize a web-browser?

We still have them in separated functions you've called  
test_QueryStatusWB and test_ExecWB. We most likely will want more tests 
on WebBrowser instance without container's IOleCommandTarget, and it 
would be nice if we could reuse test_CommandTargetPassthru for that 
(well, the name would be better more generic as well).

>> +    if (!target)
>> +        return E_FAIL;
>> A test for this case would be nice. Also with this patch, testing ExecEB shouldn't be too hard, let's test it as well.
> Ugg, apparently MSDN lies horribly - it's actually E_UNEXPECTED.

That's why we need tests :)


Jacek