threads / patch / 50349

patchMakefile: add prove and coverage-prove targets

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

## tl;dr

22 messages between Jan 29, 2019 and Jan 31, 2019. Diffs are folded; open one to read it.

replies: 21people: 5as markdown or json

Derrick Stolee via GitGitGadget· Jan 29, 2019, 14:56 UTC · lore

Sometimes there are test failures in the 'pu' branch. This is somewhat expected for a branch that takes the very latest topics under development, and those sometimes have semantic conflicts that only show up during test runs. This also can happen when running the test suite with different GIT_TEST_* environment variables that interact in unexpected ways.

This causes a problem for the test coverage reports, as the typical 'make coverage-test coverage-report' run halts at the first failed test. If that test is early in the suite, then many valuable tests are not exercising the code and the coverage report becomes noisy with false positives.

This patch adds two targets to the Makefile: 'prove' and 'coverage-prove'. The idea is to use the 'prove' tool to run the test suite, as it will track failed tests but continue running the full suite even with a failure.

If/when this merges down, I will use this new target for the automation around the test coverage reports.

Thanks, -Stolee

Derrick Stolee (1):
  Makefile: add prove and coverage-prove targets

 Makefile | 7 +++++++
 1 file changed, 7 insertions(+)

base-commit: 0d0ac3826a3bbb9247e39e12623bbcfdd722f24c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-114%2Fderrickstolee%2Fcoverage-prove-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-114/derrickstolee/coverage-prove-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/114

-- 
gitgitgadget

Derrick Stolee via GitGitGadget· Jan 29, 2019, 14:56 UTC · re: Derrick Stolee via GitGitGadget · lore

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

From: Derrick Stolee <dstolee@microsoft.com>

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.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Makefile | 7 +++++++
 1 file changed, 7 insertions(+)

Show changes to Makefile +7 −0

diff --git a/Makefile b/Makefile
index 1a44c811aa..ec886635ae 100644
--- a/Makefile
+++ b/Makefile
@@ -2665,6 +2665,9 @@ export TEST_NO_MALLOC_CHECK
 test: all
 	$(MAKE) -C t/ all
 
+prove: all
+	$(MAKE) -C t/ prove
+
 perf: all
 	$(MAKE) -C t/perf/ all
 
@@ -3077,6 +3080,10 @@ coverage-test: coverage-clean-results coverage-compile
 	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
 		DEFAULT_TEST_TARGET=test -j1 test
 
+coverage-prove: coverage-clean-results coverage-compile
+	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
+		DEFAULT_TEST_TARGET=prove -j1 prove
+
 coverage-report:
 	$(QUIET_GCOV)for dir in $(object_dirs); do \
 		$(GCOV) $(GCOVFLAGS) --object-directory=$$dir $$dir*.c || exit; \

-- 
gitgitgadget

Johannes Schindelin· Jan 29, 2019, 15:20 UTC · re: Derrick Stolee via GitGitGadget · lore

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

Hi Stolee,

On Tue, 29 Jan 2019, Derrick Stolee via GitGitGadget wrote:

Show 17 quoted lines

> From: Derrick Stolee <dstolee@microsoft.com>
> 
> 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.

The rationale, and the diff (after reading what `test` and `coverage-test` do), make a lot of sense to me.

Thanks, Dscho

Show 34 quoted lines

> 
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  Makefile | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 1a44c811aa..ec886635ae 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2665,6 +2665,9 @@ export TEST_NO_MALLOC_CHECK
>  test: all
>  	$(MAKE) -C t/ all
>  
> +prove: all
> +	$(MAKE) -C t/ prove
> +
>  perf: all
>  	$(MAKE) -C t/perf/ all
>  
> @@ -3077,6 +3080,10 @@ coverage-test: coverage-clean-results coverage-compile
>  	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
>  		DEFAULT_TEST_TARGET=test -j1 test
>  
> +coverage-prove: coverage-clean-results coverage-compile
> +	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
> +		DEFAULT_TEST_TARGET=prove -j1 prove
> +
>  coverage-report:
>  	$(QUIET_GCOV)for dir in $(object_dirs); do \
>  		$(GCOV) $(GCOVFLAGS) --object-directory=$$dir $$dir*.c || exit; \
> -- 
> gitgitgadget
>

