Hi David,
The patch looks much better now, but needs a bit more. BTW, this would be better tested on protocol level (like we do in protocol.c) than moniker binding, but this will do too.
On 1/3/11 3:47 AM, David Hedberg wrote:
Try 2: Better tests, among other things.
dlls/urlmon/http.c | 173 +++++++++++++++++++++++++++++++++++++- dlls/urlmon/tests/url.c | 211 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 373 insertions(+), 11 deletions(-)
@@ -2546,11 +2670,19 @@ static void test_BindToStorage(int protocol, DWORD flags, DWORD t) SET_EXPECT(QueryInterface_IHttpNegotiate); SET_EXPECT(QueryInterface_IWindowForBindingUI); SET_EXPECT(QueryService_IWindowForBindingUI);
SET_EXPECT(GetWindow_IWindowForBindingUI); SET_EXPECT(BeginningTransaction); SET_EXPECT(QueryInterface_IHttpNegotiate2); SET_EXPECT(GetRootSecurityId); SET_EXPECT(OnProgress_FINDINGRESOURCE); SET_EXPECT(OnProgress_CONNECTING);if(flags& BINDTEST_INVALID_CN) {SET_EXPECT(QueryInterface_IHttpSecurity);SET_EXPECT(QueryService_IHttpSecurity);SET_EXPECT(OnSecurityProblem);if(SUCCEEDED(onsecurityproblem_hres))SET_EXPECT(GetWindow_IHttpSecurity);} } if(!no_callback) { if(test_protocol == HTTP_TEST || test_protocol == HTTPS_TEST || test_protocol == FTP_TEST@@ -2600,6 +2732,47 @@ static void test_BindToStorage(int protocol, DWORD flags, DWORD t) ok(hres == INET_E_DATA_NOT_AVAILABLE, "IMoniker_BindToStorage failed: %08x, expected INET_E_DATA_NOT_AVAILABLE\n", hres); ok(unk == NULL, "istr should be NULL\n");
- }else if(flags& BINDTEST_INVALID_CN) {
if(invalid_cn_accepted) {todo_wine CHECK_NOT_CALLED(QueryInterface_IHttpSecurity);todo_wine CHECK_NOT_CALLED(QueryService_IHttpSecurity);todo_wine CHECK_NOT_CALLED(OnSecurityProblem);}else {CHECK_CALLED(QueryInterface_IHttpSecurity);CHECK_CALLED(QueryService_IHttpSecurity);CHECK_CALLED(OnSecurityProblem);if(onsecurityproblem_hres != S_OK || security_problem == ERROR_INTERNET_SEC_CERT_ERRORS) {CHECK_NOT_CALLED(QueryInterface_IInternetBindInfo);CHECK_NOT_CALLED(QueryService_IInternetBindInfo);CHECK_CALLED(QueryInterface_IHttpNegotiate);CHECK_CALLED(BeginningTransaction);CHECK_CALLED(QueryInterface_IHttpNegotiate2);CHECK_CALLED(GetRootSecurityId);CLEAR_CALLED(GetWindow_IWindowForBindingUI);CLEAR_CALLED(QueryInterface_IWindowForBindingUI);CLEAR_CALLED(QueryService_IWindowForBindingUI);if(onsecurityproblem_hres == S_FALSE) {if(security_problem == ERROR_INTERNET_SEC_CERT_ERRORS)CLEAR_CALLED(OnProgress_FINDINGRESOURCE);elseCHECK_CALLED(OnProgress_FINDINGRESOURCE);CHECK_CALLED(GetWindow_IHttpSecurity);}else {todo_wine CHECK_NOT_CALLED(OnProgress_FINDINGRESOURCE);CHECK_NOT_CALLED(GetWindow_IHttpSecurity);}}}if(binding_hres != S_OK) {ok(hres == binding_hres, "Got %08x\n", hres);ok(unk == NULL, "Got %p\n", unk);}else if(invalid_cn_accepted) {todo_wine ok(hres == S_OK, "IMoniker_BindToStorage failed: %08x\n", hres);todo_wine ok(unk != NULL, "unk == NULL\n");}else {ok(hres == S_OK, "IMoniker_BindToStorage failed: %08x\n", hres);ok(unk != NULL, "unk == NULL\n");}
This is making tests even harder to maintain than they are now. I guess you're duplicating checking called function here because of returning on failure a few lines later. It's the return that shouldn't be there. You can change it to
if(FAILED(hres) && !(flags & BINDTEST_INVALID_CN)) return;
if you don't want to deal with existing problems in error handling.
Jacek