Re: [1/6] comctl32/tests: Move v6 test initialisation routines into separate file

[Henri Verbeet]

2009/8/10 Owen Rudge :
> This is the first of a series of patches to implement the IImageList
> interface into comctl32, introduced in comctl32 v6.
>
> This first patch splits the routines for loading v6 of comctl32
> (requiring a side-by-side manifest) out of the listview tests and into a
> general file, so that it can be used by the imagelist tests too.

I don't know if there's any reason these tests should be an exception,
but the general rule is that tests are supposed to be contained within
their own file, to allow for easy compilation.

[1/6] comctl32/tests: Move v6 test initialisation routines into separate file

[Nikolay Sivov]

Henri Verbeet wrote:
> 2009/8/10 Owen Rudge :
> > This is the first of a series of patches to implement the IImageList
> > interface into comctl32, introduced in comctl32 v6.
> >
> > This first patch splits the routines for loading v6 of comctl32
> > (requiring a side-by-side manifest) out of the listview tests and into a
> > general file, so that it can be used by the imagelist tests too.
>
> I don't know if there's any reason these tests should be an exception,
> but the general rule is that tests are supposed to be contained within
> their own file, to allow for easy compilation.

He didn't move any tests but only initialization routines. This v6 tests 
should have similar structure for all
comctl32 controls - first <v6 tests, then v6 tests after proper 
initialization.

[1/6] comctl32/tests: Move v6 test initialisation routines into separate file

[Henri Verbeet]

2009/8/11 Nikolay Sivov :
> Henri Verbeet wrote:
> > I don't know if there's any reason these tests should be an exception,
> > but the general rule is that tests are supposed to be contained within
> > their own file, to allow for easy compilation.
>
> He didn't move any tests but only initialization routines. This v6 tests

Yes, but that makes the tests depend on the file containing the
initialization routines. I.e., you now need to compile both listview.c
and v6util.c (as well as needing v6util.h) to create an executable for
the listview tests.

[1/6] comctl32/tests: Move v6 test initialisation routines into separate file

[Paul Vriens]

Henri Verbeet wrote:
> 2009/8/11 Nikolay Sivov :
> > Henri Verbeet wrote:
> > > I don't know if there's any reason these tests should be an exception,
> > > but the general rule is that tests are supposed to be contained within
> > > their own file, to allow for easy compilation.
> >
> > He didn't move any tests but only initialization routines. This v6 tests
>
> Yes, but that makes the tests depend on the file containing the
> initialization routines. I.e., you now need to compile both listview.c
> and v6util.c (as well as needing v6util.h) to create an executable for
> the listview tests.

FWIW, these comctl32 tests also already rely on msg.c for all the 
message related tests.

[1/6] comctl32/tests: Move v6 test initialisation routines into separate file

[Henri Verbeet]

2009/8/11 Paul Vriens :
> FWIW, these comctl32 tests also already rely on msg.c for all the message
> related tests.

It's probably not much of an issue here then.

[1/6] comctl32/tests: Move v6 test initialisation routines into separate file

[Reece Dunn]

2009/8/11 Paul Vriens :
> Henri Verbeet wrote:
> > 2009/8/11 Nikolay Sivov :
> > > Henri Verbeet wrote:
> > > > I don't know if there's any reason these tests should be an exception,
> > > > but the general rule is that tests are supposed to be contained within
> > > > their own file, to allow for easy compilation.
> > >
> > > He didn't move any tests but only initialization routines. This v6 tests
> >
> > Yes, but that makes the tests depend on the file containing the
> > initialization routines. I.e., you now need to compile both listview.c
> > and v6util.c (as well as needing v6util.h) to create an executable for
> > the listview tests.
>
> FWIW, these comctl32 tests also already rely on msg.c for all the message
> related tests.

Is there any reason for that (same with the user32 tests, such as for
the button class).

It means that the msc.c test file is huge and it is not obvious (or
reasonably easy to find out) where the tests are, unless you know
where to look.

- Reece

