October 2007 - Wine-Devel - list.winehq.org

Re: user32: Fix a test that now passes in Windows

by Reece Dunn

On 26/10/2007, James Hawkins <truiken(a)gmail.com> wrote: > On 10/25/07, Reece Dunn <msclrhd(a)googlemail.com> wrote: > > On 25/10/2007, James Hawkins wrote: > > > Hi, > > > > > > Changelog: > > > * Fix a test that now passes in Windows. > > > > Looking at the code, it looks as if they are now passing in Wine :). > > > > I don't think you are looking at the code, nor does it seem you've run > the tests. The tests that now pass in Wine are moved out of the > todo_wine block, and the one remaining failing test is still in the > todo_wine block. Looking at the code (i.e. the supplied patch), the change that is "Fix a test that now passes in Windows" is: - ok(res == 0xdeadbeef, "Expected 0xdeadbeef, got %08x\n", res); + ok(hdata != NULL, "Expected non-NULL hdata, got %p\n", hdata); + ok(ret == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", ret); todo_wine { - ok(hdata == NULL, "Expected NULL hdata, got %p\n", hdata); - ok(ret == DMLERR_DATAACKTIMEOUT, "Expected DMLERR_DATAACKTIMEOUT, got %d\n", ret); + ok(res == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %08x\n", res); } Because the changes to hdata, res and ret have modified what passes on Wine, they have been rearranged. Therefore, my comment there was wrong. > > > dlls/user32/tests/dde.c | 13 ++++++++++--- > > > 1 files changed, 10 insertions(+), 3 deletions(-) > > > > > + str = (LPSTR)DdeAccessData(hdata, &size); > > > + ok(!lstrcmpA(str, "requested data\r\n"), "Expected 'requested data\\r\\n', got %s\n", str); > > > > Shouldn't there be quote marks around the %s (i.e. '%s', to match the > > expected value)? > > > > No. The quotes around 'requested data\\r\\n' are for ease of reading the code. What if str contains newline characters that don't match the expected case? It would be easier to see that if the string was quoted. > > > + ok(size == 19, "Expected 19, got %d\n", size); > > > + > > > + ret = DdeUnaccessData(hdata); > > > + ok(ret == TRUE, "Expected TRUE, got %d\n", ret); > > > > These are new test cases. Since these are not fixes to existing test > > cases, they should really be in a separate patch. > > > > Huh? You're really coming out of right field. I don't think it needs > explanation, but you don't seem to get it. It's not a 'new' test > case, it's a part of the test block 'XTYP_REQUEST, fAckReq = TRUE'. I was referring to the last hunk in that patch. These are /new/ lines of code that add /new/ ok checks (referring to them as test cases was bad phrasing on my part). > The call used to fail before in Windows, but was fixed by a timing > patch committed by Julliard. There's no point in checking the > requested data if the call fails. Now the call doesn't fail, so we > check the data. Ok. - Reece

Oct. 26, 2007

Beer apps under Wine :-)

by Dan Kegel

Seems that most beer apps run ok on wine, judging by this thread: http://www.tastybrew.com/forum/thread/120190

Oct. 26, 2007

Re: user32: Fix a test that now passes in Windows

by James Hawkins

On 10/25/07, Reece Dunn <msclrhd(a)googlemail.com> wrote: > On 25/10/2007, James Hawkins wrote: > > Hi, > > > > Changelog: > > * Fix a test that now passes in Windows. > > Looking at the code, it looks as if they are now passing in Wine :). > I don't think you are looking at the code, nor does it seem you've run the tests. The tests that now pass in Wine are moved out of the todo_wine block, and the one remaining failing test is still in the todo_wine block. > > dlls/user32/tests/dde.c | 13 ++++++++++--- > > 1 files changed, 10 insertions(+), 3 deletions(-) > > > + str = (LPSTR)DdeAccessData(hdata, &size); > > + ok(!lstrcmpA(str, "requested data\r\n"), "Expected 'requested data\\r\\n', got %s\n", str); > > Shouldn't there be quote marks around the %s (i.e. '%s', to match the > expected value)? > No. The quotes around 'requested data\\r\\n' are for ease of reading the code. > > + ok(size == 19, "Expected 19, got %d\n", size); > > + > > + ret = DdeUnaccessData(hdata); > > + ok(ret == TRUE, "Expected TRUE, got %d\n", ret); > > These are new test cases. Since these are not fixes to existing test > cases, they should really be in a separate patch. > Huh? You're really coming out of right field. I don't think it needs explanation, but you don't seem to get it. It's not a 'new' test case, it's a part of the test block 'XTYP_REQUEST, fAckReq = TRUE'. The call used to fail before in Windows, but was fixed by a timing patch committed by Julliard. There's no point in checking the requested data if the call fails. Now the call doesn't fail, so we check the data. Please read the code thoroughly before making comments like these. -- James Hawkins

Oct. 26, 2007

Re: Improving http://tests.winehq.org/data/

by Dan Kegel

