This matches the Wine code.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/src/TestLauncher/TestLauncher.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/testbot/src/TestLauncher/TestLauncher.c b/testbot/src/TestLauncher/TestLauncher.c index 783dbaba5..02acde04f 100644 --- a/testbot/src/TestLauncher/TestLauncher.c +++ b/testbot/src/TestLauncher/TestLauncher.c @@ -23,7 +23,7 @@ #include <errno.h> #include <windows.h>
-#define countof(Array) (sizeof(Array) / sizeof(Array[0])) +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
static unsigned Failures = 0; static unsigned Skips = 0; @@ -324,7 +324,7 @@ int main(int argc, char *argv[]) } else { - if (GetFullPathNameA(argv[Arg], countof(TestExeFullName), TestExeFullName, &TestExeFileName) == 0) + if (GetFullPathNameA(argv[Arg], ARRAY_SIZE(TestExeFullName), TestExeFullName, &TestExeFileName) == 0) { fprintf(stderr, "Can't determine full path of test executable %s, error %lu\n", argv[Arg], GetLastError());
There is no point waiting again after terminating the process. Make sure to report the WaitForSingleObject() status if the process exit code is not available.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/src/TestLauncher/TestLauncher.c | 48 +++++++------------------ 1 file changed, 13 insertions(+), 35 deletions(-)
diff --git a/testbot/src/TestLauncher/TestLauncher.c b/testbot/src/TestLauncher/TestLauncher.c index 02acde04f..bfc9f0fa4 100644 --- a/testbot/src/TestLauncher/TestLauncher.c +++ b/testbot/src/TestLauncher/TestLauncher.c @@ -3,6 +3,7 @@ * Verifies that the dlls needed for the test are present. * * Copyright 2009 Ge van Geldorp + * Copyright 2012-2021 Francois Gouget * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -295,9 +296,8 @@ int main(int argc, char *argv[]) int CommandLen; STARTUPINFOA StartupInfo; PROCESS_INFORMATION ProcessInformation; - DWORD WaitStatus; DWORD ExitCode; - + TimeOut = INFINITE; CommandLine = NULL; Arg = 1; @@ -401,54 +401,32 @@ int main(int argc, char *argv[]) fprintf(stderr, "CreateProcess failed with error %lu\n", GetLastError()); exit(1); } - CloseHandle(ProcessInformation.hThread); - WaitStatus = WaitForSingleObject(ProcessInformation.hProcess, TimeOut); - if (WaitStatus != WAIT_OBJECT_0) + + ExitCode = WaitForSingleObject(ProcessInformation.hProcess, TimeOut); + if (ExitCode != WAIT_OBJECT_0) { - switch(WaitStatus) + switch (ExitCode) { case WAIT_FAILED: fprintf(stderr, "Wait for child failed, error %lu\n", GetLastError()); break;
case WAIT_TIMEOUT: + /* The 'exit code' on the done line identifies timeouts */ break;
default: - fprintf(stderr, "Unexpected return value %lu from wait for child\n", WaitStatus); - break; - } - - ExitCode = WaitStatus; - if (! TerminateProcess(ProcessInformation.hProcess, 257)) - fprintf(stderr, "TerminateProcess failed with error %lu\n", GetLastError()); - - switch (WaitForSingleObject(ProcessInformation.hProcess, 5000)) - { - case WAIT_OBJECT_0: - break; - - case WAIT_FAILED: - fprintf(stderr, "Wait for terminate failed, error %lu\n", GetLastError()); - break; - - case WAIT_TIMEOUT: - fprintf(stderr, "Can't kill child\n"); - break; - - default: - fprintf(stderr, "Unexpected return value %lu from wait for terminate\n", WaitStatus); + fprintf(stderr, "Unexpected return value %lu from wait for child\n", ExitCode); break; } + if (!TerminateProcess(ProcessInformation.hProcess, 257)) + fprintf(stderr, "TerminateProcess failed, error %lu\n", GetLastError()); } - else + else if (!GetExitCodeProcess(ProcessInformation.hProcess, &ExitCode)) { - if (! GetExitCodeProcess(ProcessInformation.hProcess, &ExitCode)) - { - ExitCode = 259; - fprintf(stderr, "Can't get child exit code, error %lu\n", GetLastError()); - } + fprintf(stderr, "Can't get child exit code, error %lu\n", GetLastError()); + ExitCode = 259; } CloseHandle(ProcessInformation.hProcess);
Detect the critical error dialog that pops up when the test executable depends on a missing dll or entry point. This improves the diagnostic messages and avoids long timeouts.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=31609 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=32216 Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/src/TestLauncher/TestLauncher.c | 341 +++++++----------------- 1 file changed, 92 insertions(+), 249 deletions(-)
diff --git a/testbot/src/TestLauncher/TestLauncher.c b/testbot/src/TestLauncher/TestLauncher.c index bfc9f0fa4..7b58c9ad1 100644 --- a/testbot/src/TestLauncher/TestLauncher.c +++ b/testbot/src/TestLauncher/TestLauncher.c @@ -26,271 +26,91 @@
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
-static unsigned Failures = 0; -static unsigned Skips = 0;
+/* + * Missing dll and entry point detection. + */
-#define ReportError (_SetErrorLocation(__FILE__, __LINE__), 0) ? (void)0 : _ReportError +const char *Subtest; +static unsigned Skips = 0;
-static const char *LocationFile; -static unsigned LocationLine; -static void _SetErrorLocation(const char* file, int line) +BOOL CALLBACK DumpCriticalErrorDescription(HWND hWnd, LPARAM lParam) { - LocationFile = file; - LocationLine = line; -} + char Buffer[512]; + int Count;
-#ifdef __GNUC__ -static void _ReportError(const char *Format, ...) __attribute__((format (printf,1,2) )); -#endif + /* No nesting is expected */ + if (GetParent(hWnd) != (HWND)lParam) return TRUE;
-static void _ReportError(const char *Format, ...) -{ - va_list ArgList; + Count = GetClassNameA(hWnd, Buffer, ARRAY_SIZE(Buffer)); + if (!Count || strcmp(Buffer, "Static")) return TRUE; + + Count = GetWindowTextA(hWnd, Buffer, ARRAY_SIZE(Buffer)); + if (!Count) return TRUE; + printf("| %s\n", Buffer);
- va_start(ArgList, Format); - printf("%s:%d: Test failed: ", LocationFile, LocationLine); - vprintf(Format, ArgList); - Failures++; + return TRUE; }
-static DWORD ConvertRVAToDiskOffset(DWORD RVA, DWORD SectionHeaderCount, const IMAGE_SECTION_HEADER *SectionHeaders) +BOOL CALLBACK DetectCriticalErrorDialog(HWND TopWnd, LPARAM lParam) { - const IMAGE_SECTION_HEADER *SectionHeader; - - for (SectionHeader = SectionHeaders; SectionHeader < SectionHeaders + SectionHeaderCount; SectionHeader++) + const char* TestFileName = (char*)lParam; + int Count, TestFileLen; + char Buffer[512]; + const char* Reason; + + /* The window pid does not match the CreateProcess() one so it cannot be + * used for filtering. But do filter on top-level dialogs. + */ + if (GetParent(TopWnd) || GetWindow(TopWnd, GW_OWNER)) return TRUE; + + Count = GetClassNameA(TopWnd, Buffer, ARRAY_SIZE(Buffer)); + if (!Count || strcmp(Buffer, "#32770")) return TRUE; + + /* The dialog title always starts with the executable name */ + TestFileLen = strlen(TestFileName); + Count = GetWindowTextA(TopWnd, Buffer, ARRAY_SIZE(Buffer)); + if (!Count || strncmp(Buffer, TestFileName, TestFileLen)) return TRUE; + + /* Followed by a reason */ + Reason = NULL; + if (strcmp(Buffer + TestFileLen, " - System Error") == 0) + Reason = "missing dll"; + else if (strcmp(Buffer + TestFileLen, " - Entry Point Not Found") == 0) + Reason = "missing entry point"; + else if (strcmp(Buffer + TestFileLen, " - Ordinal Not Found") == 0) + Reason = "missing ordinal"; + else if (strncmp(Buffer + TestFileLen, " - ", 3) == 0) + /* Old Windows version used to translate the dialog */ + Reason = "unrecognized critical error"; + + if (Reason) { - if (SectionHeader->VirtualAddress <= RVA && RVA < SectionHeader->VirtualAddress + SectionHeader->Misc.VirtualSize) - return SectionHeader->PointerToRawData + (RVA - SectionHeader->VirtualAddress); + Skips++; + printf("%s.c:0: Tests skipped: %s (details below)\n", Subtest, Reason); + printf("| %s\n", Buffer); + EnumChildWindows(TopWnd, DumpCriticalErrorDescription, (LPARAM)TopWnd); + /* Leave the dialog open so it appears on screenshots */ + return FALSE; }
- return 0; + return TRUE; }
-static BOOL DllPresent(const char *DllName) -{ - HMODULE DllModule; - - DllModule = LoadLibraryExA(DllName, NULL, LOAD_LIBRARY_AS_DATAFILE); - if (DllModule != NULL) - FreeLibrary(DllModule); - - return DllModule != NULL; -}
/* - * When launching an app that implicitly links against a DLL that's not present, a message box - * will be shown "Unable to locate component". The child process waits until this message is - * dismissed. - * This can happen when running tests for a system DLL on a Windows version which does not include - * that DLL. It messes up our testing because the child just hangs around until the timeout expires, - * taking up testing time and generating a timeout error. - * Since the message is produced by the child process before any application code is run, we can't - * suppress it using SetErrorMode() or ProcessDefaultHardErrorMode. It is possible to suppress using - * the registry value ErrorMode in HKEY_LOCAL_MACHINE\CurrentControlSet\Control\Windows but that has - * a global effect. - * So instead we just dive into the executable's import table, determine which modules are being - * imported and check if they are present. + * Command line parsing and test running. */ -static BOOL AllImportedDllsPresent(const char *TestExeName, const char *Subtest) -{ - HANDLE TestExe; - IMAGE_DOS_HEADER DosHeader; - IMAGE_NT_HEADERS NTHeaders; - const IMAGE_DATA_DIRECTORY *DataDirectoryImportTable; - IMAGE_SECTION_HEADER *SectionHeaders; - IMAGE_IMPORT_DESCRIPTOR *ImportDescriptors; - const IMAGE_IMPORT_DESCRIPTOR *ImportDescriptor; - DWORD NR; - DWORD NewPos; - DWORD FileOffset; - char ModuleName[MAX_PATH]; - BOOL Found; - BOOL AllPresent; - - TestExe = CreateFileA(TestExeName, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL); - if (TestExe == INVALID_HANDLE_VALUE) - { - ReportError("Can't open test executable %s, error %lu\n", TestExeName, GetLastError()); - return FALSE; - } - - if (! ReadFile(TestExe, &DosHeader, sizeof(IMAGE_DOS_HEADER), &NR, NULL) || NR != sizeof(IMAGE_DOS_HEADER)) - { - CloseHandle(TestExe); - ReportError("Can't read DOS header from %s, error %lu\n", TestExeName, GetLastError()); - return FALSE; - } - if (DosHeader.e_magic != IMAGE_DOS_SIGNATURE) - { - CloseHandle(TestExe); - ReportError("%s does not start with a valid DOS header\n", TestExeName); - return FALSE; - } - - NewPos = SetFilePointer(TestExe, DosHeader.e_lfanew, NULL, FILE_BEGIN); - if (NewPos == INVALID_SET_FILE_POINTER && GetLastError() != ERROR_SUCCESS) - { - CloseHandle(TestExe); - ReportError("Can't move to NT headers in %s, error %lu\n", TestExeName, GetLastError()); - return FALSE; - } - - if (! ReadFile(TestExe, &NTHeaders, sizeof(IMAGE_NT_HEADERS), &NR, NULL) || NR != sizeof(IMAGE_NT_HEADERS)) - { - CloseHandle(TestExe); - ReportError("Can't read NT headers from %s, error %lu\n", TestExeName, GetLastError()); - return FALSE; - } - if (NTHeaders.Signature != IMAGE_NT_SIGNATURE || NTHeaders.OptionalHeader.Magic != IMAGE_NT_OPTIONAL_HDR_MAGIC) - { - CloseHandle(TestExe); - ReportError("%s does not contain valid NT headers expected 0x%08x/0x%04x found 0x%08lx/0x%04x\n", TestExeName, - IMAGE_NT_SIGNATURE, IMAGE_NT_OPTIONAL_HDR_MAGIC, NTHeaders.Signature, NTHeaders.OptionalHeader.Magic); - return FALSE; - } - DataDirectoryImportTable = NTHeaders.OptionalHeader.DataDirectory + IMAGE_DIRECTORY_ENTRY_IMPORT; - if (DataDirectoryImportTable->VirtualAddress == 0 || DataDirectoryImportTable->Size == 0) - { - CloseHandle(TestExe); - ReportError("%s does not contain valid a valid import table (RVA 0x%lx size 0x%lx)\n", TestExeName, - DataDirectoryImportTable->VirtualAddress, DataDirectoryImportTable->Size); - return FALSE; - } - - SectionHeaders = (IMAGE_SECTION_HEADER*) malloc(NTHeaders.FileHeader.NumberOfSections * sizeof(IMAGE_SECTION_HEADER)); - if (SectionHeaders == NULL) - { - CloseHandle(TestExe); - ReportError("Unable to allocate memory for section headers\n"); - return FALSE; - } - if (! ReadFile(TestExe, SectionHeaders, NTHeaders.FileHeader.NumberOfSections * sizeof(IMAGE_SECTION_HEADER), &NR, NULL) || - NR != NTHeaders.FileHeader.NumberOfSections * sizeof(IMAGE_SECTION_HEADER)) - { - free(SectionHeaders); - CloseHandle(TestExe); - ReportError("Can't read section headers from %s, error %lu\n", TestExeName, GetLastError()); - return FALSE; - } - - ImportDescriptors = (IMAGE_IMPORT_DESCRIPTOR*) malloc(DataDirectoryImportTable->Size); - if (ImportDescriptors == NULL) - { - free(SectionHeaders); - CloseHandle(TestExe); - ReportError("Unable to allocate memory for import directory\n"); - return FALSE; - } - FileOffset = ConvertRVAToDiskOffset(DataDirectoryImportTable->VirtualAddress, - NTHeaders.FileHeader.NumberOfSections, SectionHeaders); - if (FileOffset == 0) - { - free(ImportDescriptors); - free(SectionHeaders); - CloseHandle(TestExe); - ReportError("Can't locate import directory in %s\n", TestExeName); - return FALSE; - } - NewPos = SetFilePointer(TestExe, FileOffset, NULL, FILE_BEGIN); - if (NewPos == INVALID_SET_FILE_POINTER && GetLastError() != ERROR_SUCCESS) - { - free(ImportDescriptors); - free(SectionHeaders); - CloseHandle(TestExe); - ReportError("Can't move to import directory in %s, error %lu\n", TestExeName, GetLastError()); - return FALSE; - } - - if (! ReadFile(TestExe, ImportDescriptors, DataDirectoryImportTable->Size, &NR, NULL) || NR != DataDirectoryImportTable->Size) - { - free(ImportDescriptors); - free(SectionHeaders); - CloseHandle(TestExe); - ReportError("Can't read import directory from %s, error %lu\n", TestExeName, GetLastError()); - return FALSE; - } - - AllPresent = TRUE; - for (ImportDescriptor = ImportDescriptors; - (char *) ImportDescriptor < (char *) ImportDescriptors + DataDirectoryImportTable->Size && ImportDescriptor->Name != 0; - ImportDescriptor++) - { - FileOffset = ConvertRVAToDiskOffset(ImportDescriptor->Name, NTHeaders.FileHeader.NumberOfSections, SectionHeaders); - if (FileOffset == 0) - { - free(ImportDescriptors); - free(SectionHeaders); - CloseHandle(TestExe); - ReportError("Can't locate import module name in %s\n", TestExeName); - return FALSE; - } - NewPos = SetFilePointer(TestExe, FileOffset, NULL, FILE_BEGIN); - if (NewPos == INVALID_SET_FILE_POINTER && GetLastError() != ERROR_SUCCESS) - { - free(ImportDescriptors); - free(SectionHeaders); - CloseHandle(TestExe); - ReportError("Can't move to import module name in %s, error %lu\n", TestExeName, GetLastError()); - return FALSE; - } - - if (! ReadFile(TestExe, ModuleName, sizeof(ModuleName), &NR, NULL)) - { - free(ImportDescriptors); - free(SectionHeaders); - CloseHandle(TestExe); - ReportError("Can't read import directory from %s, error %lu\n", TestExeName, GetLastError()); - return FALSE; - } - - Found = FALSE; - for (NewPos = 0; ! Found && NewPos < NR; NewPos++) - Found = ModuleName[NewPos] == '\0'; - if (! Found) - { - free(ImportDescriptors); - free(SectionHeaders); - CloseHandle(TestExe); - ReportError("Import module name is too long in %s\n", TestExeName); - return FALSE; - } - - if (! DllPresent(ModuleName)) - { - if (AllPresent) - { - printf("%s.c:0: Tests skipped: required DLL %s", Subtest, ModuleName); - AllPresent = FALSE; - } - else - printf(", %s", ModuleName); - } - } - - free(ImportDescriptors); - free(SectionHeaders); - CloseHandle(TestExe); - - if (! AllPresent) - { - printf(" is missing\n"); - Skips++; - } - - return AllPresent; -}
int main(int argc, char *argv[]) { int Arg; - DWORD Start, TimeOut; + DWORD Start, TimeOut, WaitTimeOut; BOOL UsageError; char TestExeFullName[MAX_PATH]; char *TestExeFileName; const char *Suffix; char TestName[MAX_PATH]; - const char *Subtest; int TestArg; char *CommandLine; int CommandLen; @@ -378,15 +198,6 @@ int main(int argc, char *argv[])
Start = GetTickCount(); printf("%s:%s start - -\n", TestName, Subtest); - - if (! AllImportedDllsPresent(TestExeFullName, Subtest)) - { - printf("0000:%s: %u tests executed (0 marked as todo, %u failures), %u skipped.\n", Subtest, Failures, Failures, Skips); - printf("%s:%s:0000 done (%u) in %lds\n", TestName, Subtest, Failures, - (GetTickCount() - Start) / 1000); - exit(0); - } - fflush(stdout);
StartupInfo.cb = sizeof(STARTUPINFOA); @@ -396,6 +207,11 @@ int main(int argc, char *argv[]) StartupInfo.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE); StartupInfo.hStdError = GetStdHandle(STD_ERROR_HANDLE);
+ /* Unlike WineTest we do not have the luxury of first running the test with + * a --list argument. This means we cannot use SetErrorMode() to check + * whether there are missing dependencies as it could modify the test + * results... + */ if (! CreateProcessA(NULL, CommandLine, NULL, NULL, TRUE, CREATE_DEFAULT_ERROR_MODE, NULL, NULL, &StartupInfo, &ProcessInformation)) { fprintf(stderr, "CreateProcess failed with error %lu\n", GetLastError()); @@ -403,7 +219,31 @@ int main(int argc, char *argv[]) } CloseHandle(ProcessInformation.hThread);
- ExitCode = WaitForSingleObject(ProcessInformation.hProcess, TimeOut); + WaitTimeOut = 100; + ExitCode = WAIT_TIMEOUT; + while (ExitCode == WAIT_TIMEOUT) + { + DWORD Elapsed, Remaining; + + ExitCode = WaitForSingleObject(ProcessInformation.hProcess, WaitTimeOut); + Elapsed = GetTickCount() - Start; + if (ExitCode != WAIT_TIMEOUT || Elapsed > TimeOut) + break; + + /* ...instead detect the critical error dialog that pops up */ + EnumWindows(DetectCriticalErrorDialog, (LPARAM)TestExeFileName); + if (Skips) + { + ExitCode = WAIT_OBJECT_0; + break; + } + + Remaining = TimeOut == INFINITE ? TimeOut : TimeOut - Elapsed; + WaitTimeOut = (Elapsed > 3000) ? Remaining : + (2 * WaitTimeOut < Remaining) ? 2 * WaitTimeOut : + Remaining; + } + if (ExitCode != WAIT_OBJECT_0) { switch (ExitCode) @@ -423,13 +263,16 @@ int main(int argc, char *argv[]) if (!TerminateProcess(ProcessInformation.hProcess, 257)) fprintf(stderr, "TerminateProcess failed, error %lu\n", GetLastError()); } - else if (!GetExitCodeProcess(ProcessInformation.hProcess, &ExitCode)) + else if (!Skips && !GetExitCodeProcess(ProcessInformation.hProcess, &ExitCode)) { fprintf(stderr, "Can't get child exit code, error %lu\n", GetLastError()); ExitCode = 259; } CloseHandle(ProcessInformation.hProcess);
+ if (Skips) + printf("%04lx:%s: 0 tests executed (0 marked as todo, 0 failures), %u skipped.\n", ProcessInformation.dwProcessId, Subtest, Skips); + printf("%s:%s:%04lx done (%ld) in %lds\n", TestName, Subtest, ProcessInformation.dwProcessId, ExitCode, (GetTickCount() - Start) / 1000);
-
Francois Gouget