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

From: Jeff King <peff@peff.net>
Date: Jan 29, 2019, 16:00 UTC
Message-ID: <20190129160030.GA7083@sigill.intra.peff.net>
In-Reply-To: <294187c6968eff952e78bcea808c66fbedbf1f90.1548773766.git.gitgitgadget@gmail.com>

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