Re: ieframe/tests: Skip rest of tests if CoCreateInstance failed.
Hi Octavian, On 08/25/11 13:26, Octavian Voicu wrote:
Prevents a crash on WOW64, where CLSID_InternetExplorer\LocalServer32 points to nonexistent iexplore.exe in Program Files (x86) folder.
Where do you get such a broken installation from? I don't see such failures on test.winehq.org. Jacek
On Thu, Aug 25, 2011 at 2:38 PM, Jacek Caban <jacek(a)codeweavers.com> wrote:
Where do you get such a broken installation from? I don't see such failures on test.winehq.org.
Hi Jacek, It's not only me, see [1]. It fails every time on WOW64 giving something like this [2]. It's a different crash from the win64 one (which is caused by an assertion failure in oleaut32/tmarshal.c). [not related to this patch] It's strange that iexplore.exe from Program Files (x86) folder is used also for GIF, JPEG, PNG open handlers, and for HTML icons, but on the main Classes tree, so they are broken. Should 32-bit iexplore.exe be copied in Program Files (x86)? A 32-bit process should be able to communicate with 64-bit iexplore.exe via RPC, right? Octavian PS: crash reported by testbot was already there, just that ieframe tests are usually skipped on NT4. [1] http://test.winehq.org/data/6ee95c77381791299ae040248897ac58c74f49f6/index_L... [2] http://test.winehq.org/data/6ee95c77381791299ae040248897ac58c74f49f6/linux_o...
On 08/25/11 13:55, Octavian Voicu wrote:
On Thu, Aug 25, 2011 at 2:38 PM, Jacek Caban<jacek(a)codeweavers.com> wrote:
Where do you get such a broken installation from? I don't see such failures on test.winehq.org. Hi Jacek,
It's not only me, see [1]. It fails every time on WOW64 giving something like this [2]. It's a different crash from the win64 one (which is caused by an assertion failure in oleaut32/tmarshal.c).
Those are failures on Wine, so tests are fine, it's Wine that needs fixing.
[not related to this patch] It's strange that iexplore.exe from Program Files (x86) folder is used also for GIF, JPEG, PNG open handlers, and for HTML icons, but on the main Classes tree, so they are broken.
Should 32-bit iexplore.exe be copied in Program Files (x86)? A 32-bit process should be able to communicate with 64-bit iexplore.exe via RPC, right?
We should do whatever Windows does.
PS: crash reported by testbot was already there, just that ieframe tests are usually skipped on NT4.
Yeah, testbot is not smart enough to skip tests if tested DLL is not present (unlike winetest). Jacek
On Thu, Aug 25, 2011 at 2:59 PM, Jacek Caban <jacek(a)codeweavers.com> wrote:
Those are failures on Wine, so tests are fine, it's Wine that needs fixing.
Yes, the first test usually fails on Wine. Nonetheless, if CoCreateInstance fails for whatever reason (like it happens on that NT4 machine, for example), running the rest of the tests is pointless and will definitely crash. It's annoying to get crashes while running tests. The skip is not really needed I guess, but the return should be there to stop the crashing. Octavian
On 08/25/11 14:09, Octavian Voicu wrote:
On Thu, Aug 25, 2011 at 2:59 PM, Jacek Caban<jacek(a)codeweavers.com> wrote:
Those are failures on Wine, so tests are fine, it's Wine that needs fixing. Yes, the first test usually fails on Wine. Nonetheless, if CoCreateInstance fails for whatever reason (like it happens on that NT4 machine, for example), running the rest of the tests is pointless and will definitely crash.
It's annoying to get crashes while running tests. The skip is not really needed I guess, but the return should be there to stop the crashing.
As long as there is a failure (like in your patch), I'm fine with return. Just let's not pretend it's somehow valid run with skip() call. Also testing hres is cleaner than unk, IMO. Jacek
On Thu, Aug 25, 2011 at 3:31 PM, Jacek Caban <jacek(a)codeweavers.com> wrote:
As long as there is a failure (like in your patch), I'm fine with return. Just let's not pretend it's somehow valid run with skip() call. Also testing hres is cleaner than unk, IMO.
Yeah, the skip will only come after a failure because unk is set to NULL if CoCreateInstance fails. Gonna send a try 2 with testing hres; this way it will be clearer that it cannot skip unless a failure was recorded. Octavian
On 08/25/11 14:40, Octavian Voicu wrote:
On Thu, Aug 25, 2011 at 3:31 PM, Jacek Caban<jacek(a)codeweavers.com> wrote:
As long as there is a failure (like in your patch), I'm fine with return. Just let's not pretend it's somehow valid run with skip() call. Also testing hres is cleaner than unk, IMO. Yeah, the skip will only come after a failure because unk is set to NULL if CoCreateInstance fails. Gonna send a try 2 with testing hres; this way it will be clearer that it cannot skip unless a failure was recorded.
What I meant is that skip() should not be there, test failure is right and enough in this case. Jacek
participants (2)
-
Jacek Caban -
Octavian Voicu