On Fri, Jan 25, 2013 at 05:47:46PM +0000, Blue Swirl wrote: > On Thu, Jan 24, 2013 at 12:07 PM, Eduardo Habkost <ehabk...@redhat.com> wrote: > > On Wed, Jan 23, 2013 at 08:49:58PM +0100, Andreas Färber wrote: > >> Am 23.01.2013 18:58, schrieb Eduardo Habkost: [...] > >> > diff --git a/tests/Makefile b/tests/Makefile > >> > index d86e95a..4b98d4f 100644 > >> > --- a/tests/Makefile > >> > +++ b/tests/Makefile > >> > @@ -45,6 +45,10 @@ gcov-files-test-aio-$(CONFIG_WIN32) = aio-win32.c > >> > gcov-files-test-aio-$(CONFIG_POSIX) = aio-posix.c > >> > check-unit-y += tests/test-thread-pool$(EXESUF) > >> > gcov-files-test-thread-pool-y = thread-pool.c > >> > +check-unit-y += tests/test-x86-cpuid$(EXESUF) > >> > +# all code tested by test-x86-cpuid is inside topology.h, > >> > +# so add the test file itself to the gcov list > >> > +gcov-files-test-x86-cpuid-y = tests/test-x86-cpuid.c > >> > > >> > check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh > >> > > >> > >> With patch 7/9 dropped I am more comfortable with the test integration. > >> > >> I wonder however whether the gcov line is correct - won't this screw up > >> the statistics so that it's better to drop that line and to add > >> hw/pc_piix.c or target-i386/cpu.c in 9/9 instead? Blue? > > > > I want to make gcov check for coverage only of topology.h (that's where the > > tested code lives). Including test-x86-cpuid.c is the closest I could get to > > that[1]. Including pc_piix.c or cpu.c would surely screw up the numbers, as > > the > > tests don't cover any of the pc_piix.c or target-i386/cpu.c code. > > In my first version of the gcov patch, the statistics were gathered > for all tests, but that did not work well since there were double > counts due to coverage from an emulator run and standalone test case > runs.
I see. Thanks for the explanation. > > > > [1] If I set gcov-files-test-x86-cpuid-y = target-i386/topology.h, I get: > > > > GTESTER tests/test-x86-cpuid > > Gcov report for target-i386/topology.h: > > target-i386/topology.gcno:cannot open graph file > > > > It looks like the .gcno file generation is per-object-file, not > > per-source-file > > (gcov-files-*-y being a list of .c files confused me). If that's the case, > > then > > the only valid value for gcov-files-test-x86-cpuid-y is really > > tests/test-x86-cpuid.c, because all the tested code is being compiled inside > > tests/test-x86-cpuid.o. > > But we are not interested in the coverage of the test suite code. > > Perhaps the design of gcov is not so well thought out and it is not > very suitable for this kind of files. I agree it is not perfect, but including test-x86-cpuid.c in the list is the only way to get a report of the topology.h test coverage. Do you see any reason to not do it? -- Eduardo
