Re: [1/2] include/wine: add option to specify the unittest target via environment variable

[Paul Vriens]

On 04/19/2010 11:44 PM, Stefan Leichter wrote:
> include/wine/test.h |    6 +++++-
>   1 files changed, 5 insertions(+), 1 deletions(-)

Hi Stefan,

What's the idea/thought behind this (or what problem are you trying to 
solve)?

[1/2] include/wine: add option to specify the unittest target via environment variable

[Stefan Leichter]

Am Tuesday 20 April 2010 07:59:22 schrieb Paul Vriens:
> On 04/19/2010 11:44 PM, Stefan Leichter wrote:
> > include/wine/test.h |    6 +++++-
> >   1 files changed, 5 insertions(+), 1 deletions(-)
>
> Hi Stefan,
>
> What's the idea/thought behind this (or what problem are you trying to
> solve)?

Hi Paul,

the problem is the "command line parsing" of ShellExecuteEx, see bug19666 
(http://bugs.winehq.org/show_bug.cgi?id=19666#c3).

When a directory contains two executables where the name of the first, without 
extension, is the leading part of the name of the second and the second file 
have a blank at the position where the first file has the dot, wine 
implementation of ShellExecuteEx starts the first executable when the name of 
the second given to ShellExecuteEx.

If a unittest call the second executable via ShellExecuteEx, the first 
executable is started and the test target is the trailing part of the 
filename including the extension. This causes a test failure because the test 
target does not exist.

Example:
test file.exe 
test file two.exe

Builtin ShellExecuteEx starts always "test file.exe". If "test file two.exe" 
was passed as name to be ShellExecuteEx argv[1] will be "two.exe".

To get around this problem and make a valid unittest without fixing the 
problem, i looked for another way to pass the test target to the executable.

Finaly to perform the same flow as the program in bug 19666 does, 
ShellExecuteEx needs to be called without any parameter.

[1/2] include/wine: add option to specify the unittest target via environment variable

[Paul Vriens]

On 04/20/2010 08:37 PM, Stefan Leichter wrote:
> Am Tuesday 20 April 2010 07:59:22 schrieb Paul Vriens:
> > On 04/19/2010 11:44 PM, Stefan Leichter wrote:
> > > include/wine/test.h |    6 +++++-
> > >    1 files changed, 5 insertions(+), 1 deletions(-)
> >
> > Hi Stefan,
> >
> > What's the idea/thought behind this (or what problem are you trying to
> > solve)?
>
> Hi Paul,
>
> the problem is the "command line parsing" of ShellExecuteEx, see bug19666
> (http://bugs.winehq.org/show_bug.cgi?id=19666#c3).
>
> When a directory contains two executables where the name of the first, without
> extension, is the leading part of the name of the second and the second file
> have a blank at the position where the first file has the dot, wine
> implementation of ShellExecuteEx starts the first executable when the name of
> the second given to ShellExecuteEx.
>
> If a unittest call the second executable via ShellExecuteEx, the first
> executable is started and the test target is the trailing part of the
> filename including the extension. This causes a test failure because the test
> target does not exist.
>
> Example:
> test file.exe
> test file two.exe
>
> Builtin ShellExecuteEx starts always "test file.exe". If "test file two.exe"
> was passed as name to be ShellExecuteEx argv[1] will be "two.exe".
>
> To get around this problem and make a valid unittest without fixing the
> problem, i looked for another way to pass the test target to the executable.

This statement classifies this patch as a hack, not?

[1/2] include/wine: add option to specify the unittest target via environment variable

[Juan Lang]

> > To get around this problem and make a valid unittest without fixing the
> > problem, i looked for another way to pass the test target to the
> > executable.
>
> This statement classifies this patch as a hack, not?

Perhaps, but there's a chicken and egg problem:  fixing the problem
without a valid unit test doesn't seem likely to happen.
--Juan

[1/2] include/wine: add option to specify the unittest target via environment variable

[Paul Vriens]

On 04/20/2010 09:04 PM, Juan Lang wrote:
> > > To get around this problem and make a valid unittest without fixing the
> > > problem, i looked for another way to pass the test target to the
> > > executable.
> >
> > This statement classifies this patch as a hack, not?
>
> Perhaps, but there's a chicken and egg problem:  fixing the problem
> without a valid unit test doesn't seem likely to happen.
> --Juan

But a statement just before that says it 'just' causes a test failure. 
Is there no other way around this?

If we only want to have a test case to prove how Windows works I'd find 
a hack to skip the test on Wine a little bit better.

And why couldn't we fix the problem before submitting tests? This 
happens all the time. If people really want to see proof before changing 
the implementation we can use our testbot to show the tests are correct 
(just give the tests patch to the testbot and run it on all platforms).

[1/2] include/wine: add option to specify the unittest target via environment variable

[Stefan Leichter]

Am Tuesday 20 April 2010 20:57:41 schrieb Paul Vriens:
> On 04/20/2010 08:37 PM, Stefan Leichter wrote:
> > Am Tuesday 20 April 2010 07:59:22 schrieb Paul Vriens:
> > > On 04/19/2010 11:44 PM, Stefan Leichter wrote:
> > > > include/wine/test.h |    6 +++++-
> > > >    1 files changed, 5 insertions(+), 1 deletions(-)
> > >
> > > Hi Stefan,
> > >
> > > What's the idea/thought behind this (or what problem are you trying to
> > > solve)?
> >
> > Hi Paul,
> >
> > the problem is the "command line parsing" of ShellExecuteEx, see bug19666
> > (http://bugs.winehq.org/show_bug.cgi?id=19666#c3).
> >
> > When a directory contains two executables where the name of the first,
> > without extension, is the leading part of the name of the second and the
> > second file have a blank at the position where the first file has the
> > dot, wine implementation of ShellExecuteEx starts the first executable
> > when the name of the second given to ShellExecuteEx.
> >
> > If a unittest call the second executable via ShellExecuteEx, the first
> > executable is started and the test target is the trailing part of the
> > filename including the extension. This causes a test failure because the
> > test target does not exist.
> >
> > Example:
> > test file.exe
> > test file two.exe
> >
> > Builtin ShellExecuteEx starts always "test file.exe". If "test file
> > two.exe" was passed as name to be ShellExecuteEx argv[1] will be
> > "two.exe".
> >
> > To get around this problem and make a valid unittest without fixing the
> > problem, i looked for another way to pass the test target to the
> > executable.
>
> This statement classifies this patch as a hack, not?

What is your suggestion about my last point, which you removed silently?

[1/2] include/wine: add option to specify the unittest target via environment variable

[Paul Vriens]

On 04/21/2010 07:21 AM, Stefan Leichter wrote:
> Am Tuesday 20 April 2010 20:57:41 schrieb Paul Vriens:
> > On 04/20/2010 08:37 PM, Stefan Leichter wrote:
> > > Am Tuesday 20 April 2010 07:59:22 schrieb Paul Vriens:
> > > > On 04/19/2010 11:44 PM, Stefan Leichter wrote:
> > > > > include/wine/test.h |    6 +++++-
> > > > > 1 files changed, 5 insertions(+), 1 deletions(-)
> > > >
> > > > Hi Stefan,
> > > >
> > > > What's the idea/thought behind this (or what problem are you trying to
> > > > solve)?
> > >
> > > Hi Paul,
> > >
> > > the problem is the "command line parsing" of ShellExecuteEx, see bug19666
> > > (http://bugs.winehq.org/show_bug.cgi?id=19666#c3).
> > >
> > > When a directory contains two executables where the name of the first,
> > > without extension, is the leading part of the name of the second and the
> > > second file have a blank at the position where the first file has the
> > > dot, wine implementation of ShellExecuteEx starts the first executable
> > > when the name of the second given to ShellExecuteEx.
> > >
> > > If a unittest call the second executable via ShellExecuteEx, the first
> > > executable is started and the test target is the trailing part of the
> > > filename including the extension. This causes a test failure because the
> > > test target does not exist.
> > >
> > > Example:
> > > test file.exe
> > > test file two.exe
> > >
> > > Builtin ShellExecuteEx starts always "test file.exe". If "test file
> > > two.exe" was passed as name to be ShellExecuteEx argv[1] will be
> > > "two.exe".
> > >
> > > To get around this problem and make a valid unittest without fixing the
> > > problem, i looked for another way to pass the test target to the
> > > executable.
> >
> > This statement classifies this patch as a hack, not?
>
> What is your suggestion about my last point, which you removed silently?

Sorry for that but that last point looked to me like you wanted to have 
a test for ShellExecuteEx without any parameter. Not sure what to 
comment/suggest on that.

I must say that I haven't thoroughly checked the code/patches. It's just 
the statements "get around" and "without fixing the problem" that 
triggered me.