[1/6] comctl32/tests: Move v6 test initialisation routines into separate file

[Paul Vriens]

Reece Dunn wrote:
> 2009/8/11 Paul Vriens :
> > Henri Verbeet wrote:
> > > 2009/8/11 Nikolay Sivov :
> > > > Henri Verbeet wrote:
> > > > > I don't know if there's any reason these tests should be an exception,
> > > > > but the general rule is that tests are supposed to be contained within
> > > > > their own file, to allow for easy compilation.
> > > >
> > > > He didn't move any tests but only initialization routines. This v6 tests
> > >
> > > Yes, but that makes the tests depend on the file containing the
> > > initialization routines. I.e., you now need to compile both listview.c
> > > and v6util.c (as well as needing v6util.h) to create an executable for
> > > the listview tests.
> >
> > FWIW, these comctl32 tests also already rely on msg.c for all the message
> > related tests.
>
> Is there any reason for that (same with the user32 tests, such as for
> the button class).
>
> It means that the msc.c test file is huge and it is not obvious (or
> reasonably easy to find out) where the tests are, unless you know
> where to look.
> - Reece

Not sure for the reason. But one thing I don't like is that this 
comctl32/msg.c (with no actual tests itself) is also shown on 
test.winehq.org.

[1/6] comctl32/tests: Move v6 test initialisation routines into separate file

[Nikolay Sivov]

Paul Vriens wrote:
> Reece Dunn wrote:
> > 2009/8/11 Paul Vriens :
> > > Henri Verbeet wrote:
> > > > 2009/8/11 Nikolay Sivov :
> > > > > Henri Verbeet wrote:
> > > > > > I don't know if there's any reason these tests should be an 
> > > > > > exception,
> > > > > > but the general rule is that tests are supposed to be contained 
> > > > > > within
> > > > > > their own file, to allow for easy compilation.
> > > > >
> > > > > He didn't move any tests but only initialization routines. This v6 
> > > > > tests
> > > >
> > > > Yes, but that makes the tests depend on the file containing the
> > > > initialization routines. I.e., you now need to compile both listview.c
> > > > and v6util.c (as well as needing v6util.h) to create an executable for
> > > > the listview tests.
> > >
> > > FWIW, these comctl32 tests also already rely on msg.c for all the 
> > > message
> > > related tests.
> >
> > Is there any reason for that (same with the user32 tests, such as for
> > the button class).
> >
> > It means that the msc.c test file is huge and it is not obvious (or
> > reasonably easy to find out) where the tests are, unless you know
> > where to look.
> > - Reece
>
> Not sure for the reason. But one thing I don't like is that this 
> comctl32/msg.c (with no actual tests itself) is also shown on 
> test.winehq.org.

Could we get rid of .c file here and move messaging test helpers to 
header only? It's the way wine/test.h does it.

[1/6] comctl32/tests: Move v6 test initialisation routines into separate file

[Alexandre Julliard]

Nikolay Sivov  writes:
> Paul Vriens wrote:
> > Not sure for the reason. But one thing I don't like is that this
> > comctl32/msg.c (with no actual tests itself) is also shown on
> > test.winehq.org.
>
> Could we get rid of .c file here and move messaging test helpers to
> header only? It's the way wine/test.h does it.

Yes, a header would be preferable.

[1/6] comctl32/tests: Move v6 test initialisation routines into separate file

[Owen Rudge]

> Not sure for the reason. But one thing I don't like is that this 
> comctl32/msg.c (with no actual tests itself) is also shown on 
> test.winehq.org.

That is something that unfortunately does happen with v6util, too 
(v6util shows itself as another test in the test list). However, I 
thought it better than duplicating a significant amount of code - 
particularly since there was already a precedent set with msg.c.

I guess it should be possible to simply move the offending files from 
CTESTS to C_SRCS in the Makefile, perhaps? That would hopefully stop 
those files registering as tests in their own right. I'll try it later.

Placing the code in a header would I be another solution, albeit in my 
opinion a bit of a messy one.