Re: [RFC/PATCH] Makefile: add test-all target

[Jeff King <peff@peff.net>]

On Mon, Dec 13, 2021 at 12:42:37AM -0800, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Teach the main Makefile a "test-extra" target, which goes into each
> >> package in contrib/ whose Makefile has its own "test" target and
> >> runs "make test" there.  Add a "test-all" target to make it easy to
> >> drive both the primary tests and these contrib tests from CI and use
> >> it.
> >
> > That sends a strong message that the stuff in contrib/ is now fully under
> > your maintenance, i.e. first-class supported.
> 
> I do not think running tests on stuff in contrib/ sends any such
> message.  It primarily helps _us_ to catch more regressions than we
> may otherwise miss.  By the way, this is not limited to contrib/; if
> we had tests for gitk, we would have caught the recent regression in
> "diff -m" before it got inflicted on the general public, but that
> would not have been just to help "gitk", but to help keep "diff -m"
> sane and stable [*].

I'd actually be a lot more sympathetic to automatically running gitk
tests, because it's just consuming the public API of git (i.e., the
scriptable plumbing interface). If we accidentally break that, it is the
problem of the person who made the breaking change, and we would want
them to know it as soon as possible.

With something like scalar, though, it is adding new callers of the
private API. It might be useful for somebody doing tree-wide refactoring
to know they've broken something there. But it might also be a hassle,
because now they have to care about fixing it, if they are interested in
un-breaking their build (or un-breaking CI). The scalar code is now
their problem, even though it's "just" in contrib/.

In other words, it comes down to a question of where the burden for
fixing things lies. Of course it is nice if somebody doing tree-wide
refactoring fixes up scalar, too. But by making it optional to build
and/or test stuff in contrib/ (rather than tying it to "make all" or to
CI), it lets people decide how nice they want to be.

For other stuff in contrib/, I'm not sure to what degree it applies.
diff-highlight is pretty standalone for instance. I guess it _could_ be
broken by a public-API change in Git, but I find it pretty unlikely.

> Of course, you have to bear the burden of (A) changing the way
> scalar uses the API, or (B) participating in the design of the
> change to the API that may break scalar's use so that everybody
> including scalar would be happy, or both.  It's not like I am
> responsible for everything that happens in the tree, and it is our
> shared responsibility to maintain the health of the codebase.  It is
> not limited to stuff inside or outside contrib/.
> 
> There are projects that want to use libgit.a by binding us as a
> submodule and without interacting with us very much.  And they are
> on their own when we change the internals.  Do you mean that you
> want to make scalar into the same status as they are?

I kind of thought that final paragraph was the plan, at least to start
with.

-Peff