kde4_add_test is not sufficient

[Alexander Neundorf]

Hi,

On Thursday 07 June 2007 03:00, David Faure wrote:
> On Wednesday 06 June 2007, Andreas Pakulat wrote:
> > On 05.06.07 19:33:55, Thiago Macieira wrote:
> > > Andreas Pakulat wrote:
> > > >Hi,
> > > >
> > > >I'd like to know wether its expected that one needs both kde4_add_test
> > > >and add_test to make tests work even if KDE4_BUILD_TESTS was set to
> > > > off?
> > > >
> > > >If one doesn't tell cmake via add_test() that a testcase exists the
> > > > make test target doesn't find any tests, even after building them via
> > > > make <testname>. I think add_test should be included in
> > > > kde4_add_test, or is there a reason why this is not done?
> > >
> > > Because I forgot about it when I wrote the macro, most likely.
> >
> > So that means add_test can just be added to kde4_add_test?
>
> No, I don't think so. Not all test programs are unit tests that can run
> automatically.
>
> All those interactive GUI test programs need to be built with kde4_add_test
> (so that you can use "make krulertest"), but shouldn't be run automatically
> by 'make test', so we don't want to call add_test for them.
>
> The naming is a bit unfortunate IMHO. "kde4_add_test" is called this way
> to look like "kde4_add_executable", i.e. "build this test program"
> (which could be automated or interactive). But CMake's "add_test" means
> "run this automated test", and the two macros look too much alike which
> leads to confusion.
>
> Maybe kde4_add_test should be renamed to kde4_add_test_executable
> at least (like kde4_add_kdeinit_executable).

That's just what I thought too, kde4_add_test_executable().

Beside that, maybe we're not using cmake/ctest optimal by testing via "make 
test" but we should do "ctest <some fancy options>" instead...

> To solve your problem, we could have kde4_add_unit_test I suppose,
> which would do both (kde4_add_test_executable + add_test).

Not sure we need this. If it just adds add_test() to 
kde4_add_test_executable() it might more confuse than help.

Bye
Alex