This looks useful. Thank you.
On Thu, Mar 01, 2012 at 04:30:35PM -0800, Ethan Jackson wrote:
> Adds support for Ned Batchelder's code coverage tool to the
> test suite. http://nedbatchelder.com/code/coverage/
>
> Signed-off-by: Ethan Jackson <et...@nicira.com>
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?
> @@ -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
