[PATCH 1/4] wininet/tests: Update InternetErrorDlg tests taking recent Windows 10 results as a base.
Signed-off-by: Dmitry Timoshkov dmitry@baikal.ru --- dlls/wininet/tests/internet.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/dlls/wininet/tests/internet.c b/dlls/wininet/tests/internet.c index ef1adefd1a3..c61a9793a95 100644 --- a/dlls/wininet/tests/internet.c +++ b/dlls/wininet/tests/internet.c @@ -1483,20 +1483,21 @@ static void test_InternetErrorDlg(void) { ERROR_INTERNET_SEC_CERT_CN_INVALID , ERROR_CANCELLED, 0 }, { ERROR_INTERNET_HTTP_TO_HTTPS_ON_REDIR , ERROR_SUCCESS, 0 }, { ERROR_INTERNET_HTTPS_TO_HTTP_ON_REDIR , ERROR_SUCCESS, FLAG_TODO }, - { ERROR_INTERNET_MIXED_SECURITY , ERROR_CANCELLED, FLAG_TODO }, - { ERROR_INTERNET_CHG_POST_IS_NON_SECURE , ERROR_CANCELLED, FLAG_TODO }, + { ERROR_INTERNET_MIXED_SECURITY , ERROR_CANCELLED }, + { ERROR_INTERNET_CHG_POST_IS_NON_SECURE , ERROR_SUCCESS, FLAG_TODO }, { ERROR_INTERNET_POST_IS_NON_SECURE , ERROR_SUCCESS, 0 }, - { ERROR_INTERNET_CLIENT_AUTH_CERT_NEEDED, ERROR_CANCELLED, FLAG_NEEDREQ|FLAG_TODO }, + { ERROR_INTERNET_CLIENT_AUTH_CERT_NEEDED, ERROR_CANCELLED, FLAG_NEEDREQ }, { ERROR_INTERNET_INVALID_CA , ERROR_CANCELLED, 0 }, - { ERROR_INTERNET_HTTPS_HTTP_SUBMIT_REDIR, ERROR_CANCELLED, FLAG_TODO }, - { ERROR_INTERNET_INSERT_CDROM , ERROR_CANCELLED, FLAG_TODO|FLAG_NEEDREQ|FLAG_UNIMPL }, + { ERROR_INTERNET_HTTPS_HTTP_SUBMIT_REDIR, ERROR_CANCELLED }, + { ERROR_INTERNET_INSERT_CDROM , ERROR_CANCELLED, FLAG_NEEDREQ|FLAG_UNIMPL }, { ERROR_INTERNET_SEC_CERT_ERRORS , ERROR_CANCELLED, 0 }, { ERROR_INTERNET_SEC_CERT_REV_FAILED , ERROR_CANCELLED, 0 }, { ERROR_HTTP_COOKIE_NEEDS_CONFIRMATION , ERROR_HTTP_COOKIE_DECLINED, FLAG_TODO }, - { ERROR_INTERNET_BAD_AUTO_PROXY_SCRIPT , ERROR_CANCELLED, FLAG_TODO }, - { ERROR_INTERNET_UNABLE_TO_DOWNLOAD_SCRIPT, ERROR_CANCELLED, FLAG_TODO }, - { ERROR_HTTP_REDIRECT_NEEDS_CONFIRMATION, ERROR_CANCELLED, FLAG_TODO }, + { ERROR_INTERNET_BAD_AUTO_PROXY_SCRIPT , ERROR_CANCELLED }, + { ERROR_INTERNET_UNABLE_TO_DOWNLOAD_SCRIPT, ERROR_CANCELLED }, + { ERROR_HTTP_REDIRECT_NEEDS_CONFIRMATION, ERROR_CANCELLED }, { ERROR_INTERNET_SEC_CERT_REVOKED , ERROR_CANCELLED, 0 }, + { ERROR_INTERNET_SEC_CERT_WEAK_SIGNATURE, ERROR_CANCELLED, 0 }, };
res = InternetErrorDlg(NULL, NULL, ERROR_INTERNET_SEC_CERT_ERRORS, 0, NULL); @@ -1560,14 +1561,16 @@ static void test_InternetErrorDlg(void) expected = NTE_PROV_TYPE_NOT_DEF; break; case ERROR_INTERNET_CHG_POST_IS_NON_SECURE: - if(res == ERROR_SUCCESS) /* win10 returns ERROR_SUCCESS */ - expected = ERROR_SUCCESS; + if(res == ERROR_CANCELLED) /* before win10 returns ERROR_CANCELLED */ + expected = ERROR_CANCELLED; break; default: break; }
- if (expected == ERROR_NOT_SUPPORTED && res == ERROR_CANCELLED) /* Win10 1607+ */ + if(expected == ERROR_NOT_SUPPORTED && res == ERROR_CANCELLED) /* Win10 1607+ */ expected = ERROR_CANCELLED; + else if(expected == ERROR_CANCELLED && res == ERROR_NOT_SUPPORTED) /* XP, Win7, Win8 */ + expected = ERROR_NOT_SUPPORTED;
todo_wine_if(test_flags & FLAG_TODO) ok(res == expected, "Got %d, expected %d (%d)\n", res, expected, i); @@ -1583,7 +1586,7 @@ static void test_InternetErrorDlg(void) expected = ERROR_INVALID_PARAMETER;
res = InternetErrorDlg(hwnd, NULL, i, FLAGS_ERROR_UI_FLAGS_NO_UI, NULL); - todo_wine_if( test_flags & FLAG_TODO || i == ERROR_INTERNET_INCORRECT_PASSWORD) + todo_wine_if ((test_flags & FLAG_TODO) || i == ERROR_INTERNET_INCORRECT_PASSWORD || i == ERROR_INTERNET_INSERT_CDROM || i == ERROR_INTERNET_CLIENT_AUTH_CERT_NEEDED) ok(res == expected, "Got %d, expected %d (%d)\n", res, expected, i); }
Can I get a review of these patches please?
Hi Dmitry,
On 8/6/21 2:43 PM, Dmitry Timoshkov wrote:
@@ -1583,7 +1586,7 @@ static void test_InternetErrorDlg(void) expected = ERROR_INVALID_PARAMETER;
res = InternetErrorDlg(hwnd, NULL, i, FLAGS_ERROR_UI_FLAGS_NO_UI, NULL);
todo_wine_if( test_flags & FLAG_TODO || i == ERROR_INTERNET_INCORRECT_PASSWORD)
todo_wine_if ((test_flags & FLAG_TODO) || i == ERROR_INTERNET_INCORRECT_PASSWORD || i == ERROR_INTERNET_INSERT_CDROM || i == ERROR_INTERNET_CLIENT_AUTH_CERT_NEEDED)
Is there a reason why you don't use FLAG_TODO for those? It may not match what you need for your new tests, but why are you changing half of its usage here?
Thanks,
Jacek
Hi Jacek,
Thanks for the review.
Jacek Caban jacek@codeweavers.com wrote:
@@ -1583,7 +1586,7 @@ static void test_InternetErrorDlg(void) expected = ERROR_INVALID_PARAMETER;
res = InternetErrorDlg(hwnd, NULL, i, FLAGS_ERROR_UI_FLAGS_NO_UI, NULL);
todo_wine_if( test_flags & FLAG_TODO || i == ERROR_INTERNET_INCORRECT_PASSWORD)
todo_wine_if ((test_flags & FLAG_TODO) || i == ERROR_INTERNET_INCORRECT_PASSWORD || i == ERROR_INTERNET_INSERT_CDROM || i == ERROR_INTERNET_CLIENT_AUTH_CERT_NEEDED)Is there a reason why you don't use FLAG_TODO for those? It may not match what you need for your new tests, but why are you changing half of its usage here?
Because using FLAG_TODO is fragile due to inconsistent behaviour, and it fails in new or old tests. I tried different approaches in hope to make it more clean and less controvercial regarding FLAG_TODO usage, but finally decided to have it this way. I'm still open to suggestions though, however I haven't found anything better while changing the tests to follow recent Windows behaviour.
On 8/26/21 9:33 PM, Dmitry Timoshkov wrote:
Hi Jacek,
Thanks for the review.
Jacek Caban jacek@codeweavers.com wrote:
@@ -1583,7 +1586,7 @@ static void test_InternetErrorDlg(void) expected = ERROR_INVALID_PARAMETER;
res = InternetErrorDlg(hwnd, NULL, i, FLAGS_ERROR_UI_FLAGS_NO_UI, NULL);
todo_wine_if( test_flags & FLAG_TODO || i == ERROR_INTERNET_INCORRECT_PASSWORD)
todo_wine_if ((test_flags & FLAG_TODO) || i == ERROR_INTERNET_INCORRECT_PASSWORD || i == ERROR_INTERNET_INSERT_CDROM || i == ERROR_INTERNET_CLIENT_AUTH_CERT_NEEDED)Is there a reason why you don't use FLAG_TODO for those? It may not match what you need for your new tests, but why are you changing half of its usage here?
Because using FLAG_TODO is fragile due to inconsistent behaviour, and it fails in new or old tests. I tried different approaches in hope to make it more clean and less controvercial regarding FLAG_TODO usage, but finally decided to have it this way. I'm still open to suggestions though, however I haven't found anything better while changing the tests to follow recent Windows behaviour.
If you remove most of them, then you may remove all of them as well. If you remove most of them, you may as well replace the existing mechanism. How about something like this:
https://testbot.winehq.org/JobDetails.pl?Key=96801
Thanks,
Jacek
Jacek Caban jacek@codeweavers.com wrote:
@@ -1583,7 +1586,7 @@ static void test_InternetErrorDlg(void) expected = ERROR_INVALID_PARAMETER;
res = InternetErrorDlg(hwnd, NULL, i, FLAGS_ERROR_UI_FLAGS_NO_UI, NULL);
todo_wine_if( test_flags & FLAG_TODO || i == ERROR_INTERNET_INCORRECT_PASSWORD)
todo_wine_if ((test_flags & FLAG_TODO) || i == ERROR_INTERNET_INCORRECT_PASSWORD || i == ERROR_INTERNET_INSERT_CDROM || i == ERROR_INTERNET_CLIENT_AUTH_CERT_NEEDED)Is there a reason why you don't use FLAG_TODO for those? It may not match what you need for your new tests, but why are you changing half of its usage here?
Because using FLAG_TODO is fragile due to inconsistent behaviour, and it fails in new or old tests. I tried different approaches in hope to make it more clean and less controvercial regarding FLAG_TODO usage, but finally decided to have it this way. I'm still open to suggestions though, however I haven't found anything better while changing the tests to follow recent Windows behaviour.
If you remove most of them, then you may remove all of them as well. If you remove most of them, you may as well replace the existing mechanism. How about something like this:
Thanks for keeping this going, the test and the results look good to me, though I'd remove the use of broken() inside of an if statement. Are you planning to submit this patch?
-
Dmitry Timoshkov -
Jacek Caban