On 1/20/11 6:40 PM, Erich Hoover wrote:
On Thu, Jan 20, 2011 at 4:00 AM, Jacek Caban<ja...@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
