Re: shell32: Fix tests to apply to Vista+
On 4/23/2010 16:56, André Hentschel wrote:
--- dlls/shell32/tests/shlfolder.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c index f63bf6e..634c3d3 100644 --- a/dlls/shell32/tests/shlfolder.c +++ b/dlls/shell32/tests/shlfolder.c @@ -2048,7 +2048,10 @@ if (0) ok(hr == S_OK, "failed %08x\n", hr);
ret = pILIsEqual(pidl1, pidl2); - ok(ret == TRUE, "expected equal idls\n"); + todo_wine + ok(broken(ret) || + !ret, /* Vista+ */ + "expected different idls\n"); pILFree(pidl1); pILFree(pidl2);
There's no point to silence this. We need to figure out what's this pidl about on Vista+, e.g. dump it data and then probably review Wine handling of this.
Nikolay Sivov schrieb:
On 4/23/2010 16:56, André Hentschel wrote:
--- dlls/shell32/tests/shlfolder.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c index f63bf6e..634c3d3 100644 --- a/dlls/shell32/tests/shlfolder.c +++ b/dlls/shell32/tests/shlfolder.c @@ -2048,7 +2048,10 @@ if (0) ok(hr == S_OK, "failed %08x\n", hr);
ret = pILIsEqual(pidl1, pidl2); - ok(ret == TRUE, "expected equal idls\n"); + todo_wine + ok(broken(ret) || + !ret, /* Vista+ */ + "expected different idls\n"); pILFree(pidl1); pILFree(pidl2);
There's no point to silence this. We need to figure out what's this pidl about on Vista+, e.g. dump it data and then probably review Wine handling of this.
My point of view is that its not an error on windows, with this patch it passes in windows and gets a todo_wine, which it actually is (like you say). -- Best Regards, André Hentschel
My point of view is that its not an error on windows, with this patch it passes in windows and gets a todo_wine, which it actually is (like you say).
Any test that allows all possibilities, as your patch does on Windows, is worthless. That implies that it should be investigated, and, if it's truly a meaningless test, then remove it. --Juan
Juan Lang schrieb:
My point of view is that its not an error on windows, with this patch it passes in windows and gets a todo_wine, which it actually is (like you say).
Any test that allows all possibilities, as your patch does on Windows, is worthless. That implies that it should be investigated, and, if it's truly a meaningless test, then remove it. --Juan
It still has two benefits applying the patch: it states its a todo on wine and it documents the old behavior of windows as broken. I also see your point, just to be clear. Its just another possibility. -- Best Regards, André Hentschel
On 4/23/2010 20:30, André Hentschel wrote:
Juan Lang schrieb:
My point of view is that its not an error on windows, with this patch it passes in windows and gets a todo_wine, which it actually is (like you say).
Any test that allows all possibilities, as your patch does on Windows, is worthless. That implies that it should be investigated, and, if it's truly a meaningless test, then remove it. --Juan
It still has two benefits applying the patch: it states its a todo on wine and it documents the old behavior of windows as broken. I also see your point, just to be clear. Its just another possibility.
It's not the point of a test actually. If it different pidl is returned on Vista+ while parsing from desktop we need to figure out what pidl is returned and still test for equality. Having it not equal makes the rest of a test function useless.
On 04/23/2010 06:30 PM, André Hentschel wrote:
Juan Lang schrieb:
My point of view is that its not an error on windows, with this patch it passes in windows and gets a todo_wine, which it actually is (like you say).
Any test that allows all possibilities, as your patch does on Windows, is worthless. That implies that it should be investigated, and, if it's truly a meaningless test, then remove it. --Juan
It still has two benefits applying the patch: it states its a todo on wine and it documents the old behavior of windows as broken.
But who guarantees that the old behavior is broken and not the new one? I'm with Juan and Nikolay on this one, we need more investigation. -- Cheers, Paul.
On 4/23/2010 20:46, Paul Vriens wrote:
On 04/23/2010 06:30 PM, André Hentschel wrote:
Juan Lang schrieb:
My point of view is that its not an error on windows, with this patch it passes in windows and gets a todo_wine, which it actually is (like you say).
Any test that allows all possibilities, as your patch does on Windows, is worthless. That implies that it should be investigated, and, if it's truly a meaningless test, then remove it. --Juan
It still has two benefits applying the patch: it states its a todo on wine and it documents the old behavior of windows as broken.
But who guarantees that the old behavior is broken and not the new one?
I'm with Juan and Nikolay on this one, we need more investigation.
An update here for anyone who cares. I dumped returned pidls (results are here https://testbot.winehq.org/JobDetails.pl?Key=1725&log_101=1#k101). Vista failure caused by SHParseDisplayName returning pidl with a root at UsersFiles (alias for user home directory in Users). Parsing from desktop returns pidl with root at My Computer. Looks like SHParseDisplayName now uses this virtual folder (user home alias) as a root, that's all, and there's no way to switch to pre-vista behavior.
participants (5)
-
André Hentschel -
Juan Lang -
Nikolay Sivov -
Nikolay Sivov -
Paul Vriens