SZEDER Gábor· Jan 29, 2019, 15:58 UTC · re: Derrick Stolee via GitGitGadget · lore

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

On Tue, Jan 29, 2019 at 06:56:08AM -0800, Derrick Stolee via GitGitGadget wrote:

Show 15 quoted lines

> 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 the cover letter would be a better commit message.

Show 16 quoted lines

> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  Makefile | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 1a44c811aa..ec886635ae 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2665,6 +2665,9 @@ export TEST_NO_MALLOC_CHECK
>  test: all
>  	$(MAKE) -C t/ all
>  
> +prove: all
> +	$(MAKE) -C t/ prove
> +

You don't need this 'prove' target in the "main" Makefile, because 'make test' will run the test suite using DEFAULT_TEST_TARGET anyway.

Show 10 quoted lines

>  perf: all
>  	$(MAKE) -C t/perf/ all
>  
> @@ -3077,6 +3080,10 @@ coverage-test: coverage-clean-results coverage-compile
>  	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
>  		DEFAULT_TEST_TARGET=test -j1 test
>  
> +coverage-prove: coverage-clean-results coverage-compile
> +	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
> +		DEFAULT_TEST_TARGET=prove -j1 prove

First I was wondering why do you need a dedicated 'coverage-prove' target, instead of letting DEFAULT_TEST_TARGET from the environment or from 'config.mak' do its thing. But then I noticed in the hunk context, that, for some reason, the 'coverage-test' target hardcoded 'DEFAULT_TEST_TARGET=test -j1'. Then I was wondering why would it want to do that, and stumbled upon commit c14cc77c11:

    coverage: set DEFAULT_TEST_TARGET to avoid using prove
    
    If the user sets DEFAULT_TEST_TARGET=prove in his config.mak, that
    carries over into the coverage tests.  Which is really bad if he also
    sets GIT_PROVE_OPTS=-j<..> as that completely breaks the coverage
    runs.
    
    Instead of attempting to mess with the GIT_PROVE_OPTS, just force the
    test target to 'test' so that we run under make, like we intended all
    along.

I'm afraid that this issue would badly affect 'coverage-prove' as well (I didn't try). Or if doesn't (anymore?), then that should be mentioned in the commit message, and then perhaps it's time to remove that '-j1' from the 'coverage-test' target as well.

Derrick Stolee· Jan 29, 2019, 16:37 UTC · re: SZEDER Gábor · lore

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

On 1/29/2019 10:58 AM, SZEDER Gábor wrote:

Show 7 quoted lines

> On Tue, Jan 29, 2019 at 06:56:08AM -0800, Derrick Stolee via GitGitGadget wrote:
>> +prove: all
>> +	$(MAKE) -C t/ prove
>> +
> 
> You don't need this 'prove' target in the "main" Makefile, because
> 'make test' will run the test suite using DEFAULT_TEST_TARGET anyway.

Thanks!

Show 21 quoted lines

>> +coverage-prove: coverage-clean-results coverage-compile
>> +	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
>> +		DEFAULT_TEST_TARGET=prove -j1 prove
> 
> First I was wondering why do you need a dedicated 'coverage-prove'
> target, instead of letting DEFAULT_TEST_TARGET from the environment or
> from 'config.mak' do its thing.  But then I noticed in the hunk
> context, that, for some reason, the 'coverage-test' target hardcoded
> 'DEFAULT_TEST_TARGET=test -j1'.  Then I was wondering why would it
> want to do that, and stumbled upon commit c14cc77c11:
> 
>     coverage: set DEFAULT_TEST_TARGET to avoid using prove
>     
>     If the user sets DEFAULT_TEST_TARGET=prove in his config.mak, that
>     carries over into the coverage tests.  Which is really bad if he also
>     sets GIT_PROVE_OPTS=-j<..> as that completely breaks the coverage
>     runs.
>     
>     Instead of attempting to mess with the GIT_PROVE_OPTS, just force the
>     test target to 'test' so that we run under make, like we intended all
>     along.

Thanks for finding this!

