Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty
Wolfgang Walter <wine(a)stwm.de> wrote:
With my patches 1/2 and 2/2 I get:
fixme:iphlpapi:NotifyAddrChange (Handle 0x10ee8e0, overlapped 0x10ee8ec): stub wine: configuration in '/home/walterw/.wine' has been updated. fixme:comm:set_queue_size insize 1024 outsize 1024 unimplemented stub comm.c:857: Test succeeded inside todo block: WaitCommEvent failed with a timeout comm.c:881: Test succeeded inside todo block: WaitCommEvent error 997 comm.c:883: Test succeeded inside todo block: WaitCommEvent: expected EV_TXEMPTY, got 0x4 comm.c:889: Test succeeded inside todo block: WaitCommEvent used 1141 ms for waiting err:comm:set_line_control ByteSize fixme:ntdll:server_ioctl_file Unsupported ioctl 1b000c (device=1b access=0 func=3 method=0) comm.c:2036: Tests skipped: interactive tests (set WINETEST_INTERACTIVE=1)
Successes inside of todo_wine blocks are treated as failres.
So you think I should remove the wine_todos already here?
No, the source of the failures is still there.
What do you mean with that? The tests indeed do succeed now and there is a reason they do: when you call WaitCommEvent() while the tx buffer is not empty yet the wine code will detect that EV_TXEMPTY correctly:
The tests must pass under Wine without any additional "fixes" as they do currently under Windows. If you add some code to the tests which suddenly makes them pass under Wine - that's not a fix, Wine implementation should be fixed instead. -- Dmitry.
On 05 Sep 2013, at 16:32, Dmitry Timoshkov wrote:
The tests must pass under Wine without any additional "fixes" as they do currently under Windows. If you add some code to the tests which suddenly makes them pass under Wine - that's not a fix, Wine implementation should be fixed instead.
What I understood from Wolfgang's previous explanations is that there is currently this situation: todo_wine test1(); /* test1 fails because wine has a bug. As a side-effect, bytes will be left in the transmission buffer under wine, but not under windows. */ test2(); /* test2 also fails under wine because of the failure of test1 that influences the behaviour of test2, not because the functionality tested by test2 is broken. This make it impossible to individually evaluate test1() and test2(). */ So either * you add code in between that resets the state so that test2 can be run independently of whether test1 failed (as Wolfgang proposes), or * you put the tests in completely different executables so they don't influence each other (hopefully), or * you reorder the test so that first all the tests that currently work under wine are run (under the assumption that no regression will ever creep in), or * you require that bugs in wine routines are fixed in the order that they are called in that test (which would be strange) Jonas
Jonas Maebe <jonas.maebe(a)elis.ugent.be> wrote:
The tests must pass under Wine without any additional "fixes" as they do currently under Windows. If you add some code to the tests which suddenly makes them pass under Wine - that's not a fix, Wine implementation should be fixed instead.
What I understood from Wolfgang's previous explanations is that there is currently this situation:
todo_wine test1(); /* test1 fails because wine has a bug. As a side-effect, bytes will be left in the transmission buffer under wine, but not under windows. */ test2(); /* test2 also fails under wine because of the failure of test1 that influences the behaviour of test2, not because the functionality tested by test2 is broken. This make it impossible to individually evaluate test1() and test2(). */
So either * you add code in between that resets the state so that test2 can be run independently of whether test1 failed (as Wolfgang proposes), or * you put the tests in completely different executables so they don't influence each other (hopefully), or * you reorder the test so that first all the tests that currently work under wine are run (under the assumption that no regression will ever creep in), or * you require that bugs in wine routines are fixed in the order that they are called in that test (which would be strange)
How about a patch set which does: 1/2. add a workaround to the tests to pass under Wine 2/2. fix Wine code and simultaneously remove the workaround added by 1/2. But that looks even more strange if the fix contained in 2/2 could be sent alone. -- Dmitry.
On 05 Sep 2013, at 16:58, Dmitry Timoshkov wrote:
Jonas Maebe <jonas.maebe(a)elis.ugent.be> wrote:
So either * you add code in between that resets the state so that test2 can be run independently of whether test1 failed (as Wolfgang proposes), or * you put the tests in completely different executables so they don't influence each other (hopefully), or * you reorder the test so that first all the tests that currently work under wine are run (under the assumption that no regression will ever creep in), or * you require that bugs in wine routines are fixed in the order that they are called in that test (which would be strange)
How about a patch set which does: 1/2. add a workaround to the tests to pass under Wine 2/2. fix Wine code and simultaneously remove the workaround added by 1/2.
But that looks even more strange if the fix contained in 2/2 could be sent alone.
I disagree it's a "workaround to make the tests pass under Wine". It's simply good practice to make supposedly unrelated tests actually independent from each other. The fact that this makes some extra tests succeed under Wine simply means that the test code was broken before (unless the only goal was to test whether everything works or not everything works, as opposed to testing whether individual functionality works). And when a regression occurs afterwards, it will also be easier to see what exactly broke by the regression rather than having to add all that code again to make the tests independent from each other once more. Jonas
Am Donnerstag, 5. September 2013, 23:58:57 schrieb Dmitry Timoshkov:
Jonas Maebe <jonas.maebe(a)elis.ugent.be> wrote:
The tests must pass under Wine without any additional "fixes" as they do currently under Windows. If you add some code to the tests which suddenly makes them pass under Wine - that's not a fix, Wine implementation should be fixed instead.
What I understood from Wolfgang's previous explanations is that there is currently this situation:
todo_wine
test1();
/* test1 fails because wine has a bug. As a side-effect, bytes will be left in the transmission buffer under wine, but not under windows. */ test2(); /* test2 also fails under wine because of the failure of test1 that influences the behaviour of test2, not because the functionality tested by test2 is broken. This make it impossible to individually evaluate test1() and test2(). */
So either * you add code in between that resets the state so that test2 can be run independently of whether test1 failed (as Wolfgang proposes), or * you put the tests in completely different executables so they don't influence each other (hopefully), or * you reorder the test so that first all the tests that currently work under wine are run (under the assumption that no regression will ever creep in), or * you require that bugs in wine routines are fixed in the order that they are called in that test (which would be strange) How about a patch set which does: 1/2. add a workaround to the tests to pass under Wine 2/2. fix Wine code and simultaneously remove the workaround added by 1/2.
I don't have a fix for WaitCommEvent() does not return ERROR_INVALID_PARAMETER without ovl structure And I had no plans to do so nor did I claim. I removed the side effect that this test makes the next test fail which tests the EV_TXEMPTY handling even if that it would work perfectly. So: test A test B test A fails on wine and this influences test B in such a way that it will always fail, too. I remove this dependency. Now test B succeeds. This is not because I worked arround a wine-bug in what B should test but because wine behaves correctly in this test. And test A still fails.
But that looks even more strange if the fix contained in 2/2 could be sent alone.
Regards, -- Wolfgang Walter Studentenwerk München Anstalt des öffentlichen Rechts Abteilungsleiter IT Leopoldstraße 15 80802 München
Am Donnerstag, 5. September 2013, 23:32:00 schrieben Sie:
Wolfgang Walter <wine(a)stwm.de> wrote:
With my patches 1/2 and 2/2 I get:
fixme:iphlpapi:NotifyAddrChange (Handle 0x10ee8e0, overlapped 0x10ee8ec): stub wine: configuration in '/home/walterw/.wine' has been updated. fixme:comm:set_queue_size insize 1024 outsize 1024 unimplemented stub comm.c:857: Test succeeded inside todo block: WaitCommEvent failed with a timeout comm.c:881: Test succeeded inside todo block: WaitCommEvent error 997 comm.c:883: Test succeeded inside todo block: WaitCommEvent: expected EV_TXEMPTY, got 0x4 comm.c:889: Test succeeded inside todo block: WaitCommEvent used 1141 ms for waiting err:comm:set_line_control ByteSize fixme:ntdll:server_ioctl_file Unsupported ioctl 1b000c (device=1b access=0 func=3 method=0) comm.c:2036: Tests skipped: interactive tests (set WINETEST_INTERACTIVE=1)
Successes inside of todo_wine blocks are treated as failres.
So you think I should remove the wine_todos already here?
No, the source of the failures is still there.
What do you mean with that? The tests indeed do succeed now and there is a reason they do: when you call WaitCommEvent() while the tx buffer is not empty yet the wine code will detect that EV_TXEMPTY correctly: The tests must pass under Wine without any additional "fixes" as they do currently under Windows. If you add some code to the tests which suddenly makes them pass under Wine - that's not a fix, Wine implementation should be fixed instead.
You basically say that one may not fix a bug before fixing another one which is not related only because they are tested for in a special order and these tests influence each other though they are not really related. Again: In this case not wine behaves incorrectly but the test is simply wrong. It first tests if WaitCommEvent() returns ERROR_INVALID_PARAMETER if called with NULL without without ovl structure. Then it tests the EV_TXEMPTY handling. The latter test will always fail even if it works perfectly as long as wine does not return ERROR_INVALID_PARAMETER but instead does the write in the first test. This is because the second tests does not wait long enough for both writes to complete. Regards, -- Wolfgang Walter Studentenwerk München Anstalt des öffentlichen Rechts Abteilungsleiter IT Leopoldstraße 15 80802 München
Wolfgang Walter <wine(a)stwm.de> wrote:
No, the source of the failures is still there.
What do you mean with that? The tests indeed do succeed now and there is a reason they do: when you call WaitCommEvent() while the tx buffer is not empty yet the wine code will detect that EV_TXEMPTY correctly: The tests must pass under Wine without any additional "fixes" as they do currently under Windows. If you add some code to the tests which suddenly makes them pass under Wine - that's not a fix, Wine implementation should be fixed instead.
You basically say that one may not fix a bug before fixing another one which is not related only because they are tested for in a special order and these tests influence each other though they are not really related.
Adding a workaround to the tests to compensate a Wine bug and as a side effect remove some todo_wines is not a fix. Yes, some tests depend on each other, but that's on purpose, there is no need to break this dependency just because you can make some later tests suddenly "pass". -- Dmitry.
participants (3)
-
Dmitry Timoshkov -
Jonas Maebe -
Wolfgang Walter