<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 TRANSITIONAL//EN">
<HTML>
<HEAD>
<META HTTP-EQUIV="Content-Type" CONTENT="text/html; CHARSET=UTF-8">
<META NAME="GENERATOR" CONTENT="GtkHTML/3.26.0">
</HEAD>
<BODY>
On Wed, 2011-03-23 at 09:14 +1000, Peter Hutterer wrote:
<BLOCKQUOTE TYPE=CITE>
<PRE>
On Tue, Mar 22, 2011 at 11:26:24AM -0400, Gaetan Nadon wrote:
> On Tue, 2011-03-22 at 11:56 +1000, Peter Hutterer wrote:
>
> > Signed-off-by: Peter Hutterer <<A HREF="mailto:peter.hutterer@who-t.net">peter.hutterer@who-t.net</A>>
> > ---
> > I'll squash this in with the other patch, no need to have two separate ones.
> >
> > test/Makefile.am | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
>
> Now that I have cleared some of my misunderstandings, there is little
> benefits
> in using XORG_ENABLE_UNIT_TESTS for such a simple case,
> unless you foresee a more complex situation in the short term.
> But nothing wrong in using it either. It helps establishing a pattern.
>
> Using the check_PROGRAMS pattern would most likely ease maintenance and
> reduce
> the risk of errors as it would work like most/all other modules.
>
> If there is a need to build the test program in the regular build, it
> could be moved
> to the eventcomm dir. The test dir would simply make use of it. There is
> no obligation
> for programs to be built in the test dir.
answering the other email in this one too:
check_PROGRAMS are the list of binaries built on 'make check'.
TESTS are the list of binaries executed on 'make check'
this is what we currently use in the server. the problem with it is that if
the checks take a while, people are less inclined to run make check. the
result of this is that tests may have build errors until someone who
actually runs make check notices.
the reason why i chose noinst_PROGRAMS here is so that the tests
built when running 'make', but _executed_ when running 'make check'.
this avoids the aforementioned build errors in the tests. so yes,
noinst_PROGRAMS was quite intentional here, and I'm planning to send out a
similar patch for the server.
as for the XORG_ENABLE_UNIT_TESTS, I don't mind having it there, it has no
maintainance requirement and if someone absolutely wants to disable the unit
tests, so be it. this makes more sense in a long-term approach if tests use
libraries that may not be available (such as glib) or if the tests require a
certain setup that may not be available (e.g. running X server).
Cheers,
Peter
>
> > diff --git a/test/Makefile.am b/test/Makefile.am
> > index 16502ee..0b45a2d 100644
> > --- a/test/Makefile.am
> > +++ b/test/Makefile.am
> > @@ -1,3 +1,4 @@
> > +if ENABLE_UNIT_TESTS
> > AM_CPPFLAGS = -I$(top_srcdir)/src
> > AM_CFLAGS = $(XORG_CFLAGS) $(CWARNFLAGS)
> > fake_syms = fake-symbols.c fake-symbols.h
> > @@ -10,6 +11,5 @@ eventcomm_test_SOURCES = eventcomm-test.c\
> > $(fake_syms)
> > endif
> >
> > -if ENABLE_UNIT_TESTS
> > TESTS = $(noinst_PROGRAMS)
> > endif
</PRE>
</BLOCKQUOTE>
<BR>
Reviewed-by: Gaetan Nadon <<A HREF="mailto:memsize@videotron.ca">memsize@videotron.ca</A>>
</BODY>
</HTML>