Re: [PATCH spice-streaming-agent] Only build unit tests when running make check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2018-02-27 at 06:33 -0500, Frediano Ziglio wrote:
> > 
> > Removing noinst_PROGRAMS from src/unittests/Makefile.am will cause the
> > unit tests not be built on a regular build (`make`), they will only be
> > built when running the tests (`make check`).
> > 
> > C++ unit test frameworks are notorious for long build times too.
> > 
> > Signed-off-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> > ---
> > I am not sure if there was another reason for noinst_PROGRAMS, but I
> > don't think so.
> > 
> 
> Yes, this is mainly what it does. In spice-server we have same pattern,
> the reason to have it is the opposite of this patch, we want to make
> sure tests compile even if they are not executed.
> 
> If the final aim of this patch is to avoid the requirement of catch
> library this patch is just partial.
> 
> If the aim is to just save some time avoiding to compile tests
> the patch is fine but looks like presenting different behaviour
> comparing to other SPICE projects.

Not to avoid the Catch dependency, to save time and just not do what
the user doesn't want to do. If he wants to build the project, he
doesn't need the tests. You only really need to build the tests if you
want to run them.

For SPICE projects in general, I think the logic is backwards. We do
not want to build the tests on a regular build to make sure they do
build. We want to (build and) run the unit tests all the time to make
sure not only they build, but also that they pass :)

Lukas

> >  src/unittests/Makefile.am | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/src/unittests/Makefile.am b/src/unittests/Makefile.am
> > index 0fc6d50..cf7adba 100644
> > --- a/src/unittests/Makefile.am
> > +++ b/src/unittests/Makefile.am
> > @@ -23,10 +23,6 @@ TESTS = \
> >  	test-mjpeg-fallback \
> >  	$(NULL)
> >  
> > -noinst_PROGRAMS = \
> > -	$(check_PROGRAMS) \
> > -	$(NULL)
> > -
> >  hexdump_SOURCES = \
> >  	hexdump.c \
> >  	$(NULL)
> 
> Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]