On Tue, Mar 26, 2013 at 12:07 AM, Diego Biurrun <[email protected]> wrote:
> On Mon, Mar 25, 2013 at 08:37:20PM +0100, Reinhard Tartler wrote:
>> The gcov/lcov are a common toolchain for visualizing code coverage with
>> the GNU/Toolchain. The documentation and implementation of this
>> integration was heavily inspired from the blog entry by Mike Melanson:
>> http://multimedia.cx/eggs/using-lcov-with-ffmpeg/
>
> Why do you mention gcov when you are using lcov?
>
>> ---
>> .gitignore | 4 ++++
>> Makefile | 1 +
>> common.mak | 2 +-
>> configure | 4 ++++
>> doc/developer.texi | 22 ++++++++++++++++++++++
>> tests/Makefile | 9 ++++++++-
>> 6 files changed, 40 insertions(+), 2 deletions(-)
>>
>> the IMMEDIATE: "declaration" causes the coverage.info file to be deleted
>> immediately. Otherwise, the developer would have to delete it by hand
>> every time he runs another test in order to get updated results with
>> 'make lcov'. I count this as a huge usability gain.
>
> Huh, you meant INTERMEDIATE, I was doubting what you meant for a second :)
Yeah, luckily that typo is not part of the commit.
> What you describe sounds like a missing dependency declaration. Indeed
> the list of prerequisites for coverage.info is empty. What is the
> behavior you are looking for?
I don't think there is anything to improve here we are talking here
about an convenience target so that the developer does not have to
remember the exact lcov imposed workflow. The general idea is this:
a) you instrument the complete libav codebase, including the avconv
binary, with profiling probes. This is done by the magic GCC compiler
flags.
b) you run your workload. This may be 'make fate' on fate machines,
but I expect the regular developer to call avconv by hand on some more
or less random samples in his shell.
c) this invocation will create some gcda files, which contain the data
produced by the implanted probes in the code from step a)
d) lcov scans the build tree for these gcda files, and distills them
into a more or less compact format "coverage.info". This file can be
consumed by a number of front-ends, for instance, there is a jenkins
plugin that can consume it, but in this patch, I propose to use the
lcov tool, which produces HTML.
Having said this, it does make sense to provide the coverage.info file
as makefile target, because you might want to have it for other uses
than lcov. For instance, I plan to play with a jenkins job that
produces a nicer HTML output and statistics over time, etc..
These pieces of information, however, go into too much detail for
inclusion in the documentation, which is supposed to be precise and to
the point AFAIUI.
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -30,6 +33,7 @@
>> /doc/avoptions_format.texi
>> /doc/doxy/html/
>> /doc/print_options
>> +/lcov
>
> Is that a directory? End in a / then.
Fixed locally.
>
>> --- a/doc/developer.texi
>> +++ b/doc/developer.texi
>> @@ -550,6 +550,28 @@ why the expected result changed.
>>
>> +@subsection Visualizing Test Coverage
>> +
>> +The Libav build system provides means for visualizing the test coverage
>> +leveraging the coverage tools @code{gcov}/@code{lcov} in an easy manner.
>
> I'd say s/in an easy manner//, that's for the users to judge. But if
> you prefer, feel free to keep it.
I'd prefer to keep it. your questions indicate that gcov/lcov is not
exactly straight-forward to use, so that side-note seems warranted to
me.
>> +This involves the following steps:
>> +
>> +@enumerate
>> +@item
>> + Configure to compile with instrumentation enabled:
>> + @code{configure --toolchain=gcov}.
>
> Use 4 spaces as indentation. I know that there is a list in that file
> that uses only 3, but let's not add more weirdness.
fixed locally.
Can you propose a patch that normalizes the indentation for the
texinfo files to avoid such copy mistakes in the future, please?
>> +@item
>> + Run your test case, either manually or via FATE.
>
> It's not totally clear to me what you mean; "make fate" and possibly
> an example would be useful IMO.
That's deliberately kept vague as the developer can really do what he
wants here. I've explained the situation above, feel free to propose a
better wording.
>> +@item
>> + Run @code{make lcov} to generate coverage data in HTML format
>
> .
>
> End the sentence in a period.
fixed locally.
>
>> +@item
>> + View @code{lcov/index.html} in your preferred HTML viewer.
>> +@end enumerate
>> +
>> +You can use the command @code{lcov --directory . --zerocounters} to
>> +reset the coverage measurements. You will need to rerun @code{make lcov}
>> +after running a new test.
>
> That might be done with a separate clean target, but let's first hear
> about the dependency issue.
I can certainly add such a target:
reset-lcov:
lcov -d $(CURDIR) --zerocounters
(and of course adding it to PHONY)
It feels a bit overdone to me, though. What do you think?
>
>> --- a/tests/Makefile
>> +++ b/tests/Makefile
>> @@ -123,6 +123,12 @@ $(FATE): $(FATE_UTILS:%=tests/%$(HOSTEXESUF))
>>
>> +coverage.info:
>> + lcov -d $(CURDIR) -b $(SRC_PATH) --capture -o $@
>> +
>> +lcov: coverage.info
>> + genhtml -o $(CURDIR)/lcov $<
>
> What's the output for these commands? I guess we could create a proper
> tag to have pretty and terse output.
It is similar verbose and useless like the output of doxygen.
Beautifying the invocation seems unhelpful to me.
--
regards,
Reinhard
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
