[prev in list] [next in list] [prev in thread] [next in thread] 

List:       perl-qa
Subject:    TPF Devel::Cover grant report December 2012
From:       Paul Johnson <paul () pjcj ! net>
Date:       2013-01-02 15:33:39
Message-ID: 20130102153338.GJ23032 () pjcj ! net
[Download RAW message or body]

In accordance with the terms of my grant from TPF this is the monthly
report for my work on improving Devel::Cover covering December 2012.

This month I released Devel::Cover version 0.99.

I tested against the newly released Perl version 5.17.7.

I added Devel::Cover to 24pullrequests.com and got four pull requests this
month.  Whether or not this is related I can't say, but it was very welcome in
any case.  It's always nice to get patches and bug reports, and I'm very
grateful to everyone who contributes to Devel::Cover.

If you've been following my grant reports, you'll know that I have put up the
site cpancover.com to show coverage reports of CPAN modules.  I have been
hosting the site on bigv.io, which has been in beta for a few months.  bigv.io
is now coming out of beta, and bytemark.co.uk has generously agreed to waive
the costs of the server which hosts cpancover.com.  I added a note of
appreciation, a logo and a link at the bottom of cpancover.com to recognise
and thank Bytemark for this.  Thanks Bytemark!

The number of modules on cpancover is slowly increasing.  Sometimes people ask
me to add modules, or (even better) send me a pull request adding them.  And I
usually remember to add modules which have shown problems or bugs in
Devel::Cover.  If you would like any modules added to cpancover I'm always
happy to do so.

I've also noted that github is the preferred bug tracker for Devel::Cover and
I've made a meta ticket there pointing to open bugs in RT.

I spent some time looking at https://github.com/pjcj/Devel--Cover/issues/38.
This is a bug report from David Cantrell in which there is different behaviour
when a program is run under Devel::Cover to when it is run normally.  To me
this is the most serious type of problem.  Devel::Cover should not alter the
output of the program being covered.  This is more important than missing or
giving incorrect coverage information, or being slow.

From the mail message I wrote to p5p to clarify the situation:

--------------------------------------------------------------------------------
I have narrowed the problem down to the following code:

    my $attributed;
    sub MODIFY_CODE_ATTRIBUTES {
        my ($class, $code, @attrs) = @_;
        $attributed = $code;
        return;
    }
    my $foo = sub :MyAttribute {};
    print "$attributed, $foo\n";

If you run this code normally, you find that $attributed and $foo have
the same value.  This is as it should be since the docs for
attributes.pm say that $code should be a reference to the sub with the
attribute.  (Actually, the docs don't explicitly say that but I think
the intent is clear.)

When run under the debugger you find that $code is actually a different
subroutine reference.  The reason is this little bit of code in
pad.c:1727:

    if (PL_cv_has_eval || PL_perldb) {
        const CV *cv;
        for (cv = PL_compcv ;cv; cv = CvOUTSIDE(cv)) {
            if (cv != PL_compcv && CvCOMPILED(cv))
                break; /* no need to mark already-compiled code */
            if (CvANON(cv)) {
                DEBUG_Xv(PerlIO_printf(Perl_debug_log,
                    "Pad clone on cv=0x%"UVxf"\n", PTR2UV(cv)));
                CvCLONE_on(cv);
            }
            CvHASEVAL_on(cv);
        }
    }

The code is preceded by this comment:

    /* If this CV has had any 'eval-capable' ops planted in it
     * (ie it contains eval '...', //ee, /$var/ or /(?{..})/), Then any
     * anon prototypes in the chain of CVs should be marked as cloneable,
     * so that for example the eval's CV in C<< sub { eval '$x' } >> gets
     * the right CvOUTSIDE.
     * If running with -d, *any* sub may potentially have an eval
     * executed within it.
     */

So, there are a few questions.

First, is it correct that these two subroutine refs should be identical?

If so, should that also hold when running under the debugger?

And if it shouldn't necessarily hold, is that also true for $^P being
set to any value, as is currently the case?

The reason this shows up in Devel::Cover is because $^P is set to 0x104
which, I think, doesn't meet the conditions in the comment.
--------------------------------------------------------------------------------

So the upshot of this one is that I don't think this is a bug in Devel:Cover.
It seems that the expectation that $code should be a reference to the sub with
the attribute is false.  As Dave Mitchell wrote:

] Given that MODIFY_CODE_ATTRIBUTES() is only called once at compile time,
] while sub {} can be called multiple times at runtime, I'm not sure it
] makes sense in general for $attributed to match anything; e.g. in
]
]   my $x;
]   push @foo, sub :MyAttribute {$x} for 1..10;
]
] which (if any) of the elements of @foo would you expect to be equal to
] $attributed?

I also spent some time tracking down a problem that someone had with a
Catalyst application that was giving a Unique Constraint Error in SQLite only
when run with Devel::Cover.  As it happens, that turned out to be exactly the
same problem.  So that's Dancer and Catalyst where this problem is showing up.

Then I looked into the possibility of adding a link on MetaCPAN to a module's
coverage on CPANcover.  I put together a patch to the front-end to see whether
there was general interest, which seems to be the case.  However, some
back-end support is probably needed since CPANcover only has coverage for a
subset of CPAN, and I don't really want to be directing people to 404s.  With
backend support it will also be possible to go directly to the coverage for a
specific file.

I've not made a backend patch since I'm not really sure how to do it.  I'm
hoping someone will offer to either do that, or tell me how to do it.  The
patch is at https://github.com/CPAN-API/metacpan-web/pull/732 should anyone
feel like taking this further.

I decided that it was time to look at some of the old RT tickets that I have
been putting off because they need some concerted effort, and outside of the
grant work I just wasn't able to do that.

I looked into a problem where coverage was not being reported on subroutines
which were removed from the symbol table at run-time.  What was happening was
that a named subroutine was being declared, and a reference to it was taken.
The subroutine was then removed from the symbol table, but called via the
reference that had been taken.  Devel::Cover was not finding the subroutine to
be able to report on its coverage.  It took a while, but I fixed this and
added a test.


The work I have completed in the time covered by this report is:

Closed RT tickets:

  13207 Cover doesnt like some Symbole Table modifications
   8660 Devel::Cover can't handle Test::More thread tests
  24599 Some classes not covered

Closed Github tickets:

Merged pull requests:

  34 Doc tweaks
  37 Add modules to cpancover
  39 Adding my modules

You can see the commits at https://github.com/pjcj/Devel--Cover/commits/master

Hours worked: 18:05

Total hours worked on grant: 200:00

-- 
Paul Johnson - paul@pjcj.net
http://www.pjcj.net
[prev in list] [next in list] [prev in thread] [next in thread]