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

[Junio C Hamano <gitster@pobox.com>]

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 [*].

By running tests on in-tree contrib/ like scalar, at least we would
notice when we are making breaking changes.  At least, the need for
scalar (either for the API broken by such a change to be kept
unchanged or done in a different way, or the code that uses the API
on the scalar side to be updated) would be noticed earlier than
stuff totally outside and not even in contrib/.

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?

> Not that it needs more review, I don't think, as both Stolee and Elijah
> gave their thumbs-up already, and I've not received any feedback that
> would require further changes to `scalar.c`, at least as of _this_ patch
> series.

So that argues even more to have a way to make sure we catch
unintended breakages by any future mindless tree-wide "clean-ups"
and interface changes, no?


[Footnote]

* I just double checked the candidates for "test-extra" to see if
  they are meant to run with a random Git they happen to see on the
  $PATH, or they are designed to test with the version of Git we
  just built, and it seems it is the latter for the ones nominated
  in the test-extra patch.  Otherwise it would indeed reduce the
  benefit in half---we are not helping to catch regressions in the
  core stuff in such a case.