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
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to