On 1/29/2019 11:00 AM, Jeff King wrote: > On Tue, Jan 29, 2019 at 06:56:08AM -0800, Derrick Stolee via GitGitGadget wrote: > >> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> >> >> 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? >> @@ -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
