Subject: Re: [PATCH 1/1] Makefile: add prove and coverage-prove targets

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 30 Jan 2019 14:08:28 +0100

On Wed, Jan 30 2019, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Tue, 29 Jan 2019, Ævar Arnfjörð Bjarmason wrote:
>
>> On Tue, Jan 29 2019, Jeff King wrote:
>>
>> > On Tue, Jan 29, 2019 at 06:56:08AM -0800, Derrick Stolee via
>> > GitGitGadget wrote:
>> >
>> >> From: Derrick Stolee <[email-masked]>
>> >>
>> >> When running the test suite for code coverage using
>> >> 'make coverage-test', a single test failure stops the
>> >> test suite from completing. This leads to significant
>> >> undercounting of covered blocks.
>> >>
>> >> Add two new targets to the Makefile:
>> >>
>> >> * 'prove' runs the test suite using 'prove'.
>> >>
>> >> * 'coverage-prove' compiles the source using the
>> >>   coverage flags, then runs the test suite using
>> >>   'prove'.
>> >>
>> >> These targets are modeled after the 'test' and
>> >> 'coverage-test' targets.
>> >
>> > I think these are reasonable to have (and I personally much prefer
>> > "prove" to the raw "make test" output anyway).
>>
>> I wonder if anyone would mind if we removed the non-prove path.
>>
>> When I added it in 5099b99d25 ("test-lib: Adjust output to be valid TAP
>> format", 2010-06-24) there were still some commonly shipped OS's that
>> had a crappy old "prove", but now almost a decade later that's not a
>> practical problem, and it's installed by default with perl, and we
>> already depend on perl for the tests.
>
> It's not only about crappy old `prove`, it is also about requiring Perl
> (and remember, Perl is not really native in Git for Windows' case;

We require perl now for testing, NO_PERL is just for the installed
version of git. If you change the various test-lib.sh and
test-lib-functions.sh that unconditionally uses "perl" or "$PERL_PATH"
hundreds/thousands (didn't take an exact count, just watched fail scroll
by) tests fail.

So my assumption is that anyone running the tests now has perl anyway,
and thus a further hard dependency on it won't hurt anything.

> I still have a hunch that we could save on time *dramatically* by
> simply running through regular `make` rather than through `prove`).

My hunch is that on the OS's where this would matter (e.g. Windows) the
overhead is mainly spawning the processes, and it doesn't matter if it's
make or perl doing the spawning, but I have nothing to back that up...

> I did start to implement a parallel test runner for use with BusyBox-based
> MinGit, but dropped the ball on that front before I could satisfy myself
> that this is robust enough. Once it *is* robust enough, we could even
> replace the entire `prove` support with a native, test-tool driven test
> framework.

Let's be clear about what "prove support" means.

When I added support for "prove" I was really adding support for TAP
which is a standardized test output protocol: https://testanything.org/

It just so happens that "prove" is the most ubiquitous implementation,
but there's plenty of others: https://testanything.org/consumers.html

So hard-depending on "prove" in no way ties us to that particular tool
forever. We'd just do away with ferrying information via a side-channel
(*.counts files) that we also get from its output.

>> I don't feel strongly about it, but it would allow us to prune some
>> login in the test library / Makefile.
>>
>> Maybe something for a show of hands at the contributor summit?
>
> Sure, let's put it up for discussion.

*Nod*