[PATCH weston 1/5] tests: always build tests

[sardemff7+wayland at sardemff7.net]

On 11/09/2013 11:12, Sam Spilsbury wrote:
> Quick thought: there's also an important psychological effect to
> building the tests on a standard make because it promotes them to the
> same importance as the rest of your code. They become less of an
> afterthought and it promotes greater care around how people design
> the tests (eg, making the tests clean, making sure they run
> quickly), as well as how the rest of the codebase interacts with the
> tests. We observed a similar effect at Canonical between the projects
> which had test building on by default as opposed to those that did
> not.

Then we should definitely fix users (developers) and their workflow,
not some arbitrary “problem”, as I said already.


> It all depends on whether or not the tests are there as a basic
> safety line for managing releases or whether or not tests are used
> as a tool to iterate and improve quality. In the latter case,
> building them by default is a very sensible decision indeed.

Not at all. They should be *run* by default in this case, not just be
built. If their point is to check the code, they must do that, not just
build against some headers. See the end of this email.


On Wed, Sep 11, 2013 at 4:53 PM, Peter Hutterer wrote:
> from my experience, every project I've worked on that has a test
> suite needed this patch eventually, there's always a way to forget
> to run make check and suddenly you find out that it's been broken
> for months. (this is largely because test suites have a tendency to
> become outdated and useless, but...)

Again, users need a “patch”. I’ll add a bit on that at the end of this
email.


> how so? TESTS defines what's run during make check. check_* defines
> what's built, the two are related but not the same.

Because building tests means you need their *dependencies*, which should
definitely not be needed if you do not want to run them.


> I know the principle, I'm claiming that without this, tests will
> eventually break unless you find a way to force everyone to run make
> check.
>
> Which, coincidentally, wastes maintainer time too if you get a patch
> that's fine but breaks make check and you have to get through another
> revision.

So, you have tests that build. Fine. Now what? If nobody run them, they 
are useless. The best way to force them is the vcs. Just add a git hook 
that runs them on commit or push, are you are sure you repo will never 
break. But you should teach people to run them, not force them to do so.

A good example (for me) is Cairo. Their tests are noinst_* already (side 
note: we have to patch that in Exherbo to avoid circular dependencies on 
fresh installs).
They are known to be broken for ages, but they build fine, sure!

Having tests built by default is not a bad idea, it is a bad fix. The 
developer *will* think “Hey, tests build, everything’s ok!” while his 
commit just broke them.


If you want to force tests build or run, you can tweak rules a bit:

if RUN_TESTS_BY_DEFAULT
  ifneq ($(ALREADY_INSIDE),yes)
all-local:
         $(MAKE) ALREADY_INSIDE=yes check
  endif
else
if BUILD_TESTS_BY_DEFAULT
  ifneq ($(ALREADY_INSIDE),yes)
all-local:
         $(MAKE) TESTS= ALREADY_INSIDE=yes check
  endif
endif
endif

It is both more explicit and saner, imo.


As a last note: if tests do not introduce new dependencies *and* are 
quick to build, it is fine to build them by default from a packager’s 
point of view (but not with a check_ vs. noinst_ hack).
But please, do not tell me “it will make people keep them up-to-date”. 
They should be *already*, since people *should* run (and thus build) 
them. Also, having them broken clearly states “we don’t care about 
them”, which is sane if upstream really do not care.

-- 

Quentin “Sardem FF7” Glidic