Fixing crashes in Tests (OS version check)
Hi, I've being investigating crashes that occur on Win98 when the tests run. I have fixed a couple already which where trivial. When I attempted to fix the snmpapi tests, I have found that i will end up removing alot of tests (~ 17). If I add a check for the windows version, I can skip the tests that will crash, also there is already tests with if(0) around them. What is the policy about checking the version of the OS before running certain tests? Best Regards Alistair Leslie-Hughes
On Fri, May 30, 2008 at 11:18 PM, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote:
Hi,
I've being investigating crashes that occur on Win98 when the tests run. I have fixed a couple already which where trivial. When I attempted to fix the snmpapi tests, I have found that i will end up removing alot of tests (~ 17). If I add a check for the windows version, I can skip the tests that will crash, also there is already tests with if(0) around them. What is the policy about checking the version of the OS before running certain tests?
Version checks are not allowed. -- James Hawkins
James Hawkins wrote:
On Fri, May 30, 2008 at 11:18 PM, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote:
Hi,
I've being investigating crashes that occur on Win98 when the tests run. I have fixed a couple already which where trivial. When I attempted to fix the snmpapi tests, I have found that i will end up removing alot of tests (~ 17). If I add a check for the windows version, I can skip the tests that will crash, also there is already tests with if(0) around them. What is the policy about checking the version of the OS before running certain tests? Version checks are not allowed. Is this actually a sensible policy? As Windows version age (Win95 arrived 12 years ago in 96!) there are going to be more and more incompatibilities with later versions. Windows is not backward compatible. Apps built for Vista are not guaranteed to run on earlier versions and the Windows api moves along. The upshot is that Wine tests must be dumbed down (ie. we accept multiple results (Win95 and above) or we delete the test). The outcome is that we don't actually know what we are testing as Wine could be producing results for XP, Vista, W2K but it tests ok.
2008/5/31 Jeff Latimer <lats(a)yless4u.com.au>:
James Hawkins wrote:
On Fri, May 30, 2008 at 11:18 PM, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote:
Hi,
I've being investigating crashes that occur on Win98 when the tests run. I have fixed a couple already which where trivial. When I attempted to fix the snmpapi tests, I have found that i will end up removing alot of tests (~ 17). If I add a check for the windows version, I can skip the tests that will crash, also there is already tests with if(0) around them. What is the policy about checking the version of the OS before running certain tests? Version checks are not allowed. Is this actually a sensible policy? As Windows version age (Win95 arrived 12 years ago in 96!) there are going to be more and more incompatibilities with later versions. Windows is not backward compatible. Apps built for Vista are not guaranteed to run on earlier versions and the Windows api moves along. The upshot is that Wine tests must be dumbed down (ie. we accept multiple results (Win95 and above) or we delete the test). The outcome is that we don't actually know what we are testing as Wine could be producing results for XP, Vista, W2K but it tests ok.
The reason applications built for Vista are not guaranteed to work on earlier versions is that they depend on new API available in Vista. For example, they may use the new 'glass' API directly, rely on the new menu button support in the button control, or some new behaviour in an existing API. The only case that may cause issues here in the tests is the last one, as Wine may be validly conforming to XP or earlier behaviour. Note though that most of the differences are in what error codes are returned. Applications in Windows should not depend on specific error codes being returned (as they may vary from version to version as the Wine tests verify), just whether the call succeeded or failed, unless those error codes are specifically called out in the documentation - QueryInterface returning E_NOINTERFACE if an interface is not supported by the COM object, for instance. Also with the error codes, they may change with each hot fix, service pack and application (e.g. Internet Explorer) update. For example, the shlwapi istream tests have one case that returns E_NOTIMPL on Windows 2000 except when IE6 or later is installed (i.e. the shlwapi version has been updated to match the one on XP or later). Older versions of Windows don't expose certain API. In this case (as is such in this instance), the tests should not crash but should be skipped with a message saying that the function(s) are not available. This scales better, since the API may not be available on a range of Windows versions, or the API may be added when installing IE6 or IE7. New window and user/common control messages can be handled in a similar way, although I don't know the exact mechanics of how this would work. The interesting case is were you have specific behavioural differences (e.g. crash when passed a NULL argument vs returning an error code such as E_POINTER). In these cases, Wine is free to choose what it does as Windows is not consistent with itself (see the application compatibility issues with Vista). Here, if a specific application is dependant on certain behaviour, Wine should support that and have that documented in the tests. Writing tests that work on all Windows platforms is hard. That does not mean that the tests should use OS checks and I think the existing methodology is the right approach to writing the tests. There will be cases where things can't be tested due to incompatible differences, and that would be an area that could be investigated to see if those can be improved (such as the new buggy functionality and possibly others in that family) to allow support for older documented behaviour in the tests, while keeping the Wine implementation sane. - Reece
Jeff Latimer wrote:
James Hawkins wrote:
On Fri, May 30, 2008 at 11:18 PM, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote:
Hi,
I've being investigating crashes that occur on Win98 when the tests run. I have fixed a couple already which where trivial. When I attempted to fix the snmpapi tests, I have found that i will end up removing alot of tests (~ 17). If I add a check for the windows version, I can skip the tests that will crash, also there is already tests with if(0) around them. What is the policy about checking the version of the OS before running certain tests? Version checks are not allowed. Is this actually a sensible policy? As Windows version age (Win95 arrived 12 years ago in 96!) there are going to be more and more incompatibilities with later versions. Windows is not backward compatible. Apps built for Vista are not guaranteed to run on earlier versions and the Windows api moves along. The upshot is that Wine tests must be dumbed down (ie. we accept multiple results (Win95 and above) or we delete the test). The outcome is that we don't actually know what we are testing as Wine could be producing results for XP, Vista, W2K but it tests ok.
Hi Jeff, I agree with you. We don't really know if a test gave the correct value, just it whether passed or failed, but I can live with that. I want to see a excellent coverage of tests for each feature that wine implements (no matter what win version). But having to dump down the tests to support the lowest common windows isn't going to achieve this. Maybe the policy could be changed to allow version checking if it they is more than one OS that crashes due to single particular test. Best Regards Alistair Leslie-Hughes
Alistair Leslie-Hughes wrote:
Hi,
I've being investigating crashes that occur on Win98 when the tests run. I have fixed a couple already which where trivial. When I attempted to fix the snmpapi tests, I have found that i will end up removing alot of tests (~ 17). If I add a check for the windows version, I can skip the tests that will crash, also there is already tests with if(0) around them. What is the policy about checking the version of the OS before running certain tests?
Best Regards Alistair Leslie-Hughes
In this particular case you could make use of this fact: util.c:42:GetProcAddress(SnmpUtilAsnAnyCpy) failed util.c:43:GetProcAddress(SnmpUtilAsnAnyFree) failed util.c:44:GetProcAddress(SnmpUtilOctetsCmp) failed util.c:45:GetProcAddress(SnmpUtilOctetsCpy) failed util.c:46:GetProcAddress(SnmpUtilOctetsFree) failed util.c:47:GetProcAddress(SnmpUtilOctetsNCmp) failed This happens to be true for win95, win95, winme and NT4. So if those functions are not available set a variable. Then use that variable to skip some tests that crash. As these tests crash on several versions of Windows I think it's worthwhile doing this. -- Cheers, Paul.
Paul Vriens wrote:
In this particular case you could make use of this fact:
util.c:42:GetProcAddress(SnmpUtilAsnAnyCpy) failed util.c:43:GetProcAddress(SnmpUtilAsnAnyFree) failed util.c:44:GetProcAddress(SnmpUtilOctetsCmp) failed util.c:45:GetProcAddress(SnmpUtilOctetsCpy) failed util.c:46:GetProcAddress(SnmpUtilOctetsFree) failed util.c:47:GetProcAddress(SnmpUtilOctetsNCmp) failed
This happens to be true for win95, win95, winme and NT4.
So if those functions are not available set a variable. Then use that variable to skip some tests that crash.
Hi Paul, These functions aren't the issue, since they are worked out at runtime. An example of one that fails is ret = SnmpUtilOidToA(NULL); Under win98, this function is valid to call, but will crash if you call it will a NULL parameter. I don't think removing this test a good idea, since we are setting XP as a base, at this passes. Best Regards Alistair Leslie-Hughes
Alistair Leslie-Hughes wrote:
Paul Vriens wrote:
In this particular case you could make use of this fact:
util.c:42:GetProcAddress(SnmpUtilAsnAnyCpy) failed util.c:43:GetProcAddress(SnmpUtilAsnAnyFree) failed util.c:44:GetProcAddress(SnmpUtilOctetsCmp) failed util.c:45:GetProcAddress(SnmpUtilOctetsCpy) failed util.c:46:GetProcAddress(SnmpUtilOctetsFree) failed util.c:47:GetProcAddress(SnmpUtilOctetsNCmp) failed
This happens to be true for win95, win95, winme and NT4.
So if those functions are not available set a variable. Then use that variable to skip some tests that crash.
Hi Paul,
These functions aren't the issue, since they are worked out at runtime.
An example of one that fails is ret = SnmpUtilOidToA(NULL); Under win98, this function is valid to call, but will crash if you call it will a NULL parameter. I don't think removing this test a good idea, since we are setting XP as a base, at this passes.
Best Regards Alistair Leslie-Hughes
What I meant is that these non available functions are an indication for NT4 and below. This means you can detect by logic whether you are on NT4 (and below) or above. This logic can be used to skip some tests. -- Cheers, Paul.
An example of one that fails is ret = SnmpUtilOidToA(NULL); Under win98, this function is valid to call, but will crash if you call it will a NULL parameter. I don't think removing this test a good idea, since we are setting XP as a base, at this passes.
I disagree that the test shouldn't be removed. The tests are meant to show Windows behavior that apps are likely to expect. Since the apps will crash if they pass NULL to SnmpUtilOidToA, it's reasonable to expect that they will not. Unless there's an app that depends on being able to do so, removing the test seems more sensible as it means we'll get better test coverage on Win98. --Juan
On Sat, 31 May 2008, Juan Lang wrote: [...]
An example of one that fails is ret = SnmpUtilOidToA(NULL); Under win98, this function is valid to call, but will crash if you call it will a NULL parameter. I don't think removing this test a good idea, since we are setting XP as a base, at this passes.
I disagree that the test shouldn't be removed. The tests are meant to show Windows behavior that apps are likely to expect.
The thing is that this evolves with time. Back in 2000 nobody would have called SnmpUtilOidToA with a NULL parameter as it would have meant crashing on a lot of the Windows install base. Nowadays every ISV expects you to have at least Windows XP and wouldn't care one bit if that call means crashing the application on NT4 or lower. So you could say that nowadays applications are likely to expect SnmpUtilOidToA(NULL) not to crash which means keeping these tests. However I don't think our conformance tests should crash on older systems and that's what makes things tricky... -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ Broadcast message : fin du monde dans cinq minutes, repentez vous !
The thing is that this evolves with time. Back in 2000 nobody would have called SnmpUtilOidToA with a NULL parameter as it would have meant crashing on a lot of the Windows install base. Nowadays every ISV expects you to have at least Windows XP and wouldn't care one bit if that call means crashing the application on NT4 or lower.
So you could say that nowadays applications are likely to expect SnmpUtilOidToA(NULL) not to crash which means keeping these tests. However I don't think our conformance tests should crash on older systems and that's what makes things tricky...
I agree with you that resolving issues like this can be subtle, since there will always be specific examples when a general principle does not apply. They don't invalidate the general principle unless they outnumber it or outweigh it in some way. So, in my opinion, the general principle is this: if the behavior is very different on different Windows versions, and if the behavior on some Windows version is to crash, it seems reasonable to remove the test, as: 1) we want our tests to reflect Windows behavior that applications count on, and different behavior (especially crashing) often implies the applications don't count on a particular behavior, and 2) having regression tests crash is undesirable, as many other valid tests will not be run on a platform. This doesn't mean that we can't make Wine not crash given the bogus input, it just means we may not be able to have a regression test for this behavior. And, of course, there may be specific instances in which we wish to test potentially crashy behavior, but I'd argue that they better be really important to override points 1) and 2). And coming back to this test: I still think it's easier just to remove the test than to come up with a hacky way of avoiding it on a crashy system. --Juan
Juan Lang wrote:
So, in my opinion, the general principle is this: if the behavior is very different on different Windows versions, and if the behavior on some Windows version is to crash, it seems reasonable to remove the test, as: 1) we want our tests to reflect Windows behavior that applications count on, and different behavior (especially crashing) often implies the applications don't count on a particular behavior, and 2) having regression tests crash is undesirable, as many other valid tests will not be run on a platform.
I don't think you can fully argument with this nowadays. There are many applications out there that specifically state to not support pre W2K or sometimes even XP and often refuse to even install on older systems. So in this case they could be perfectly happy with passing a NULL value to that function and since they never have been tested on an older system the developer wouldn't even know that there exists a potential problem. So I would think it's useful to test that the Wine API does not crash on a NULL parameter since newer Windows versions won't do that either.
This doesn't mean that we can't make Wine not crash given the bogus input, it just means we may not be able to have a regression test for this behavior. And, of course, there may be specific instances in which we wish to test potentially crashy behavior, but I'd argue that they better be really important to override points 1) and 2).
This case seems fairly simple to me. As long as Wine strives to provide support for Win9x and NT4 it needs to make efforts to support both the newer and the older behaviour. For the API itself this means allowing NULL values to be passed without crashing and for the tests it means skipping that particular test for those versions that do crash on it. And while a version test seems the easiest I can also understand why the tests should not have such artificial dependencies especially since these tests would have to distinguish between running on Windows or Wine set to emulate Win9x. But here it seems fairly easy to work around this problem. The fact that certain Snmp APIs are not implemented or possibly return an ERROR_UNIMPLEMENTED status on those platforms should be enough to determine if this test should be skipped.
And coming back to this test: I still think it's easier just to remove the test than to come up with a hacky way of avoiding it on a crashy system.
No argument with the being easier part. But not writing tests at all would be also easier, but I do not think that is a valid argument to not do them. Typically to do a good test costs more time than an actual implementation of a function and Wine development could be sped up by not writing tests. But it would only implement more functionality with a lot more errors (or should that be less bugs :-). Rolf Kalbermatter
participants (8)
-
Alistair Leslie-Hughes -
Francois Gouget -
James Hawkins -
Jeff Latimer -
Juan Lang -
Paul Vriens -
Reece Dunn -
Rolf Kalbermatter