On Tue, Jan 29, 2019 at 11:35:41AM -0500, Derrick Stolee wrote: > > 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). > >> +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. > > 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