> I'm afraid that this issue would badly affect 'coverage-prove' as well
> (I didn't try).  Or if doesn't (anymore?), then that should be
> mentioned in the commit message, and then perhaps it's time to remove
> that '-j1' from the 'coverage-test' target as well.

I'll fix this by forcing an update to GIT_PROVE_OPTS. It does limit our ability to use GIT_PROVE_OPTS as a pass-through, but at least this new target will have that assumption built in.

Thanks, -Stolee

Jeff King· Jan 29, 2019, 16:49 UTC · re: Derrick Stolee · lore

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

On Tue, Jan 29, 2019 at 11:37:34AM -0500, Derrick Stolee wrote:

Show 8 quoted lines

> > I'm afraid that this issue would badly affect 'coverage-prove' as well
> > (I didn't try).  Or if doesn't (anymore?), then that should be
> > mentioned in the commit message, and then perhaps it's time to remove
> > that '-j1' from the 'coverage-test' target as well.
> 
> I'll fix this by forcing an update to GIT_PROVE_OPTS. It does limit our
> ability to use GIT_PROVE_OPTS as a pass-through, but at least this new
> target will have that assumption built in.

I think doing:

  make GIT_PROVE_OPTS="$(GIT_PROVE_OPTS) -j1"

should be sufficient to pass through most of the options but override -j, since prove does the usual left-to-right "last one wins" behavior for its options.

-Peff

SZEDER Gábor· Jan 29, 2019, 17:34 UTC · re: SZEDER Gábor · lore

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

On Tue, Jan 29, 2019 at 04:58:27PM +0100, SZEDER Gábor wrote:

Show 22 quoted lines

> On Tue, Jan 29, 2019 at 06:56:08AM -0800, Derrick Stolee via GitGitGadget wrote:
> > @@ -3077,6 +3080,10 @@ coverage-test: coverage-clean-results coverage-compile
> >  	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
> >  		DEFAULT_TEST_TARGET=test -j1 test
> >  
> > +coverage-prove: coverage-clean-results coverage-compile
> > +	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
> > +		DEFAULT_TEST_TARGET=prove -j1 prove
> 
> First I was wondering why do you need a dedicated 'coverage-prove'
> target, instead of letting DEFAULT_TEST_TARGET from the environment or
> from 'config.mak' do its thing.  But then I noticed in the hunk
> context, that, for some reason, the 'coverage-test' target hardcoded
> 'DEFAULT_TEST_TARGET=test -j1'.  Then I was wondering why would it
> want to do that, and stumbled upon commit c14cc77c11:
> 
>     coverage: set DEFAULT_TEST_TARGET to avoid using prove
>     
>     If the user sets DEFAULT_TEST_TARGET=prove in his config.mak, that
>     carries over into the coverage tests.  Which is really bad if he also
>     sets GIT_PROVE_OPTS=-j<..> as that completely breaks the coverage
>     runs.

So this text is really dramatic and implies (to me, at least), that parallelized coverage builds just don't work. I've just run a coverage build with this patch and my usual GIT_PROVE_OPTS containing '-j4', and the tests went well and the generated report looks good, too (I don't know how it's supposed to look, but at least I didn't notice anything obviously wrong with it). However, this might mean nothing, because further digging turned up the follow paragraph in 901c369af5 (Support coverage testing with GCC/gcov, 2009-02-19):

    The tests are run serially (with -j1).  The coverage code should
    theoretically allow concurrent access to its data files, but the
    author saw random test failures.  Obviously this could be
    improved.

And in the related email discussion [1]:

  But even though the docs claim it [-j<N>] should be possible,
  I've been getting "random" test failures when compiled with coverage
  support, that went away with -j1.  So the tests still run with -j1, as
  with the first version of the series.

So it doesn't seem to be that bad after all, because it's not "completely breaks" but "random test failures". Still far from ideal, but the original coverage patch is just about 3 weeks short of a decade old, so maybe things have improved since then, and it'd be worth a try to leave GIT_PROVE_OPTS as is and see what happens.

[1] https://public-inbox.org/git/200902191512.16755.trast@student.ethz.ch/

Derrick Stolee· Jan 29, 2019, 18:10 UTC · re: SZEDER Gábor · lore

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

On 1/29/2019 12:34 PM, SZEDER Gábor wrote:

Show 13 quoted lines

> On Tue, Jan 29, 2019 at 04:58:27PM +0100, SZEDER Gábor wrote:
> And in the related email discussion [1]:
> 
>   But even though the docs claim it [-j<N>] should be possible,
>   I've been getting "random" test failures when compiled with coverage
>   support, that went away with -j1.  So the tests still run with -j1, as
>   with the first version of the series.
> 
> So it doesn't seem to be that bad after all, because it's not
> "completely breaks" but "random test failures".  Still far from ideal,
> but the original coverage patch is just about 3 weeks short of a
> decade old, so maybe things have improved since then, and it'd be
> worth a try to leave GIT_PROVE_OPTS as is and see what happens.

It would certainly be nice if the build time could be reduced through parallel test runs. I've kicked off a build using GIT_PROVE_OPTS="-j12" to see what happens.

Thanks! -Stolee

Derrick Stolee· Jan 29, 2019, 20:49 UTC · re: Derrick Stolee · lore

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

On 1/29/2019 1:10 PM, Derrick Stolee wrote:

Show 18 quoted lines

> On 1/29/2019 12:34 PM, SZEDER Gábor wrote:
>> On Tue, Jan 29, 2019 at 04:58:27PM +0100, SZEDER Gábor wrote:
>> And in the related email discussion [1]:
>>
>>   But even though the docs claim it [-j<N>] should be possible,
>>   I've been getting "random" test failures when compiled with coverage
>>   support, that went away with -j1.  So the tests still run with -j1, as
>>   with the first version of the series.
>>
>> So it doesn't seem to be that bad after all, because it's not
>> "completely breaks" but "random test failures".  Still far from ideal,
>> but the original coverage patch is just about 3 weeks short of a
>> decade old, so maybe things have improved since then, and it'd be
>> worth a try to leave GIT_PROVE_OPTS as is and see what happens.
> 
> It would certainly be nice if the build time could be reduced through
> parallel test runs. I've kicked off a build using GIT_PROVE_OPTS="-j12"
> to see what happens.

I did get a failed test with this run:

t0025-crlf-renormalize.sh                 (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1

This was on the 'jch' branch, and an equivalent build with sequential execution did not have this failure. That's flaky enough for me to stick to sequential runs.

Thanks,
-Stolee

SZEDER Gábor· Jan 29, 2019, 21:58 UTC · re: Derrick Stolee · lore

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

On Tue, Jan 29, 2019 at 03:49:58PM -0500, Derrick Stolee wrote:

Show 29 quoted lines

> On 1/29/2019 1:10 PM, Derrick Stolee wrote:
> > On 1/29/2019 12:34 PM, SZEDER Gábor wrote:
> >> On Tue, Jan 29, 2019 at 04:58:27PM +0100, SZEDER Gábor wrote:
> >> And in the related email discussion [1]:
> >>
> >>   But even though the docs claim it [-j<N>] should be possible,
> >>   I've been getting "random" test failures when compiled with coverage
> >>   support, that went away with -j1.  So the tests still run with -j1, as
> >>   with the first version of the series.
> >>
> >> So it doesn't seem to be that bad after all, because it's not
> >> "completely breaks" but "random test failures".  Still far from ideal,
> >> but the original coverage patch is just about 3 weeks short of a
> >> decade old, so maybe things have improved since then, and it'd be
> >> worth a try to leave GIT_PROVE_OPTS as is and see what happens.
> > 
> > It would certainly be nice if the build time could be reduced through
> > parallel test runs. I've kicked off a build using GIT_PROVE_OPTS="-j12"
> > to see what happens.
> 
> I did get a failed test with this run:
> 
> t0025-crlf-renormalize.sh                 (Wstat: 256 Tests: 3 Failed: 1)
>   Failed test:  2
>   Non-zero exit status: 1
> 
> This was on the 'jch' branch, and an equivalent build with sequential
> execution did not have this failure. That's flaky enough for me to stick
> to sequential runs.

That failure is not coverage-related, but as it turned out 9e5da3d055 (add: use separate ADD_CACHE_RENORMALIZE flag, 2019-01-17) made t0025 rather flaky:

  https://public-inbox.org/git/20190129213533.GE13764@szeder.dev/

When reading those old commit messages and discussions in the afternoon, I was wondering what "random test failures" actually meant, since it was not stated explicitly that it was coverage-related. For all we know it could have been "general" test flakiness that happened to manifest under the higher load of a parallel test run.

Jeff King· Jan 29, 2019, 16:00 UTC · re: Derrick Stolee via GitGitGadget · lore

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

On Tue, Jan 29, 2019 at 06:56:08AM -0800, Derrick Stolee via GitGitGadget wrote:

Show 17 quoted lines

> From: Derrick Stolee <dstolee@microsoft.com>
> 
> 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).

For people who don't have "prove" available, I think they could just do "make -k test" to make sure the full suite runs. Should we perhaps be doing that automatically in the sub-make run by coverage-test?

Show 8 quoted lines

> @@ -3077,6 +3080,10 @@ coverage-test: coverage-clean-results coverage-compile
>  	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
>  		DEFAULT_TEST_TARGET=test -j1 test
>  
> +coverage-prove: coverage-clean-results coverage-compile
> +	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
> +		DEFAULT_TEST_TARGET=prove -j1 prove
> +

You probably don't need to override DEFAULT_TEST_TARGET here, since the "prove" target doesn't look at it. Likewise, "-j1" probably does nothing here, since prove itself is a single target.

I'm not sure why we want to enforce -j1 for these targets, but if it's important to do so for the prove case, as well, you'd need to add it to GIT_PROVE_OPTS.

-Peff

Derrick Stolee· Jan 29, 2019, 16:35 UTC · re: Jeff King · lore

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

On 1/29/2019 11:00 AM, Jeff King wrote:

Show 26 quoted lines

> On Tue, Jan 29, 2019 at 06:56:08AM -0800, Derrick Stolee via GitGitGadget wrote:
> 
>> From: Derrick Stolee <dstolee@microsoft.com>
>>
>> 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).
> 
> For people who don't have "prove" available, I think they could just do
> "make -k test" to make sure the full suite runs. Should we perhaps be
> doing that automatically in the sub-make run by coverage-test?

I wanted to avoid changing the existing behavior, if I could. But, if we can reasonably assume that anyone running 'make coverage-test' wants to run the full suite even with failures, then that's fine by me.

I see from the make docs that '-k' will still result in an error code at the end of the command, so no automation would result in an incorrect response to a failed test. Am I correct?

Show 12 quoted lines

>> @@ -3077,6 +3080,10 @@ coverage-test: coverage-clean-results coverage-compile
>>  	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
>>  		DEFAULT_TEST_TARGET=test -j1 test
>>  
>> +coverage-prove: coverage-clean-results coverage-compile
>> +	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
>> +		DEFAULT_TEST_TARGET=prove -j1 prove
>> +
> 
> You probably don't need to override DEFAULT_TEST_TARGET here, since the
> "prove" target doesn't look at it. Likewise, "-j1" probably does nothing
> here, since prove itself is a single target.

As Szeder mentioned, I can probably just drop the 'prove' target and use DEFAULT_TEST_TARGET instead. Or do we think anyone will want to use 'make prove' from root?

> I'm not sure why we want to enforce -j1 for these targets, but if it's
> important to do so for the prove case, as well, you'd need to add it to
> GIT_PROVE_OPTS.

The '-j1' is necessary because the coverage data is collected in a way that is not thread-safe. Our compile options also force single-threaded behavior.

I'll specifically override GIT_PROVE_OPTS here to force -j1, but also send -j1 to the 'make' command, too.

Thanks, -Stolee

Jeff King· Jan 29, 2019, 16:46 UTC · re: Derrick Stolee · lore

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

On Tue, Jan 29, 2019 at 11:35:41AM -0500, Derrick Stolee wrote:

Show 7 quoted lines

> > For people who don't have "prove" available, I think they could just do
> > "make -k test" to make sure the full suite runs. Should we perhaps be
> > doing that automatically in the sub-make run by coverage-test?
> 
> I wanted to avoid changing the existing behavior, if I could. But, if
> we can reasonably assume that anyone running 'make coverage-test' wants
> to run the full suite even with failures, then that's fine by me.

Another option would be to relay "-k" from the caller. I think it's not enough to just use $(MAKE), but if you use $(MAKE) $(MAKEFLAGS), then running "make -k coverage-test" from your coverage script would (I think) do what you want.

> I see from the make docs that '-k' will still result in an error code
> at the end of the command, so no automation would result in an incorrect
> response to a failed test. Am I correct?

Yeah, that matches my understanding. I don't think you'd have to deal with that failure code manually for coverage-report because it does not depend on coverage-test (but obviously if you did "make coverage-test && make coverage-report", the "&&" needs to become a semicolon).

Show 12 quoted lines

> >> +coverage-prove: coverage-clean-results coverage-compile
> >> +	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
> >> +		DEFAULT_TEST_TARGET=prove -j1 prove
> >> +
> > 
> > You probably don't need to override DEFAULT_TEST_TARGET here, since the
> > "prove" target doesn't look at it. Likewise, "-j1" probably does nothing
> > here, since prove itself is a single target.
> 
> As Szeder mentioned, I can probably just drop the 'prove' target and use
> DEFAULT_TEST_TARGET instead. Or do we think anyone will want to use
> 'make prove' from root?

Yeah, that works. I typically do run prove, and I do run the tests from the root, but DEFAULT_TEST_TARGET already makes it all work for me. So yeah, I think it's fine to leave off the prove target until somebody actually wants it.

Show 6 quoted lines

> > I'm not sure why we want to enforce -j1 for these targets, but if it's
> > important to do so for the prove case, as well, you'd need to add it to
> > GIT_PROVE_OPTS.
> 
> The '-j1' is necessary because the coverage data is collected in a way that
> is not thread-safe. Our compile options also force single-threaded behavior.

Ah, right, I vaguely recall that now.

> I'll specifically override GIT_PROVE_OPTS here to force -j1, but also send
> -j1 to the 'make' command, too.

Makes sense.

-Peff

Ævar Arnfjörð Bjarmason· Jan 29, 2019, 21:03 UTC · re: Jeff King · lore

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

On Tue, Jan 29 2019, Jeff King wrote:

Show 22 quoted lines

> On Tue, Jan 29, 2019 at 06:56:08AM -0800, Derrick Stolee via GitGitGadget wrote:
>
>> From: Derrick Stolee <dstolee@microsoft.com>
>>
>> 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.

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?

Jeff King· Jan 29, 2019, 22:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

On Tue, Jan 29, 2019 at 10:03:46PM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 15 quoted lines

> > 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.
> 
> 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?

Certainly no argument from me personally, but I do wonder if anybody uses the old one. Maybe we could mark it with a deprecation notice or something. I'd be happy to do a straw poll at the contributor summit (or on the list, if anybody reads this ;) ).

-Peff

Johannes Schindelin· Jan 30, 2019, 12:20 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Hi Ævar,

On Tue, 29 Jan 2019, Ævar Arnfjörð Bjarmason wrote:

Show 33 quoted lines

> 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 <dstolee@microsoft.com>
> >>
> >> 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; I still have a hunch that we could save on time *dramatically* by simply running through regular `make` rather than through `prove`).

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.

> 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.

Ciao, Dscho

Ævar Arnfjörð Bjarmason· Jan 30, 2019, 13:08 UTC · re: Johannes Schindelin · lore

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

On Wed, Jan 30 2019, Johannes Schindelin wrote:

Show 40 quoted lines

> 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 <dstolee@microsoft.com>
>> >>
>> >> 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...

Show 5 quoted lines

> 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.

Show 6 quoted lines

>> 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*

Johannes Schindelin· Jan 30, 2019, 18:42 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Hi Ævar,

On Wed, 30 Jan 2019, Ævar Arnfjörð Bjarmason wrote:

Show 43 quoted lines

> On Wed, Jan 30 2019, Johannes Schindelin wrote:
> 
> > 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 <dstolee@microsoft.com>
> >> >>
> >> >> 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.

Which is confusing, if you want to put it nicely.

> 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.

I know. Oh boy, I know.

But we do not have to keep that status quo, nor do we have to make it worse.

It would not surprise me in the least if we could accelerate our entire test suite by reducing our heavy reliance on scripting (including Perl) to the point that it really takes too little time *not* to run. (Right now, if you are on Windows, you better think twice before you start the test suite, it will easily take over 3h (!!!) to run in a regular developer setup. Even on a regular Mac, I would think twice before starting the run that blocks my machine for easily 20 minutes straight. Needless to say that few developers, if any, use it to validate their patches, in particular on Windows. Meaning: for all real purposes, the test suite is nearly useless on Windows.)

So let's not bake *even more* Perl usage into our test suite. Thanks.

> 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.

By that token, the effort to turn many a script into a built-in for better performance and substantially better error checking would be totally nonsensical. "Because anyone running Git used those scripts anyway, so making them a hard dependency won't hurt anything"?

I do not believe even a fraction of a second that that effort is nonsensical. Just like I do not believe even a fraction of a second that it makes sense for our test suite to rely on scripting so much. Or for us to make that reliance even bigger, for that matter.

Show 6 quoted lines

> > 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 have at least the experience of several thousands runs of the test suite on Windows, together with a couple dozen hours spent recently *just* on making the CI of GitGitGadget at least bearable.

So I do not quite understand why you offered a contrary opinion when you have nothing to back it up.

I mean, I would really like to have an informed discussion with you, to benefit from your skills and from your experience to make the entire design of our test suite better (there is so much room for improvement, we should really be able to put together our knowledge to enhance it). It needs to be based on facts, of course.

Ciao, Dscho

Ævar Arnfjörð Bjarmason· Jan 30, 2019, 19:32 UTC · re: Johannes Schindelin · lore

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

On Wed, Jan 30 2019, Johannes Schindelin wrote:

Show 104 quoted lines

> Hi Ævar,
>
> On Wed, 30 Jan 2019, Ævar Arnfjörð Bjarmason wrote:
>
>> On Wed, Jan 30 2019, Johannes Schindelin wrote:
>>
>> > 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 <dstolee@microsoft.com>
>> >> >>
>> >> >> 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.
>
> Which is confusing, if you want to put it nicely.
>
>> 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.
>
> I know. Oh boy, I know.
>
> But we do not have to keep that status quo, nor do we have to make it
> worse.
>
> It would not surprise me in the least if we could accelerate our entire
> test suite by reducing our heavy reliance on scripting (including Perl) to
> the point that it really takes too little time *not* to run. (Right now,
> if you are on Windows, you better think twice before you start the test
> suite, it will easily take over 3h (!!!) to run in a regular developer
> setup. Even on a regular Mac, I would think twice before starting the run
> that blocks my machine for easily 20 minutes straight. Needless to say
> that few developers, if any, use it to validate their patches, in
> particular on Windows. Meaning: for all real purposes, the test suite is
> nearly useless on Windows.)
>
> So let's not bake *even more* Perl usage into our test suite. Thanks.
>
>> 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.
>
> By that token, the effort to turn many a script into a built-in for better
> performance and substantially better error checking would be totally
> nonsensical. "Because anyone running Git used those scripts anyway, so
> making them a hard dependency won't hurt anything"?
>
> I do not believe even a fraction of a second that that effort is
> nonsensical. Just like I do not believe even a fraction of a second that
> it makes sense for our test suite to rely on scripting so much. Or for us
> to make that reliance even bigger, for that matter.
>
>> > 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 have at least the experience of several thousands runs of the test suite
> on Windows, together with a couple dozen hours spent recently *just* on
> making the CI of GitGitGadget at least bearable.
>
> So I do not quite understand why you offered a contrary opinion when you
> have nothing to back it up.
>
> I mean, I would really like to have an informed discussion with you, to
> benefit from your skills and from your experience to make the entire
> design of our test suite better (there is so much room for improvement, we
> should really be able to put together our knowledge to enhance it). It
> needs to be based on facts, of course.

Let's get some numbers then. On master, go to the "t" directory and run this:

    for f in t[0-9]*.sh; do (echo '#!/bin/sh' && echo "echo ok 1 $f" && echo sleep 1 && echo echo 1..1) >$f; done

That effectively turns all our tests into a "hello world" with a sleep of 1 second.

Then run both:

    time prove -j12 t00[0-9]*.sh

And:

    time make -j12 t00[0-9]*.sh

For some value of -j12 and t00[0-9]*.sh. In my testing "make" is a bit faster, but not by any amount that would matter when this is run for real.

Johannes Schindelin· Jan 31, 2019, 07:23 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Hi Ævar,

On Wed, 30 Jan 2019, Ævar Arnfjörð Bjarmason wrote:

Show 19 quoted lines

> Let's get some numbers then. On master, go to the "t" directory and run
> this:
> 
>     for f in t[0-9]*.sh; do (echo '#!/bin/sh' && echo "echo ok 1 $f" && echo sleep 1 && echo echo 1..1) >$f; done
>
> That effectively turns all our tests into a "hello world" with a sleep
> of 1 second.
> 
> Then run both:
> 
>     time prove -j12 t00[0-9]*.sh
> 
> And:
> 
>     time make -j12 t00[0-9]*.sh
> 
> For some value of -j12 and t00[0-9]*.sh. In my testing "make" is a bit
> faster, but not by any amount that would matter when this is run for
> real.

Hmm. You're right, I basically see a range of 5.17-5.27 seconds, all well in the noise.

So apart from the size for Perl (which accounts for about a third of the subset of the Git for Windows SDK we have to download into each and every CI run, multiple times) it does not hurt that much at the moment, you're right.

Still, I would like to get away from our reliance on Perl in the test suite. It does not make sense to require Perl even with NO_PERL.

Ciao, Dscho

Derrick Stolee via GitGitGadget· Jan 29, 2019, 17:05 UTC · re: Derrick Stolee via GitGitGadget · lore

[PATCH v2 0/1] Makefile: add prove and coverage-prove targets

If/when this merges down, I will use this new target for the automation around the test coverage reports.

Updates in V2:

 * Dropped the 'prove' target
   
   
 * Append '-j1' to GIT_PROVE_OPTS
   
   
 * Commit message tweaks.

Derrick Stolee (1):
  Makefile: add coverage-prove target

 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

base-commit: 0d0ac3826a3bbb9247e39e12623bbcfdd722f24c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-114%2Fderrickstolee%2Fcoverage-prove-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-114/derrickstolee/coverage-prove-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/114

Range-diff vs v1:

 1:  294187c696 < -:  ---------- Makefile: add prove and coverage-prove targets
 -:  ---------- > 1:  af810fad97 Makefile: add coverage-prove target

-- 
gitgitgadget

Derrick Stolee via GitGitGadget· Jan 29, 2019, 17:05 UTC · re: Derrick Stolee via GitGitGadget · lore

[PATCH v2 1/1] Makefile: add coverage-prove target

From: Derrick Stolee <dstolee@microsoft.com>

Add a new 'coverage-prove' target to the Makefile, modeled after the 'coverage-test' target. This compiles the source using the coverage flags, then runs the test suite using the 'prove' tool. Since the coverage machinery is not thread-safe, enforce that the tests are run in sequence by appending '-j1' to GIT_PROVE_OPTS.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

Show changes to Makefile +5 −0

diff --git a/Makefile b/Makefile
index 1a44c811aa..23d8730482 100644
--- a/Makefile
+++ b/Makefile
@@ -3077,6 +3077,11 @@ coverage-test: coverage-clean-results coverage-compile
 	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
 		DEFAULT_TEST_TARGET=test -j1 test
 
+coverage-prove: coverage-clean-results coverage-compile
+	$(MAKE) CFLAGS="$(COVERAGE_CFLAGS)" LDFLAGS="$(COVERAGE_LDFLAGS)" \
+		DEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS="$(GIT_PROVE_OPTS) -j1" \
+		-j1 test
+
 coverage-report:
 	$(QUIET_GCOV)for dir in $(object_dirs); do \
 		$(GCOV) $(GCOVFLAGS) --object-directory=$$dir $$dir*.c || exit; \

-- 
gitgitgadget