Re: [PATCH 2/5] oleaut32/tests: Make tests working on older systems, prevent crash on some wine boxes
On 03/17/2010 10:05 PM, Piotr Caban wrote:
hres = ICreateTypeInfo_AddRefTypeInfo(createti, unknown,&hreftype); ok(hres == S_OK, "got %08x\n", hres); + if(hres != S_OK) { + skip("Skipping some tests\n"); + return; + }
Hi Piotr, If it's acceptable to get something else besides S_OK you should change the ok() and add a broken() (still bail out of course). A skip() usually means that something valid happened and we need to bail out. -- Cheers, Paul.
Hi,
If it's acceptable to get something else besides S_OK you should change the ok() and add a broken() (still bail out of course). It's not acceptable to get anything else than S_OK here. I thought skip should be used to inform that not all tests are being executed.
A skip() usually means that something valid happened and we need to bail out. Should I just return from the function without invoking skip?
Regards, Piotr
On 03/18/2010 05:03 PM, Piotr Caban wrote:
Hi,
If it's acceptable to get something else besides S_OK you should change the ok() and add a broken() (still bail out of course). It's not acceptable to get anything else than S_OK here. I thought skip should be used to inform that not all tests are being executed.
If it's not acceptable you should do something like: ok(hres == S_OK || broken(hres != S_OK), /* Platform where this happens */ "got %08x\n", hres); if(hres != S_OK) { skip("Skipping some tests\n"); return; }
A skip() usually means that something valid happened and we need to bail out. Should I just return from the function without invoking skip?
A broken() would not be reported as such, a skip would at least give us some information. If the skip() is only going to happen on Windows you should use win_skip(), although in this particular case the broken() already makes sure Wine won't regress. The end result should 'always' be that we have zero failures on all platforms. -- Cheers, Paul.
This test is only failing on some wine boxes, that doesn't have oleaut2.tlb registered (or QueryPathOfRegTypeLib is broken). It probably happens only when 64bit tests are being run. Should I check if we're running in 64bit tests and add todo_wine in this case?
On 03/18/2010 05:23 PM, Piotr Caban wrote:
This test is only failing on some wine boxes, that doesn't have oleaut2.tlb registered (or QueryPathOfRegTypeLib is broken). It probably happens only when 64bit tests are being run.
Ok, that makes the approach a bit different.
Should I check if we're running in 64bit tests and add todo_wine in this case?
Are you a 100% sure this is the case on 64bit Wine only? If so a todo_wine should be appropriate (in the 64bit case). The bail out is still needed (with the skip()) of course. -- Cheers, Paul.
On 03/18/2010 05:26 PM, Paul Vriens wrote:
On 03/18/2010 05:23 PM, Piotr Caban wrote:
This test is only failing on some wine boxes, that doesn't have oleaut2.tlb registered (or QueryPathOfRegTypeLib is broken). It probably happens only when 64bit tests are being run.
Ok, that makes the approach a bit different.
Should I check if we're running in 64bit tests and add todo_wine in this case?
Are you a 100% sure this is the case on 64bit Wine only? If so a todo_wine should be appropriate (in the 64bit case). The bail out is still needed (with the skip()) of course.
Patch is committed. So all you need to do now if fix the 64bit implementation (if that's indeed the case) ;) -- Cheers, Paul.
On 03/18/10 17:42, Paul Vriens wrote:
Patch is committed. So all you need to do now if fix the 64bit implementation (if that's indeed the case) ;)
As far as I know both oleaut typelib creation and widl are not working well on 64bit systems (is widl ever compiled as 64bit application?). It will probably need lots of work to get it working correctly.
Piotr Caban <piotr(a)codeweavers.com> writes:
On 03/18/10 17:42, Paul Vriens wrote:
Patch is committed. So all you need to do now if fix the 64bit implementation (if that's indeed the case) ;)
As far as I know both oleaut typelib creation and widl are not working well on 64bit systems (is widl ever compiled as 64bit application?). It will probably need lots of work to get it working correctly.
widl is definitely compiled as 64-bit, and works fairly well. The typelib side does need some love though. -- Alexandre Julliard julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
Paul Vriens -
Piotr Caban