On 09/16/11 15:25, Francois Gouget wrote:
This fixes this test on the WNT4WSSP6 testbot machine. It also helps on my NT4 VM but one would not notice because there's no ieframes dll so WineTest does not even try to run this test!
dlls/ieframe/tests/webbrowser.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/dlls/ieframe/tests/webbrowser.c b/dlls/ieframe/tests/webbrowser.c index a36c134..0506be6 100644 --- a/dlls/ieframe/tests/webbrowser.c +++ b/dlls/ieframe/tests/webbrowser.c @@ -212,7 +212,10 @@ static HRESULT _create_WebBrowser(unsigned line, IUnknown **unk)
hres = CoCreateInstance(&CLSID_WebBrowser, NULL, CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER, &IID_IUnknown, (void**)unk);
- ok_(__FILE__,line)(hres == S_OK, "Creating WebBrowser object failed: %08x\n", hres);
- if (hres == REGDB_E_CLASSNOTREG)
win_skip_(__FILE__,line)("WebBrowser is not supported\n");- else
ok_(__FILE__,line)(hres == S_OK, "Creating WebBrowser object failed: %08x\n", hres); return hres;
The failure is valid here, it's the testbot that is not smart enough. The tested DLL is not present on WINNT, so the test should be skipped automatically, like winetest does.
Jacek
On Fri, 16 Sep 2011, Jacek Caban wrote: [...]
hres = CoCreateInstance(&CLSID_WebBrowser, NULL,CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER, &IID_IUnknown, (void**)unk);
- ok_(__FILE__,line)(hres == S_OK, "Creating WebBrowser object failed:
%08x\n", hres);
- if (hres == REGDB_E_CLASSNOTREG)
win_skip_(__FILE__,line)("WebBrowser is not supported\n");
[...]
The failure is valid here, it's the testbot that is not smart enough. The tested DLL is not present on WINNT, so the test should be skipped automatically, like winetest does.
Yes, that's why I used a wine_skip() rather than a skip(). I think this approach is valid too. My understanding is that in theory CLSID_WebBrowser could be provided by any other dll. So given that this test makes no direct use of ieframes.dll (that is does not link with it nor loads it at run time), that makes checking for the presence of ieframes.dll a bit questionable.
Indead on my NT4 VM I do get a CLSID_WebBrowser object but various tests then fail. It appears the same thing happens on the W2KPROSP4 testbot VM.
I presume that's because these Windows versions provided this object through some older dll that did not support everything. But it would be nice to have the tests detect that and win_skip().
On 09/16/11 15:42, Francois Gouget wrote:
On Fri, 16 Sep 2011, Jacek Caban wrote: [...]
hres = CoCreateInstance(&CLSID_WebBrowser, NULL,CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER, &IID_IUnknown, (void**)unk);
- ok_(__FILE__,line)(hres == S_OK, "Creating WebBrowser object failed:
%08x\n", hres);
- if (hres == REGDB_E_CLASSNOTREG)
win_skip_(__FILE__,line)("WebBrowser is not supported\n");[...]
The failure is valid here, it's the testbot that is not smart enough. The tested DLL is not present on WINNT, so the test should be skipped automatically, like winetest does.
Yes, that's why I used a wine_skip() rather than a skip(). I think this approach is valid too. My understanding is that in theory CLSID_WebBrowser could be provided by any other dll. So given that this test makes no direct use of ieframes.dll (that is does not link with it nor loads it at run time), that makes checking for the presence of ieframes.dll a bit questionable.
IEs older than 7 had WebBrowser control implemented in shdocvw.dll. We surely don't want to run tests on anything older than IE6. IE6 results very little value in this case, so I consider DLL presence check as a nice way of skipping tests on too old platforms and I've even stripped the code from no longer needed checks that you want to add when I was moving them from shdocvw to ieframe, In this case, skipping tests is really a job for testbot.
Jacek
On Fri, 16 Sep 2011, Jacek Caban wrote: [...]
IEs older than 7 had WebBrowser control implemented in shdocvw.dll. We surely don't want to run tests on anything older than IE6. IE6 results very little value in this case, so I consider DLL presence check as a nice way of skipping tests on too old platforms and I've even stripped the code from no longer needed checks that you want to add when I was moving them from shdocvw to ieframe, In this case, skipping tests is really a job for testbot.
The problem is that the fact we want the ieframes.dll implementation and none other is absolutely not documented currently. Just relying on the test executable's name is insufficient imho, especially as it runs counter to the COM philosophy[1]. Similarly, relying on WineTest or Testbot behavior is insufficient in this case.
So I think it owuld be much better if the test itself documented what its requirements are, either with a LoadLibrary() or with at least a trace or comment in the code.
[1] That is, the fact that a piece of code tries to create a CLSID_WebBrowser object does not imply that it wants ieframes.dll or any other specific dll.
-
Francois Gouget -
Jacek Caban