[PATCH v2 1/3] advapi32/tests: Clean up tests for duplicate service names.
Use our own services instead of relying on a system service, and account for differing Windows 10 behaviour. Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/advapi32/tests/service.c | 72 ++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/dlls/advapi32/tests/service.c b/dlls/advapi32/tests/service.c index 6c78900bd4..c607e292aa 100644 --- a/dlls/advapi32/tests/service.c +++ b/dlls/advapi32/tests/service.c @@ -203,10 +203,9 @@ static void test_create_delete_svc(void) static const CHAR pathname [] = "we_dont_care.exe"; static const CHAR empty [] = ""; static const CHAR password [] = "secret"; - BOOL spooler_exists = FALSE; + char buffer[200]; + DWORD size; BOOL ret; - CHAR display[4096]; - DWORD display_size = sizeof(display); /* Get the username and turn it into an account to be used in some tests */ GetUserNameA(username, &user_size); @@ -356,42 +355,45 @@ static void test_create_delete_svc(void) ok(!svc_handle1, "Expected failure\n"); ok(GetLastError() == ERROR_INVALID_PARAMETER, "Expected ERROR_INVALID_PARAMETER, got %d\n", GetLastError()); - /* The service already exists (check first, just in case) */ - svc_handle1 = OpenServiceA(scm_handle, spooler, GENERIC_READ); - if (svc_handle1) + /* Test duplicate service names */ + svc_handle1 = CreateServiceA(scm_handle, "winetest_dupname", "winetest_display", DELETE, + SERVICE_WIN32_OWN_PROCESS, SERVICE_AUTO_START, 0, pathname, NULL, NULL, NULL, NULL, NULL); + ok(!!svc_handle1, "Failed to create service, error %u\n", GetLastError()); + + svc_handle2 = CreateServiceA(scm_handle, "winetest_dupname", NULL, 0, + SERVICE_WIN32_OWN_PROCESS, SERVICE_AUTO_START, 0, pathname, NULL, NULL, NULL, NULL, NULL); + ok(!svc_handle2, "Expected failure\n"); + ok(GetLastError() == ERROR_SERVICE_EXISTS, "Got wrong error %u\n", GetLastError()); + + svc_handle2 = CreateServiceA(scm_handle, "winetest_dupname2", "winetest_dupname", DELETE, + SERVICE_WIN32_OWN_PROCESS, SERVICE_AUTO_START, 0, pathname, NULL, NULL, NULL, NULL, NULL); + todo_wine ok(!svc_handle2, "Expected failure\n"); + todo_wine ok(GetLastError() == ERROR_DUPLICATE_SERVICE_NAME, "Got wrong error %u\n", GetLastError()); + if (svc_handle2) { - spooler_exists = TRUE; - CloseServiceHandle(svc_handle1); - SetLastError(0xdeadbeef); - svc_handle1 = CreateServiceA(scm_handle, spooler, NULL, 0, SERVICE_WIN32_OWN_PROCESS, - SERVICE_DISABLED, 0, pathname, NULL, NULL, NULL, NULL, NULL); - ok(!svc_handle1, "Expected failure\n"); - ok(GetLastError() == ERROR_SERVICE_EXISTS, "Expected ERROR_SERVICE_EXISTS, got %d\n", GetLastError()); + DeleteService(svc_handle2); + CloseServiceHandle(svc_handle2); + } + + svc_handle2 = CreateServiceA(scm_handle, "winetest_dupname2", "winetest_display", DELETE, + SERVICE_WIN32_OWN_PROCESS, SERVICE_AUTO_START, 0, pathname, NULL, NULL, NULL, NULL, NULL); + if (svc_handle2) /* Win10 1709+ */ + { + size = sizeof(buffer); + ret = GetServiceKeyNameA(scm_handle, "winetest_display", buffer, &size); + ok(ret, "Failed to get key name, error %u\n", GetLastError()); + ok(!strcmp(buffer, "winetest_dupname"), "Got wrong name \"%s\"\n", buffer); + + ret = DeleteService(svc_handle2); + ok(ret, "Failed to delete service, error %u\n", GetLastError()); + CloseServiceHandle(svc_handle2); } else - skip("Spooler service doesn't exist\n"); + ok(GetLastError() == ERROR_DUPLICATE_SERVICE_NAME, "Got wrong error %u\n", GetLastError()); - /* To find an existing displayname we check the 'Spooler' service. Although the registry - * doesn't show DisplayName on NT4, this call will return a displayname which is equal - * to the servicename and can't be used as well for a new displayname. - */ - if (spooler_exists) - { - ret = GetServiceDisplayNameA(scm_handle, spooler, display, &display_size); - - if (!ret) - skip("Could not retrieve a displayname for the Spooler service\n"); - else - { - svc_handle1 = CreateServiceA(scm_handle, servicename, display, 0, SERVICE_WIN32_OWN_PROCESS, - SERVICE_DISABLED, 0, pathname, NULL, NULL, NULL, NULL, NULL); - ok(!svc_handle1, "Expected failure for display name '%s'\n", display); - ok(GetLastError() == ERROR_DUPLICATE_SERVICE_NAME, - "Expected ERROR_DUPLICATE_SERVICE_NAME, got %d\n", GetLastError()); - } - } - else - skip("Could not retrieve a displayname (Spooler service doesn't exist)\n"); + ret = DeleteService(svc_handle1); + ok(ret, "Failed to delete service, error %u\n", GetLastError()); + CloseServiceHandle(svc_handle1); /* Windows doesn't care about the access rights for creation (which makes * sense as there is no service yet) as long as there are sufficient -- 2.20.1
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/advapi32/tests/service.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dlls/advapi32/tests/service.c b/dlls/advapi32/tests/service.c index c607e292aa..75da3cd483 100644 --- a/dlls/advapi32/tests/service.c +++ b/dlls/advapi32/tests/service.c @@ -2275,8 +2275,9 @@ static void test_queryconfig2(void) } ok(ret, "expected QueryServiceConfig2W to succeed (%d)\n", GetLastError()); ok(needed == sizeof(preshutdown_info), "needed = %d\n", needed); - ok(preshutdown_info.dwPreshutdownTimeout == 180000, "Default PreshutdownTimeout = %d\n", - preshutdown_info.dwPreshutdownTimeout); + ok(preshutdown_info.dwPreshutdownTimeout == 180000 + || preshutdown_info.dwPreshutdownTimeout == 10000 /* Win10 1709+ */, + "Default PreshutdownTimeout = %d\n", preshutdown_info.dwPreshutdownTimeout); SetLastError(0xdeadbeef); preshutdown_info.dwPreshutdownTimeout = -1; -- 2.20.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=58427 Your paranoid android. === w1064v1809_he (32 bit report) === advapi32: service.c:1724: Test failed: Expected a process id for this running service (sppsvc) === wvistau64 (64 bit report) === advapi32: service.c:2467: Test failed: Got wrong SleepEx result: 0 service.c:2468: Test failed: APC wasn't called service.c:2469: Test failed: APC wasn't called service.c:2478: Test failed: NotifyServiceStatusChangeW failed: 1242 service.c:2483: Test failed: StartService failed: 1056 service.c:2486: Test failed: Got wrong SleepEx result: 192 service.c:2490: Test failed: ControlService failed: 1062
Simplify different paths, explicitly test data2.was_called, and test that the correct notification is produced for Windows 8+ (previously an intermittent failure was observed due to this uninitialized field). Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/advapi32/tests/service.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/dlls/advapi32/tests/service.c b/dlls/advapi32/tests/service.c index 75da3cd483..aa9a69fed2 100644 --- a/dlls/advapi32/tests/service.c +++ b/dlls/advapi32/tests/service.c @@ -2434,16 +2434,23 @@ static void test_servicenotify(SC_HANDLE scm_handle, const char *servicename) data2.notify.dwVersion = SERVICE_NOTIFY_STATUS_CHANGE; data2.notify.pfnNotifyCallback = ¬ify_cb; data2.notify.pContext = &data2; + data2.phase = PHASE_RUNNING; + data2.was_called = FALSE; dr = pNotifyServiceStatusChangeW(svc, SERVICE_NOTIFY_STOPPED | SERVICE_NOTIFY_RUNNING, &data2.notify); - ok(dr == ERROR_SUCCESS || /* win8+ */ - dr == ERROR_ALREADY_REGISTERED, "NotifyServiceStatusChangeW gave wrong result: %u\n", dr); - - /* should receive no notification because status has not changed. - * on win8+, SleepEx quits early but the callback is still not invoked. */ - dr2 = SleepEx(100, TRUE); - ok((dr == ERROR_SUCCESS && dr2 == WAIT_IO_COMPLETION) || /* win8+ */ - (dr == ERROR_ALREADY_REGISTERED && dr2 == 0), "Got wrong SleepEx result: %u\n", dr); + ok(dr == ERROR_ALREADY_REGISTERED || !dr /* Win8+ */, "wrong error %u\n", dr); + if (!dr) + { + dr = SleepEx(100, TRUE); + ok(dr == WAIT_IO_COMPLETION, "got %u\n", dr); + ok(data2.was_called, "APC was not called\n"); + } + else + { + dr = SleepEx(100, TRUE); + ok(!dr, "got %u\n", dr); + ok(!data2.was_called, "APC should not have been called\n"); + } ok(data.was_called == FALSE, "APC should not have been called\n"); memset(&data2.notify, 0, sizeof(data2.notify)); -- 2.20.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=58428 Your paranoid android. === w8 (32 bit report) === advapi32: service.c:1337: Test failed: Expected 109 services to be returned service.c:1607: Test failed: Expected the same number of service from this function
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=58426 Your paranoid android. === w8 (32 bit report) === advapi32: service.c:1337: Test failed: Expected 109 services to be returned service.c:1607: Test failed: Expected the same number of service from this function
participants (2)
-
Marvin -
Zebediah Figura