James Hawkins wrote:
Hi,
Changelog:
- Fix a test that fails on all NT platforms.
dlls/kernel32/tests/toolhelp.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
Hi James,
I've been looking at some of the patches to fix the tests but I'm a bit unsure if some of them are the correct way to go. Things like:
+ /* win9x does not return a thread, NT does */ te.dwSize = sizeof(te); - ok(!pThread32First( hSnapshot, &te ), "shouldn't return a thread\n"); + ret = pThread32First( hSnapshot, &te ); + ok(ret || !ret, "You'll never see this\n");
renders the whole test useless. It turns out that win9x in this case leaves te.dwSize and XP-and-up set te.dwSize to 0. Shouldn't we make use of that fact? So something like (or another combination):
if (te.dwSize == 0) { XP-and-up, expect failure } else if (te.dwSize == sizeof(te)) { win9x, expect success } else { something is wrong/unexpected }
On a sidenote (and this has been the case for a long time):
I've seen patches where
ok(function(a,b),"Failed");
is turned into
c = function(a,b); if (c) { ..... }
but done without a skip or such. So if Wine has a regression for 'function' we won't notice.
We have loads of patches where we expect 3 or more different last errors. This will make sure the tests always succeed of course but makes it difficult to tell if we (Wine) are doing the correct/best thing.
On Tue, Apr 22, 2008 at 1:33 AM, Paul Vriens paul.vriens.wine@gmail.com wrote:
James Hawkins wrote:
Hi,
Changelog:
- Fix a test that fails on all NT platforms.
dlls/kernel32/tests/toolhelp.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
Hi James,
I've been looking at some of the patches to fix the tests but I'm a bit unsure if some of them are the correct way to go. Things like:
- /* win9x does not return a thread, NT does */ te.dwSize = sizeof(te);
- ok(!pThread32First( hSnapshot, &te ), "shouldn't return a thread\n");
- ret = pThread32First( hSnapshot, &te );
- ok(ret || !ret, "You'll never see this\n");
renders the whole test useless.
That's the point; the test is useless. If a test fails on one platform but succeeds in another, then we're testing a behavior that can't be relied on, or the test is not specific enough. We leave it around for documentation of that fact.
It turns out that win9x in this case leaves te.dwSize and XP-and-up set te.dwSize to 0. Shouldn't we make use of that fact?
You could, but it doesn't change the fact that the behavior is not reliable across different platforms. If we knew an app that makes this exact check and depends on the results, then sure we should add a pickier test. As it stands, it's just a bit too nit-picky.
On a sidenote (and this has been the case for a long time):
I've seen patches where
ok(function(a,b),"Failed");
is turned into
c = function(a,b); if (c) { ..... }
but done without a skip or such. So if Wine has a regression for 'function' we won't notice.
Tell that to the platforms that fail too. Thus, this is a behavior that can't be relied on. Wine strives to have compatibility with all the platforms we test on. We don't stick to one specific version, and if we did, we could arbitrarily pick the version that fails this function, then failing would be the right behavior.
We have loads of patches where we expect 3 or more different last errors. This will make sure the tests always succeed of course but makes it difficult to tell if we (Wine) are doing the correct/best thing.
You're missing the broader picture. If a function returns three or more different last errors, how can any app depend on that either? What is the definition of the "correct/best thing"?
James Hawkins wrote:
On Tue, Apr 22, 2008 at 1:33 AM, Paul Vriens paul.vriens.wine@gmail.com wrote:
James Hawkins wrote:
Hi,
Changelog:
- Fix a test that fails on all NT platforms.
dlls/kernel32/tests/toolhelp.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
Hi James,
I've been looking at some of the patches to fix the tests but I'm a bit unsure if some of them are the correct way to go. Things like:
- /* win9x does not return a thread, NT does */ te.dwSize = sizeof(te);
- ok(!pThread32First( hSnapshot, &te ), "shouldn't return a thread\n");
- ret = pThread32First( hSnapshot, &te );
- ok(ret || !ret, "You'll never see this\n");
renders the whole test useless.
That's the point; the test is useless. If a test fails on one platform but succeeds in another, then we're testing a behavior that can't be relied on, or the test is not specific enough. We leave it around for documentation of that fact.
It turns out that win9x in this case leaves te.dwSize and XP-and-up set te.dwSize to 0. Shouldn't we make use of that fact?
You could, but it doesn't change the fact that the behavior is not reliable across different platforms. If we knew an app that makes this exact check and depends on the results, then sure we should add a pickier test. As it stands, it's just a bit too nit-picky.
But if it's just for documentation purposes (as for now we don't know an app that depends on it) isn't my example a bit better? (I agree about the nit-picky though).
On a sidenote (and this has been the case for a long time):
I've seen patches where
ok(function(a,b),"Failed");
is turned into
c = function(a,b); if (c) { ..... }
but done without a skip or such. So if Wine has a regression for 'function' we won't notice.
Tell that to the platforms that fail too. Thus, this is a behavior that can't be relied on. Wine strives to have compatibility with all the platforms we test on. We don't stick to one specific version, and if we did, we could arbitrarily pick the version that fails this function, then failing would be the right behavior.
If a function consistently fails that's true but there are all kinds of circumstances that a function could fail. I agree we shouldn't care about the specific version but just ignoring failure because one of the versions fails is not correct in my opinion. We already had this 'discussion' with failures that only happen on win9x and the fact whether we should cater for that are just let it be. 'Let it be' was AJ's opinion back than, unless we have a nice way of catching it.
We have loads of patches where we expect 3 or more different last errors. This will make sure the tests always succeed of course but makes it difficult to tell if we (Wine) are doing the correct/best thing.
You're missing the broader picture. If a function returns three or more different last errors, how can any app depend on that either? What is the definition of the "correct/best thing"?
Agreed, if versions pass different last errors an app could hardly be depending on it.
I agree on most of your points as we merely showing that versions return different errors or even succeed or fail depending on the OS. But I also think that the more we know about the circumstances the more we should document them.
Having a 1.0 soon will most likely broaden the numbers of users and hence apps that people are going to (try to) run with Wine. The chances will become bigger that there is an app out there that will make use of version differences by whatever means. I've seen several apps that do a whole lot of things just to to find out on what version/OS they are running on (and I'm not talking about copy protection apps).
On Tue, Apr 22, 2008 at 2:48 AM, Paul Vriens paul.vriens.wine@gmail.com wrote:
James Hawkins wrote:
On Tue, Apr 22, 2008 at 1:33 AM, Paul Vriens paul.vriens.wine@gmail.com
wrote:
James Hawkins wrote:
Hi,
Changelog:
- Fix a test that fails on all NT platforms.
dlls/kernel32/tests/toolhelp.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
Hi James,
I've been looking at some of the patches to fix the tests but I'm a bit unsure if some of them are the correct way to go. Things like:
- /* win9x does not return a thread, NT does */ te.dwSize = sizeof(te);
- ok(!pThread32First( hSnapshot, &te ), "shouldn't return a
thread\n");
- ret = pThread32First( hSnapshot, &te );
- ok(ret || !ret, "You'll never see this\n");
renders the whole test useless.
That's the point; the test is useless. If a test fails on one platform but succeeds in another, then we're testing a behavior that can't be relied on, or the test is not specific enough. We leave it around for documentation of that fact.
It turns out that win9x in this case leaves te.dwSize and XP-and-up set te.dwSize to 0. Shouldn't we make use of
that
fact?
You could, but it doesn't change the fact that the behavior is not reliable across different platforms. If we knew an app that makes this exact check and depends on the results, then sure we should add a pickier test. As it stands, it's just a bit too nit-picky.
But if it's just for documentation purposes (as for now we don't know an app that depends on it) isn't my example a bit better? (I agree about the nit-picky though).
On a sidenote (and this has been the case for a long time):
I've seen patches where
ok(function(a,b),"Failed");
is turned into
c = function(a,b); if (c) { ..... }
but done without a skip or such. So if Wine has a regression for
'function'
we won't notice.
Tell that to the platforms that fail too. Thus, this is a behavior that can't be relied on. Wine strives to have compatibility with all the platforms we test on. We don't stick to one specific version, and if we did, we could arbitrarily pick the version that fails this function, then failing would be the right behavior.
If a function consistently fails that's true but there are all kinds of circumstances that a function could fail. I agree we shouldn't care about the specific version but just ignoring failure because one of the versions fails is not correct in my opinion. We already had this 'discussion' with failures that only happen on win9x and the fact whether we should cater for that are just let it be. 'Let it be' was AJ's opinion back than, unless we have a nice way of catching it.
We have loads of patches where we expect 3 or more different last
errors.
This will make sure the tests always succeed of course but makes it difficult to tell if we (Wine) are doing the correct/best thing.
You're missing the broader picture. If a function returns three or more different last errors, how can any app depend on that either? What is the definition of the "correct/best thing"?
Agreed, if versions pass different last errors an app could hardly be depending on it.
I agree on most of your points as we merely showing that versions return different errors or even succeed or fail depending on the OS. But I also think that the more we know about the circumstances the more we should document them.
Having a 1.0 soon will most likely broaden the numbers of users and hence apps that people are going to (try to) run with Wine. The chances will become bigger that there is an app out there that will make use of version differences by whatever means. I've seen several apps that do a whole lot of things just to to find out on what version/OS they are running on (and I'm not talking about copy protection apps).
Sure, but like I said before, we'll deal with that when we find those apps. In the meantime, I'm going to keep sending in these patches fixing up the tests. Hopefully people will take notice and help me out :-)
-
James Hawkins -
Paul Vriens