[PATCH 0/1] MR4052: secur32/tests: Skip the tests instead of crashing if Kerberos is not supported.
From: Francois Gouget <fgouget(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org//show_bug.cgi?id=55555 --- dlls/secur32/tests/secur32.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/dlls/secur32/tests/secur32.c b/dlls/secur32/tests/secur32.c index b1a1db76c3e..db81e23bd7e 100644 --- a/dlls/secur32/tests/secur32.c +++ b/dlls/secur32/tests/secur32.c @@ -56,6 +56,8 @@ static BOOLEAN (WINAPI * pGetUserNameExW)(EXTENDED_NAME_FORMAT NameFormat, LPWST static PSecurityFunctionTableA (SEC_ENTRY * pInitSecurityInterfaceA)(void); static PSecurityFunctionTableW (SEC_ENTRY * pInitSecurityInterfaceW)(void); +static BOOLEAN has_kerberos; + static EXTENDED_NAME_FORMAT formats[] = { NameUnknown, NameFullyQualifiedDN, NameSamCompatible, NameDisplay, NameUniqueId, NameCanonical, NameUserPrincipal, NameCanonicalEx, @@ -446,9 +448,13 @@ static void test_kerberos(void) status = QuerySecurityPackageInfoA(provider, &info); - ok(status == SEC_E_OK, "Kerberos package not installed, skipping test\n"); + if(status == SEC_E_SECPKG_NOT_FOUND) + skip("Skipping the Kerberos tests (package is not installed).\n"); + else + ok(status == SEC_E_OK, "Failed to query the Kerberos package: %lx\n", status); if(status != SEC_E_OK) return; + has_kerberos = TRUE; ok( (info->fCapabilities & ~optional_mask) == expected_flags, "got %08lx, expected %08lx\n", info->fCapabilities, expected_flags ); ok( info->wVersion == 1, "got %u\n", info->wVersion ); @@ -479,7 +485,15 @@ static void test_ticket_cache(void) RtlInitAnsiString( &name, MICROSOFT_KERBEROS_NAME_A ); status = LsaLookupAuthenticationPackage( lsa, &name, &package ); - ok( !status, "got %08lx\n", status ); + if (has_kerberos) + ok( !status, "got %08lx\n", status ); + else + { + ok( status, "expected Kerberos lookup to fail\n" ); + skip("Skipping the ticket cache tests (no Kerberos: %lx).\n", status); + LsaDeregisterLogonProcess( lsa ); + return; + } status = LsaCallAuthenticationPackage( lsa, package, &req, sizeof(req), (void **)&resp, &len, &status ); ok( !status, "got %08lx\n", status ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4052
Francois Gouget <wine(a)gitlab.winehq.org> wrote:
+static BOOLEAN has_kerberos;
Please use BOOL instead.
status = QuerySecurityPackageInfoA(provider, &info); - ok(status == SEC_E_OK, "Kerberos package not installed, skipping test\n"); + if(status == SEC_E_SECPKG_NOT_FOUND) + skip("Skipping the Kerberos tests (package is not installed).\n"); + else + ok(status == SEC_E_OK, "Failed to query the Kerberos package: %lx\n", status); if(status != SEC_E_OK) return; + has_kerberos = TRUE;
ok( (info->fCapabilities & ~optional_mask) == expected_flags, "got %08lx, expected %08lx\n", info->fCapabilities, expected_flags ); ok( info->wVersion == 1, "got %u\n", info->wVersion ); @@ -479,7 +485,15 @@ static void test_ticket_cache(void)
RtlInitAnsiString( &name, MICROSOFT_KERBEROS_NAME_A ); status = LsaLookupAuthenticationPackage( lsa, &name, &package ); - ok( !status, "got %08lx\n", status ); + if (has_kerberos) + ok( !status, "got %08lx\n", status ); + else + { + ok( status, "expected Kerberos lookup to fail\n" ); + skip("Skipping the ticket cache tests (no Kerberos: %lx).\n", status); + LsaDeregisterLogonProcess( lsa ); + return; + }
status = LsaCallAuthenticationPackage( lsa, package, &req, sizeof(req), (void **)&resp, &len, &status ); ok( !status, "got %08lx\n", status );
Missing Kerberos makes the installation broken and not supported. Crashing in this situation is correct behaviour. It's better to spend time on fixing broken configuration instead of hiding the problem. -- Dmitry.
The tests don't crash anymore according to the patterns page. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4052#note_48031
None of the test failures are related to this MR: * amstream:amstream -> bug 55725 * msxml3:domdoc -> bug 55720 * psapi:psapi_main -> bug 55744 * quartz:filtergraph -> bug 55758 * tasklist.exe:tasklist -> bug 55745 * wscript.exe:run -> bug 55746 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4052#note_48033
And I think the test should fail if the package is not available, like we do in the NTLM test. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4052#note_48047
On Wed Oct 11 08:28:43 2023 +0000, Hans Leidekker wrote:
The tests don't crash anymore according to the patterns page. Right. I wanted to check out the remaining failures on macOS but I don't have a Mac. So I compiled Wine using --without-krb5 instead and I got a crash on line 486 because LsaCallAuthenticationPackage() failed and set resp to NULL.
So the failures on Remi's macOS machines is different but I think this crash is worth fixing anyway. So you would prefer something more like test_kerberos(): an ok() that says skip and a return? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4052#note_48310
On Wed Oct 11 08:28:43 2023 +0000, Francois Gouget wrote:
Right. I wanted to check out the remaining failures on macOS but I don't have a Mac. So I compiled Wine using --without-krb5 instead and I got a crash on line 486 because LsaCallAuthenticationPackage() failed and set resp to NULL. So the failures on Remi's macOS machines is different but I think this crash is worth fixing anyway. So you would prefer something more like test_kerberos(): an ok() that says skip and a return? Yes.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4052#note_48311
participants (4)
-
Dmitry Timoshkov -
Francois Gouget -
Francois Gouget (＠fgouget)
-
Hans Leidekker (＠hans)