On Tue, Feb 27, 2018 at 01:04:31PM +0100, Lukáš Hrázký wrote:
> 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 :)
This logic is following what the autotools give us, a separate make
check target. With this patch, the situation becomes odd I think,
configure.ac requires an unusual (imo) dependency ('catch') which is
then not going to be used in our default build (a straight 'make').
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel