On 4/23/2010 16:50, Gerald Pfeifer wrote:
In addition to just building this patch as is, I also gave it a spin changing the if (0) to if (1) to ensure this still builds (it should, of course, and it does, but...)
Gerald
ChangeLog: Remove variable res which is not really used from test_XcvClosePort.
We should better test for return value than remove it.
On Sat, 24 Apr 2010, Nikolay Sivov wrote:
ChangeLog: Remove variable res which is not really used from test_XcvClosePort.
We should better test for return value than remove it.
Okay. Is this something you can look into (or do you have a hint on how you'd like such a test to look like)? It would be great could you make this change, but if you have guidance I will do my best to give it a try, too.
Gerald
On 5/1/2010 21:52, Gerald Pfeifer wrote:
On Sat, 24 Apr 2010, Nikolay Sivov wrote:
ChangeLog: Remove variable res which is not really used from test_XcvClosePort.
We should better test for return value than remove it.
Okay. Is this something you can look into (or do you have a hint on how you'd like such a test to look like)? It would be great could you make this change, but if you have guidance I will do my best to give it a try, too.
Gerald
This is a primitive thing, try it if you didn't before. All you need is to add some lines like: --- ok(res == ERROR_SUCCESS /*a proper code here*/, "expected ERROR_SUCCESS, got %d\n", res); --- After that run a test on Windows and Wine (using a bot for example) and make corrections if needed.
On Sat, 1 May 2010, Nikolay Sivov wrote:
ChangeLog: Remove variable res which is not really used from test_XcvClosePort.
We should better test for return value than remove it.
This is a primitive thing, try it if you didn't before. All you need is to add some lines like:
ok(res == ERROR_SUCCESS /*a proper code here*/, "expected ERROR_SUCCESS, got %d\n", res);
After that run a test on Windows and Wine (using a bot for example) and make corrections if needed.
Aaactually, now that I was going to look into this, I recall why I didn't consider this originally: The entire code in that function is inside an if (0) { }... :-o
But, since you asked for it, I added such tests across the board, made one simplification, verified that it actually (if enabled) passes properly on Wine and will submit this in a minute. Then we have things in place when the FIXME is addressed. Plus a learned a thing or two in the process. ;-)
Thanks for your continous feedbac, Nikolay!
Gerlald
-
Gerald Pfeifer -
Nikolay Sivov