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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Dec 09 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> So, how about doing it this way?  This is based on 'master' and does
>>> not cover contrib/scalar, but if we want to go this route, it should
>>> be trivial to do it on top of a merge of ab/ci-updates and js/scalar
>>> into 'master'.  Good idea?  Terrible idea?  Not good enough?
>>
>> With the caveat that I think the greater direction here makes no sense,
>> i.e. scalar didn't need its own build system etc. in the first place, so
>> having hack-upon-hack to fix various integration issues is clearly worse
>> than just having it behave like everything else....
>
> We decided to start Scalar in contrib/, as it hasn't been proven
> that Scalar is in a good enough shape to deserve to be in this tree,
> and we are giving it a chance by adding it to contrib/ first, hoping
> that it may graduate to the more official status someday [*].
>
> And 'test-extra' is a way to give test coverage to things already in
> contrib/ that has 'test' target in their Makefile.  When js/scalar
> gets merged to a tree with 'test-extra' target, it may be tested in
> that target, too, because we want to have it behave like everything
> else.
>
> [Footnote]
>

I'm referring to the divide between testing things in CI v.s. "make
test" making no sense. Not the path at which scalar is stored in-tree,
which I don't care about, except insofar as it's used as a proxy for
behavior that doesn't make sense.

Scalar uses libgit.a, and is built by default in the js/scalar topic. So
we don't have a choice to really ignore it. E.g. there's part of the
refs.h API used only by it. If you're changing that API you need to test
against scalar.

Which makes sense, and I'd like to have scalar in-tree.

I just don't think it makes any sense that I edit say refs.[ch], run
"make test" locally, but only see that something broke in scalar's
specific use of libgit.a later when I look at GitHub CI.

If you're preparing a series for submission you'll need to get the CI
passing. Except for portability issues etc. it should be trivial to run
the same set of tests locally as we run in CI, that's the case now with
any change you make to libgit and its consumers.

With the scalar topic we lose that 1=1 mapping. I don't think doing that
in the name of it living in contrib makes sense.

If I'm preparing patches for submission I'll need to get CI passing, so
I'll need to fix those tests & behavior either way as it's
in-tree. Knowing about the failures later-not-sooner wastes more time,
not less.

> *1* You may not like the "try unproven things in contrib/ first and
>     then we may graduate it later" approach, but that particular
>     ship has sailed and this is not a time to complain and waste
>     project's time.

We have t/t9902-completion.sh testing contrib/completion/, and we run
that under "make test" and in CI alike, the same goes for
t/t1021-rerere-in-workdir.sh and contrib/workdir/git-new-workdir, we've
got similar (but optional) tests for contrib/credential in t/ too.

The reason we do that with the completion is because some changes to
e.g. tweak getopts will need to have a corresponding change to the
completion.

The reason we've not done that with contrib/{subtree,mw-to-git}/ is
because those are thoroughly in the category of only incidentally being
in-tree.

I.e. we don't have any reason to think that testing those would stress
core features of git itself any more than testing say out-of-tree
software like git-lfs or git-annex.

Testing those is still interesting, but that's for the benefit of that
software itself not bitrotting, not that we're likely to introduce
in-tree breakages by changing an API and missing one of its users.

Scalar is thoroughly on the "completion" side of that divide, not
"subtree".