[PATCH 1/2] kernel32/tests: Windows 10 dropped support for GetConsoleFontInfo().
Windows 10 1507 still supports it but not 1607+. Signed-off-by: Francois Gouget <fgouget(a)free.fr> --- I don't know what's up with returning the low word of E_NOTIMPL rather than ERROR_CALL_NOT_IMPLEMENTED. Seemed better to write it that way than to use a magic 0x4001. Note that the SetConsoleFont() tests need the same treatment so there will still be 3 or so test failures on recent Windows 10. If this approach is validated I'll send a similar patch for SetConsoleFont(). dlls/kernel32/tests/console.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index f85cd44ca73..97bea44e195 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -2865,6 +2865,14 @@ static void test_GetConsoleFontInfo(HANDLE std_output) SetLastError(0xdeadbeef); ret = pGetConsoleFontInfo(NULL, FALSE, 0, cfi); ok(!ret, "got %d, expected zero\n", ret); + if (GetLastError() == LOWORD(E_NOTIMPL) /* win10 1709+ */ || + broken(GetLastError() == ERROR_GEN_FAILURE) /* win10 1607 */) + { + skip("GetConsoleFontInfo is not implemented\n"); + SetConsoleScreenBufferSize(std_output, orig_sb_size); + HeapFree(GetProcessHeap(), 0, cfi); + return; + } todo_wine ok(GetLastError() == ERROR_INVALID_HANDLE, "got %u, expected 6\n", GetLastError()); SetLastError(0xdeadbeef); @@ -2887,14 +2895,11 @@ static void test_GetConsoleFontInfo(HANDLE std_output) memset(cfi, 0, memsize); ret = pGetConsoleFontInfo(std_output, FALSE, num_fonts, cfi); - todo_wine ok(ret || broken(!ret) /* win10 1809 */, "got %d, expected non-zero\n", ret); - if (ret) - { - todo_wine ok(cfi[index].dwFontSize.X == win_width, "got %d, expected %d\n", - cfi[index].dwFontSize.X, win_width); - todo_wine ok(cfi[index].dwFontSize.Y == win_height, "got %d, expected %d\n", - cfi[index].dwFontSize.Y, win_height); - } + todo_wine ok(ret, "got %d, expected non-zero\n", ret); + todo_wine ok(cfi[index].dwFontSize.X == win_width, "got %d, expected %d\n", + cfi[index].dwFontSize.X, win_width); + todo_wine ok(cfi[index].dwFontSize.Y == win_height, "got %d, expected %d\n", + cfi[index].dwFontSize.Y, win_height); for (i = 0; i < num_fonts; i++) { @@ -2923,14 +2928,11 @@ static void test_GetConsoleFontInfo(HANDLE std_output) memset(cfi, 0, memsize); ret = pGetConsoleFontInfo(std_output, TRUE, num_fonts, cfi); - todo_wine ok(ret || broken(!ret) /* win10 1809 */, "got %d, expected non-zero\n", ret); - if (ret) - { - todo_wine ok(cfi[index].dwFontSize.X == csbi.dwMaximumWindowSize.X, "got %d, expected %d\n", - cfi[index].dwFontSize.X, csbi.dwMaximumWindowSize.X); - todo_wine ok(cfi[index].dwFontSize.Y == csbi.dwMaximumWindowSize.Y, "got %d, expected %d\n", - cfi[index].dwFontSize.Y, csbi.dwMaximumWindowSize.Y); - } + todo_wine ok(ret, "got %d, expected non-zero\n", ret); + todo_wine ok(cfi[index].dwFontSize.X == csbi.dwMaximumWindowSize.X, "got %d, expected %d\n", + cfi[index].dwFontSize.X, csbi.dwMaximumWindowSize.X); + todo_wine ok(cfi[index].dwFontSize.Y == csbi.dwMaximumWindowSize.Y, "got %d, expected %d\n", + cfi[index].dwFontSize.Y, csbi.dwMaximumWindowSize.Y); for (i = 0; i < num_fonts; i++) { -- 2.20.1
That is return the same 'E_NOTIMPL' error code. Remove the todo_wine-s but keep the tests in case we want to implement the API for compatibility with older Windows versions. Signed-off-by: Francois Gouget <fgouget(a)free.fr> --- Again the SetConsoleFont() tests need the same treatment so there will still be 3 or so test failures on recent Windows 10. dlls/kernel32/console.c | 2 +- dlls/kernel32/tests/console.c | 42 +++++++++++++++++------------------ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/dlls/kernel32/console.c b/dlls/kernel32/console.c index 03df90c7e17..4c0bee2caf9 100644 --- a/dlls/kernel32/console.c +++ b/dlls/kernel32/console.c @@ -1711,7 +1711,7 @@ COORD WINAPI GetConsoleFontSize(HANDLE hConsole, DWORD index) BOOL WINAPI GetConsoleFontInfo(HANDLE hConsole, BOOL maximize, DWORD numfonts, CONSOLE_FONT_INFO *info) { FIXME("(%p %d %u %p): stub!\n", hConsole, maximize, numfonts, info); - SetLastError(ERROR_CALL_NOT_IMPLEMENTED); + SetLastError(LOWORD(E_NOTIMPL) /* win10 1709+ */); return FALSE; } diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 97bea44e195..a22051deafe 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -2847,7 +2847,7 @@ static void test_GetConsoleFontInfo(HANDLE std_output) pGetNumberOfConsoleFonts = (void *)GetProcAddress(hmod, "GetNumberOfConsoleFonts"); if (!pGetNumberOfConsoleFonts) { - win_skip("GetNumberOfConsoleFonts is not available\n"); + skip("GetNumberOfConsoleFonts is not available\n"); return; } @@ -2873,17 +2873,17 @@ static void test_GetConsoleFontInfo(HANDLE std_output) HeapFree(GetProcessHeap(), 0, cfi); return; } - todo_wine ok(GetLastError() == ERROR_INVALID_HANDLE, "got %u, expected 6\n", GetLastError()); + ok(GetLastError() == ERROR_INVALID_HANDLE, "got %u, expected 6\n", GetLastError()); SetLastError(0xdeadbeef); ret = pGetConsoleFontInfo(GetStdHandle(STD_INPUT_HANDLE), FALSE, 0, cfi); ok(!ret, "got %d, expected zero\n", ret); - todo_wine ok(GetLastError() == ERROR_INVALID_HANDLE, "got %u, expected 6\n", GetLastError()); + ok(GetLastError() == ERROR_INVALID_HANDLE, "got %u, expected 6\n", GetLastError()); SetLastError(0xdeadbeef); ret = pGetConsoleFontInfo(std_output, FALSE, 0, cfi); ok(!ret, "got %d, expected zero\n", ret); - todo_wine ok(GetLastError() == 0xdeadbeef, "got %u, expected 0xdeadbeef\n", GetLastError()); + ok(GetLastError() == 0xdeadbeef, "got %u, expected 0xdeadbeef\n", GetLastError()); GetConsoleScreenBufferInfo(std_output, &csbi); win_width = csbi.srWindow.Right - csbi.srWindow.Left + 1; @@ -2895,11 +2895,11 @@ static void test_GetConsoleFontInfo(HANDLE std_output) memset(cfi, 0, memsize); ret = pGetConsoleFontInfo(std_output, FALSE, num_fonts, cfi); - todo_wine ok(ret, "got %d, expected non-zero\n", ret); - todo_wine ok(cfi[index].dwFontSize.X == win_width, "got %d, expected %d\n", - cfi[index].dwFontSize.X, win_width); - todo_wine ok(cfi[index].dwFontSize.Y == win_height, "got %d, expected %d\n", - cfi[index].dwFontSize.Y, win_height); + ok(ret, "got %d, expected non-zero\n", ret); + ok(cfi[index].dwFontSize.X == win_width, "got %d, expected %d\n", + cfi[index].dwFontSize.X, win_width); + ok(cfi[index].dwFontSize.Y == win_height, "got %d, expected %d\n", + cfi[index].dwFontSize.Y, win_height); for (i = 0; i < num_fonts; i++) { @@ -2907,32 +2907,32 @@ static void test_GetConsoleFontInfo(HANDLE std_output) tmp_font = GetConsoleFontSize(std_output, cfi[i].nFont); tmp_w = (double)orig_font.X / tmp_font.X * win_width; tmp_h = (double)orig_font.Y / tmp_font.Y * win_height; - todo_wine ok(cfi[i].dwFontSize.X == tmp_w, "got %d, expected %d\n", cfi[i].dwFontSize.X, tmp_w); - todo_wine ok(cfi[i].dwFontSize.Y == tmp_h, "got %d, expected %d\n", cfi[i].dwFontSize.Y, tmp_h); + ok(cfi[i].dwFontSize.X == tmp_w, "got %d, expected %d\n", cfi[i].dwFontSize.X, tmp_w); + ok(cfi[i].dwFontSize.Y == tmp_h, "got %d, expected %d\n", cfi[i].dwFontSize.Y, tmp_h); } SetLastError(0xdeadbeef); ret = pGetConsoleFontInfo(NULL, TRUE, 0, cfi); ok(!ret, "got %d, expected zero\n", ret); - todo_wine ok(GetLastError() == ERROR_INVALID_HANDLE, "got %u, expected 6\n", GetLastError()); + ok(GetLastError() == ERROR_INVALID_HANDLE, "got %u, expected 6\n", GetLastError()); SetLastError(0xdeadbeef); ret = pGetConsoleFontInfo(GetStdHandle(STD_INPUT_HANDLE), TRUE, 0, cfi); ok(!ret, "got %d, expected zero\n", ret); - todo_wine ok(GetLastError() == ERROR_INVALID_HANDLE, "got %u, expected 6\n", GetLastError()); + ok(GetLastError() == ERROR_INVALID_HANDLE, "got %u, expected 6\n", GetLastError()); SetLastError(0xdeadbeef); ret = pGetConsoleFontInfo(std_output, TRUE, 0, cfi); ok(!ret, "got %d, expected zero\n", ret); - todo_wine ok(GetLastError() == 0xdeadbeef, "got %u, expected 0xdeadbeef\n", GetLastError()); + ok(GetLastError() == 0xdeadbeef, "got %u, expected 0xdeadbeef\n", GetLastError()); memset(cfi, 0, memsize); ret = pGetConsoleFontInfo(std_output, TRUE, num_fonts, cfi); - todo_wine ok(ret, "got %d, expected non-zero\n", ret); - todo_wine ok(cfi[index].dwFontSize.X == csbi.dwMaximumWindowSize.X, "got %d, expected %d\n", - cfi[index].dwFontSize.X, csbi.dwMaximumWindowSize.X); - todo_wine ok(cfi[index].dwFontSize.Y == csbi.dwMaximumWindowSize.Y, "got %d, expected %d\n", - cfi[index].dwFontSize.Y, csbi.dwMaximumWindowSize.Y); + ok(ret, "got %d, expected non-zero\n", ret); + ok(cfi[index].dwFontSize.X == csbi.dwMaximumWindowSize.X, "got %d, expected %d\n", + cfi[index].dwFontSize.X, csbi.dwMaximumWindowSize.X); + ok(cfi[index].dwFontSize.Y == csbi.dwMaximumWindowSize.Y, "got %d, expected %d\n", + cfi[index].dwFontSize.Y, csbi.dwMaximumWindowSize.Y); for (i = 0; i < num_fonts; i++) { @@ -2940,8 +2940,8 @@ static void test_GetConsoleFontInfo(HANDLE std_output) tmp_font = GetConsoleFontSize(std_output, cfi[i].nFont); tmp_w = (double)orig_font.X / tmp_font.X * csbi.dwMaximumWindowSize.X; tmp_h = (double)orig_font.Y / tmp_font.Y * csbi.dwMaximumWindowSize.Y; - todo_wine ok(cfi[i].dwFontSize.X == tmp_w, "got %d, expected %d\n", cfi[i].dwFontSize.X, tmp_w); - todo_wine ok(cfi[i].dwFontSize.Y == tmp_h, "got %d, expected %d\n", cfi[i].dwFontSize.Y, tmp_h); + ok(cfi[i].dwFontSize.X == tmp_w, "got %d, expected %d\n", cfi[i].dwFontSize.X, tmp_w); + ok(cfi[i].dwFontSize.Y == tmp_h, "got %d, expected %d\n", cfi[i].dwFontSize.Y, tmp_h); } HeapFree(GetProcessHeap(), 0, cfi); -- 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=59315 Your paranoid android. === debian10 (32 bit report) === kernel32: change.c:320: Test failed: should be ready debugger: Timeout === debian10 (64 bit WoW report) === kernel32: debugger.c:320: Test failed: GetThreadContext failed: 5 debugger.c:320: Test failed: GetThreadContext failed: 5
tl;dr; Failures not caused by my patch! First the reason why the TestBot reran all of the kernel32 tests on Wine is that I patched Wine's kernel32 dll. The TestBot does not really understand what the patch does so it assume that could break any of the dll's tests. In fact a change in kernel32 could break pretty much any Wine test so to be thorough all the tests should be rerun. But while technically the TestBot has been able to do this for over a year, rerunning all the tests would take a lot of time (requiring more hardware resources), and, more importantly, as this case shows the quality of the Wine tests is just too poor for this to be workable. Marvin wrote:
=== debian10 (32 bit report) === [...] debugger: Timeout
I'm starting the analysis with this timeout. It's intermittent. The exception that preceded the timeout is pretty frequent, but the timeout happens less frequently. The last time it happened in this VM configuration was on October 31st: https://test.winehq.org/data/ccec532879ec14b2e79da08288152a69221ec4d1/linux_... It is to avoid these false positives I'm working on bug 47998. https://bugs.winehq.org/show_bug.cgi?id=47998 [...]
kernel32: change.c:320: Test failed: should be ready
This failure too is intermittent: https://test.winehq.org/data/tests/kernel32:change.html But it's much rarer. So much so that it never happened in this VM configuration, i.e. debian10 + win32 build + English, in the last 29 WineTest runs on record. It did happen four times in other configurations though: Oct 31 - debian10 + win32 build + French Oct 27 - debian10 + win32 build + Japanese Oct 22 - debian10 + wow64 build + English Sep 25 - debian10 + win32 build + Japanese Based on the test it does not look like the language would have an impact on this test so this failure could really happen at any time in any kernel32:change run. But the TestBot cannot know that and, since this failure did not happen in this specific configuration in the history available to the TestBot, a fix for bug 47998 would not help with this false positive. Sometimes fixing an intermittent failure is the only solution for cleaner test runs.
=== debian10 (64 bit WoW report) ===
kernel32: debugger.c:320: Test failed: GetThreadContext failed: 5 debugger.c:320: Test failed: GetThreadContext failed: 5
This might be confusing. The TestBot used the November 5 WineTest run as a reference which did have two of these failures. So why is it reporting these two failures as new? That's because this run has 4 such failures in total. So the TestBot ignores the first two but correctly reports the next two as new. In fact this failure happens a variable number of times because it's called for each of the process' threads and fails randomly. https://source.winehq.org/git/wine.git/blob/5b62f89baa82daecd430897de0bb5cab... On Oct 28 it happened 5 times so a fix for bug 47998 would have avoided this particular false positive. But there's always the risk of having one more failure than the historical record. Should the TestBot deduplicate the failure lines to avoid such issues? (*) Duplicate failures (identical line number and failure) typically happen when a test has an array containing test information and loops on it. In such a case a patch adding a new entry that causes an extra failure should be flagged which would not happen if the TestBot is deduplicating the failures. But such loops should contain the index of the tested array entry in their message which would thwart the deduplication. The kernel32:debugger test is a special case in that there does not seem to be a meaningful way to differentiate the failures so it's a good case for deduplicating. (*) If it is to be implemented the deduplication should be performed while extrating the failures from the test report. That's because once the failures have been extracted, any trace that we present between them has been lost. But such traces provide context and thus should prevent the deduplication from happening. This is also a place where line numbers can be taken into account (they are ignored when comparing failures since patches will change them). -- Francois Gouget <fgouget(a)codeweavers.com>
participants (3)
-
Francois Gouget -
Francois Gouget -
Marvin