On 10/25/07, James Hawkins <truiken(a)gmail.com> wrote: > > 1) runtest could take an option --skip-long-tests > > which would skip all tests that had that option set, and > > I don't think that's fair to long tests, say msi:install. There will > always be people that don't want to wait for the tests, and thus the > long tests get less exposure. But it's common practice in the testing community to offer a "quick smoke test" along with an exhaustive test. I'm just proposing we do that. If "make test" is fast enough, people will use it more frequently. I think that makes up for the "long tests aren't run as often" effect. > In the case of msi:install, there's > technically nothing wrong with the tests, so it should get just as > much exposure as any other test. Just today I noticed a bug in > msi:install that I didn't know was there before because the tests were > timing out. Like I said before, we shouldn't specify a limit for > certain tests like msi:install because they'll always get longer and > we'll always be chasing a moving target.

Oct. 26, 2007

What if heap errors caused 'make test' to fail?

by Dan Kegel

Let's just say, for grins, that we applied a patch that made heap corruption fatal under WINEDEBUG=warn+heap, e.g. --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -909,6 +909,7 @@ static BOOL HEAP_ValidateInUseArena( con WARN( "Heap %p: unaligned arena pointer %p\n", subheap->heap, pArena ); if ( TRACE_ON(heap) ) HEAP_Dump( subheap->heap ); + assert( FALSE ); } return FALSE; } @@ -924,6 +925,7 @@ static BOOL HEAP_ValidateInUseArena( con WARN("Heap %p: invalid in-use arena magic for %p\n", subheap->heap, pArena ); if (TRACE_ON(heap)) HEAP_Dump( subheap->heap ); + assert( FALSE ); } return FALSE; } How many tests would that break? Any guesses? Scroll down for the answer: 25! Here's the output of WINEDEBUG=warn+heap make -k test piped through grep make.*Error: make[2]: *** [alloc.ok] Error 134 make[2]: *** [pipe.ok] Error 1 make[2]: *** [systray.ok] Error 6 make[2]: *** [thread.ok] Error 9 make[2]: *** [security.ok] Error 1 make[1]: *** [advapi32/tests/__test__] Error 2 make[2]: *** [info.ok] Error 134 make[1]: *** [ntdll/tests/__test__] Error 2 make[2]: *** [heap.ok] Error 134 make[2]: *** [change.ok] Error 2 make[2]: *** [heap.ok] Error 5 make[2]: *** [util.ok] Error 134 make[1]: *** [msvcrt/tests/__test__] Error 2 make[1]: *** [snmpapi/tests/__test__] Error 2 make[2]: *** [apibuf.ok] Error 134 make[1]: *** [netapi32/tests/__test__] Error 2 make[2]: *** [device.ok] Error 4 make[1]: *** [d3d9/tests/__test__] Error 2 make[2]: *** [moniker.ok] Error 134 make[2]: *** [toolhelp.ok] Error 8 make[2]: *** [webbrowser.ok] Error 134 make[1]: *** [shdocvw/tests/__test__] Error 2 make[2]: *** [input.ok] Error 64 make[2]: *** [editor.ok] Error 5 make[1]: *** [riched20/tests/__test__] Error 2 make[2]: *** [mixer.ok] Error 10 make[2]: *** [msg.ok] Error 18 make[2]: *** [shellpath.ok] Error 3 make[2]: *** [class.ok] Error 2 make[2]: *** [comm.ok] Error 3 make[2]: *** [vartype.ok] Error 134 make[1]: *** [oleaut32/tests/__test__] Error 2 make[2]: *** [parser.ok] Error 134 make[1]: *** [setupapi/tests/__test__] Error 2 make[2]: *** [win.ok] Error 8 make[2]: *** [capture.ok] Error 134 make[1]: *** [dsound/tests/__test__] Error 2 make[2]: *** [htmldoc.ok] Error 134 make[2]: *** [timer.ok] Error 1 make[2]: *** [mci.ok] Error 134 make[1]: *** [mshtml/tests/__test__] Error 2 make[2]: *** [font.ok] Error 26 make[1]: *** [gdi32/tests/__test__] Error 2 make[1]: *** [kernel32/tests/__test__] Error 2 make[2]: *** [wave.ok] Error 8 make[1]: *** [winmm/tests/__test__] Error 2 make[2]: *** [server.ok] Error 4 make[1]: *** [rpcrt4/tests/__test__] Error 2 make[2]: *** [dde.ok] Error 5 make[1]: *** [user32/tests/__test__] Error 2 make[2]: *** [shlexec.ok] Error 35 make[1]: *** [shell32/tests/__test__] Error 2 make[1]: *** [ole32/tests/__test__] Error 2 I wonder if some of these tests are themselves buggy such that they pass on Wine by accident (without this patch) but fail on Windows? - Dan

Oct. 26, 2007

Re: Improving http://tests.winehq.org/data/

by James Hawkins

