What should we do if a test succeeds in a todo_wine on Mac but fails on Linux, FreeBSD and Solaris.
Specifically I'm thinking about this test: /* Check if we have some return values */ trace("WorkingSetSize : %ld\n", pvi.WorkingSetSize); todo_wine ok( pvi.WorkingSetSize > 0, "Expected a WorkingSetSize > 0\n");
Which the following commit caused to succeed on OSX:
commit 1f9fe124b2bd7e69dad5c90b7897380e32d43481 Author: Snorri Sturluson snorri.sturluson@ccpgames.com Date: Thu Jan 28 17:22:10 2016 +0000
ntdll: Fill in memory counters under OS X.
I find it pretty ironic that the OSX results, which already have more than their fair share or failures, are saddled with one extra failure when it is in fact the only Wine platform where the test succeeds!
I guess implementing fill_VM_COUNTERS() on Linux, FreeBSD and Solaris would be one solution. It might not even be that hard depending on how correct you want the implementation to be.
But do we have other solutions? I initially thought setting WINETEST_PLATFORM to Linux, FreeBSD, etc. could do the trick. But it currently it is only ever be set to Windows or Wine. Even worse, todo_wine explicitly checks if it is set to "wine" so anything else would break it. Should todo_wine check for 'not windows' instead?
On 16.02.2016 00:31, Francois Gouget wrote:
What should we do if a test succeeds in a todo_wine on Mac but fails on Linux, FreeBSD and Solaris.
Specifically I'm thinking about this test: /* Check if we have some return values */ trace("WorkingSetSize : %ld\n", pvi.WorkingSetSize); todo_wine ok( pvi.WorkingSetSize > 0, "Expected a WorkingSetSize > 0\n");
Which the following commit caused to succeed on OSX:
commit 1f9fe124b2bd7e69dad5c90b7897380e32d43481 Author: Snorri Sturluson snorri.sturluson@ccpgames.com Date: Thu Jan 28 17:22:10 2016 +0000
ntdll: Fill in memory counters under OS X.
I find it pretty ironic that the OSX results, which already have more than their fair share or failures, are saddled with one extra failure when it is in fact the only Wine platform where the test succeeds!
I guess implementing fill_VM_COUNTERS() on Linux, FreeBSD and Solaris would be one solution. It might not even be that hard depending on how correct you want the implementation to be.
But do we have other solutions? I initially thought setting WINETEST_PLATFORM to Linux, FreeBSD, etc. could do the trick. But it currently it is only ever be set to Windows or Wine. Even worse, todo_wine explicitly checks if it is set to "wine" so anything else would break it. Should todo_wine check for 'not windows' instead?
Changing the definition of todo_wine is not a problem, but in the long term a more complicated system will get very difficult to maintain.
Its not really feasible to test each commit on all supported systems {linux,mac,bsd,...} and architectures {x86,x86_64,arm,...} to find the correct combination of todo_wine_* flags/commands. In some cases, when the behavior depends on the kernel version, it gets even more complicated.
What we have to keep in mind is that the wine tests are mainly for developers, and (with minor exceptions) they probably never passed on any other machine than Alexandres developer box. Adding todo's will not make it better, so we just have to live with test failures / succeeding tests caused by different hardware and software, and manually check if its something critical or if it can be safely ignored.
On Tue, 16 Feb 2016, Francois Gouget wrote:
What should we do if a test succeeds in a todo_wine on Mac but fails on Linux, FreeBSD and Solaris.
Specifically I'm thinking about the ntdll:info test: /* Check if we have some return values */ trace("WorkingSetSize : %ld\n", pvi.WorkingSetSize); todo_wine ok( pvi.WorkingSetSize > 0, "Expected a WorkingSetSize > 0\n");
[...]
But do we have other solutions? I initially thought setting WINETEST_PLATFORM to Linux, FreeBSD, etc. could do the trick.
To be clear, I don't think we should adjust the todos for every missing library, graphics card version, etc. The tests should already deal with variations in the graphics cards behavior.
So I think this should really just be about actually being able to use winetest_platform to have a check to be a todo on just Linux or in this case non-Darwin. Here's a proposed patch (the test.h part should go in regardless):
diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c index be355fb..b81d78d 100644 --- a/dlls/ntdll/tests/info.c +++ b/dlls/ntdll/tests/info.c @@ -1062,7 +1062,7 @@ static void test_query_process_vm(void)
/* Check if we have some return values */ trace("WorkingSetSize : %ld\n", pvi.WorkingSetSize); - todo_wine + todo_wine_if(strcmp(winetest_platform, "Darwin")) { ok( pvi.WorkingSetSize > 0, "Expected a WorkingSetSize > 0\n"); } diff --git a/include/wine/test.h b/include/wine/test.h index 0d81a24..eddfd13 100644 --- a/include/wine/test.h +++ b/include/wine/test.h @@ -121,8 +121,8 @@ extern void __winetest_cdecl winetest_trace( const char *msg, ... ) WINETEST_PRI #define todo_if(is_todo) for (winetest_start_todo(is_todo); \ winetest_loop_todo(); \ winetest_end_todo()) -#define todo_wine todo_if(!strcmp(winetest_platform, "wine")) -#define todo_wine_if(is_todo) todo_if((is_todo) && !strcmp(winetest_platform, "wine")) +#define todo_wine todo_if(strcmp(winetest_platform, "windows")) +#define todo_wine_if(is_todo) todo_if((is_todo) && strcmp(winetest_platform, "windows"))
#ifdef NONAMELESSUNION diff --git a/tools/runtest b/tools/runtest index 79bd2f7..6863f9d 100755 --- a/tools/runtest +++ b/tools/runtest @@ -137,6 +137,7 @@ if [ -n "$modules" ]; then WINEDLLOVERRIDES="$WINEDLLOVERRIDES;$modules=b" export WINEDLLOVERRIDES fi +[ -z "$platform" ] || platform=`uname -s` WINETEST_PLATFORM=${platform:-wine} export WINETEST_PLATFORM WINETEST_DEBUG
If that's still deemed too complex we could also have the policy that a test that succeeds on any Wine platform should not be a todo on any Wine platform.
Francois Gouget fgouget@free.fr writes:
To be clear, I don't think we should adjust the todos for every missing library, graphics card version, etc. The tests should already deal with variations in the graphics cards behavior.
I agree, but I don't see any reason to single out the OS either. There are probably more failures caused by graphics card differences than by OS differences, that doesn't mean we want a todo for each one.
If that's still deemed too complex we could also have the policy that a test that succeeds on any Wine platform should not be a todo on any Wine platform.
The policy is that todos should be added as needed to make the tests succeed on my box, since that's what is used to ensure that committed patches don't add failures.
This means that there may be some failing todos on systems that differ from mine; the solution is not to develop more complicated todo infrastructure, it's to fix the bugs so that we can remove the todo.