[ovs-dev] [unixctl_py 3/6] tests: Add code coverage for Python.

Ethan Jackson ethan at nicira.com
Fri Mar 2 01:20:20 UTC 2012


> 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



More information about the dev mailing list