On 10/25/07, Dan Kegel <dank(a)kegel.com> wrote: > James wrote: > >Looking at the test data, all of the msi:install tests timeout. I > >just ran the install tests in XP running under vmware on a 3ghz > >machine. The tests took 9m41s. That completely blows away the 2min > >timeout. There's nothing wrong with the tests, they just take a long > >time. I don't think we should extend the timeout, because it's very > >subjective and more tests will be added, meaning we'll have to change > >the timeout eventually. I do think we should have a flag or variable > >that allows the timeout to be ignored for certain tests. Any > >opinions? > > I'd like a way to specify the expected runtime, > for use on tests that we observe take a long time. > It would be used in two ways: > 1) runtest could take an option --skip-long-tests > which would skip all tests that had that option set, and > 2) by default runtest would only abort those tests > if they took one minute over their expected time (say). > > That would give us both a quick interactive make test > and a more reliable slow-but-complete make test. > - Dan > I don't think that's fair to long tests, say msi:install. There will always be people that don't want to wait for the tests, and thus the long tests get less exposure. In the case of msi:install, there's technically nothing wrong with the tests, so it should get just as much exposure as any other test. Just today I noticed a bug in msi:install that I didn't know was there before because the tests were timing out. Like I said before, we shouldn't specify a limit for certain tests like msi:install because they'll always get longer and we'll always be chasing a moving target. -- James Hawkins

Oct. 25, 2007

Re: ddraw:d3d tests failing consistently

by Stefan Dösinger

Am Freitag, 26. Oktober 2007 01:54:59 schrieb Reece Dunn: > This is true. The tests should only check for valid Windows results. > > The problem is how to filter out the noise of the VMware bugs from > actual failures in Windows for the Wine tests. This is so that we know > which tests need fixing (so that the Wine behaviour can be corrected), > and those that don't. > > Thinking about this, my idea of updating the tests for the broken > VMware implementation was wrong. However, I don't have a better > solution. I guess the only way is to skip ddraw/d3d tests entirely on vmware, or disable 3D acceleration in vmware, then the tests should skip themselves or run with the reference rasterizer. > > Appart of that, with these specific tests I have seen one failure on a > > real windows box too, perhaps I should loosen the test precision on some > > of those tests(This was in the area of 0.000x, not a difference of 129 > > and 133). > > Having a +/- 0.000x tolerance is sensible. Yeah, I have that, perhaps it is not enough. I didn't investigate that yet, though

Oct. 25, 2007

Re: ddraw:d3d tests failing consistently

by Reece Dunn

On 26/10/2007, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > Am Freitag, 26. Oktober 2007 01:22:22 schrieb Reece Dunn: > > Also, I suspect that it would be worth adding vmware in the test > > cases. That is, something like: > > > > ok( ret == 129 /* Windows */ || ret == 133 /* VMware */, ... ); > > > > This would mean that the tests that are consistently (and predictably) > > failing under vmware will not show up on the test failures, so we can > > investigate actual failures. > Well, then we could add a ret == 666 /* Parallels */ too... I am not really > fond of "fixing" our tests to succeed in vmware, I'd rather skip them > entirely. Although the technically correct thing to do is to just let them > fail. VMware has a broken d3d implementation, our tests detect that, mission > accomplished. This is true. The tests should only check for valid Windows results. The problem is how to filter out the noise of the VMware bugs from actual failures in Windows for the Wine tests. This is so that we know which tests need fixing (so that the Wine behaviour can be corrected), and those that don't. Thinking about this, my idea of updating the tests for the broken VMware implementation was wrong. However, I don't have a better solution. > Appart of that, with these specific tests I have seen one failure on a real > windows box too, perhaps I should loosen the test precision on some of those > tests(This was in the area of 0.000x, not a difference of 129 and 133). Having a +/- 0.000x tolerance is sensible. - Reece

Oct. 25, 2007

Re: ddraw:d3d tests failing consistently

by Stefan Dösinger

Am Freitag, 26. Oktober 2007 01:22:22 schrieb Reece Dunn: > Also, I suspect that it would be worth adding vmware in the test > cases. That is, something like: > > ok( ret == 129 /* Windows */ || ret == 133 /* VMware */, ... ); > > This would mean that the tests that are consistently (and predictably) > failing under vmware will not show up on the test failures, so we can > investigate actual failures. Well, then we could add a ret == 666 /* Parallels */ too... I am not really fond of "fixing" our tests to succeed in vmware, I'd rather skip them entirely. Although the technically correct thing to do is to just let them fail. VMware has a broken d3d implementation, our tests detect that, mission accomplished. Appart of that, with these specific tests I have seen one failure on a real windows box too, perhaps I should loosen the test precision on some of those tests(This was in the area of 0.000x, not a difference of 129 and 133).

Oct. 25, 2007

Re: ddraw:d3d tests failing consistently

by Reece Dunn

On 26/10/2007, Francois Gouget <fgouget(a)free.fr> wrote: > > Would it be possible to gather this kind of information in winetest.exe? > Maybe simply grab the name of the graphics card? This would be useful. Also, I suspect that it would be worth adding vmware in the test cases. That is, something like: ok( ret == 129 /* Windows */ || ret == 133 /* VMware */, ... ); This would mean that the tests that are consistently (and predictably) failing under vmware will not show up on the test failures, so we can investigate actual failures. - Reece

Oct. 25, 2007