> It seems odd to run coverage on the "check-structs" build tool. I > guess it's actually harder to avoid it than to run it?
Exactly, there isn't a good way to disable. The coverage doesn't add a noticeable amount of time to the test sweet so I don't think it's worth it. I'll send out an incremental addressing the rest of your comments. Ethan >> @@ -115,6 +116,11 @@ SUFFIXES += .in >> fi >> mv $@.tmp $@ >> >> +.PHONY: clean-pycov >> +clean-pycov: >> + cd $(abs_top_srcdir) && rm -f $(PYCOV_CLEAN_FILES) > > It's generally better to use the non-"abs" versions when you can, > since they are generally shorter (usually just . or ..) and more > obvious and avoid issues with word splitting in the shell and funny > characters (/home/Ben's Home Directory/Open vSwitch, anyone?). Plus, > you don't need "top" because this is the top level anyway. > > So: > cd '$(srcdir)' && rm -f $(PYCOV_CLEAN_FILES) > > (I see that I'm guilty of some uses of abs_ and top_ where they're not > needed. Shame on me.) > > This doesn't make sense, I guess it's my fault from our in-person > discussion: >> +if test x"$COVERAGE_FILE" = x; then >> + export COVERAGE_FILE >> +fi > >> diff --git a/tests/automake.mk b/tests/automake.mk >> index b133467..387d9cd 100644 >> --- a/tests/automake.mk >> +++ b/tests/automake.mk >> @@ -66,6 +66,21 @@ AUTOTEST_PATH = utilities:vswitchd:ovsdb:tests >> check-local: tests/atconfig tests/atlocal $(TESTSUITE) >> $(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH) >> $(TESTSUITEFLAGS) >> >> +# Python Coverage support. >> +# Requires coverage.py http://nedbatchelder.com/code/coverage/. >> + >> +COVERAGE = coverage >> +COVERAGE_FILE=$(abs_top_srcdir)/.coverage >> +check-pycov: all tests/atconfig tests/atlocal $(TESTSUITE) clean-pycov > > Probably a good idea to put single-quotes around $(COVERAGE_FILE) here: > >> + COVERAGE_FILE=$(COVERAGE_FILE) PYTHON='$(COVERAGE) run -p' $(SHELL) >> '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH) $(TESTSUITEFLAGS) > > Should you s/coverage/$(COVERAGE)/ in three places below? > Also s/$(abs_top_srcdir)/'$(srcdir)'/ as above. > >> + @cd $(abs_top_srcdir) && coverage combine && >> COVERAGE_FILE=$(COVERAGE_FILE) coverage annotate >> + @echo >> + @echo >> '----------------------------------------------------------------------' >> + @echo 'Annotated coverage source has the ",cover" extension.' >> + @echo >> '----------------------------------------------------------------------' >> + @echo >> + @COVERAGE_FILE=$(COVERAGE_FILE) coverage report _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
