pjcj / Devel--Cover

Code coverage metrics for Perl
http://www.pjcj.net/perl.html
93 stars 89 forks source link

Code outside named subroutines is invisible to Devel::Cover #51

Open DrHyde opened 11 years ago

DrHyde commented 11 years ago

For example, if you generate a coverage report for CPU::Emulator::Z80 (there's a t/coverage.sh script; see git@github.com:DrHyde/perl-modules-CPU-Emulator-Z80.git) and look at cover_db/blib-lib-CPU-Emulator-Z80-pm.html, it doesn't say anything at all for lines like this:

10: $VERSION = '1.0';

...

32: my @REGISTERS8 = qw(A B C D E F R W Z I);

although they obviously got compiled - and in this case I know that they got executed too.

From looking at various other examples, eg http://lists.preshweb.co.uk/pipermail/dancer-users/2011-October/001990.html, it appears that the general rule is that code gets reported on if it is within a named subroutine. Anonymous subs' code appear in reports if the anonysub is defined within a named subroutine (see eg CPU::Emulator::Z80 line 209) but not if it is defined outside any named sub (see eg line 7 in TestApp/lib/TestApp.pm as generated by Michael Dorman in his email to dancer-users).

jkeenan commented 11 years ago

This is not a new issue. Devel::Cover has never, in my nine years' use of it, provided data on anonymous subs. I suspect that if pjcj hasn't done that already, it's because it's very hard.

My feeling is that what we should do in the short-run is simply to document the fact than data is provided only on named subroutines.

Thank you very much. Jim Keenan

pjcj commented 11 years ago

You're quite correct that this is rather tricky, but I have been making some progress on it recently and I have a local branch for it. But documentation of the limitiations is good.

It's not that only named subroutines are covered but that within a module, only code within named subroutines can be reported on.

A workaround is to put all code in a module into a named subroutine and call it.

DrHyde commented 11 years ago

For the particular situation at work that prompted me to raise this issue, I've got a work-around that does exactly that. It looks to see if Devel::Cover is loaded, and if it is, it puts a sub-ref in @INC so that the affected files get wrapped in:

sub main { ... } main();

which is EVIL. When your branch is ready for real-world testing, do please let me know, because I'd love to delete my hideous voodoo code.

jkeenan commented 11 years ago

Today I made my first attempt at $job to use Devel::Cover. The package whose coverage I was most concerned with is a Moose/Catalyst class in which the particular code I was concerned with was inside a Moose around method modifier. Pseudo-code:

around 'my_hook' => sub {
  my $first = shift;
  my $self = shift;
  my ($c, $form) = @_;
### lots of code
};

I got no coverage of this, notwithstanding the fact that manual debugging indicates the code is hit hundreds of times.

Is this a particular instance of the "only visible in named subroutines" problem?

Thank you very much. Jim Keenan

DrHyde commented 11 years ago

Yup. I expect that this will fix it ...

sub _my_hook { my($first, $self, $c, $form) = @_; ... }
around my_hook => \&_my_hook;

or you could adapt my nasty (and potentially a bit fragile) solution, which looks something like this ...

(code updated on 2020-03-04 to be a lot more reliable)

    unshift @INC, sub {
        my(undef, $filename) = @_;

        if(my $found = (grep { -e $_ } map { "$_/$filename" } grep { !ref } @INC)[0]) {
            local $/ = undef;
            open(my $fh, '<', $found) || die("Can't read module file $found\n");
            my $module_text = <$fh>;
            close($fh);

            # define everything in a sub, so Devel::Cover will DTRT
            # NB this introduces minimal extra line feeds to avoid any
            # horrible confusion between what's on disk and in memory.
            # The two linefeeds are to avoid putting } after a closing
            # =cut and to make sure __DATA__ or __END__ is at the start
            # of a line
            $module_text =~ s/
                (.*?package\s+\S+;)      # package declaration
                (.*?)                    # body
                ((__END__|__DATA__).*|$) # trailer
            /$1sub main {$2\n};main();\n$3/sx;

            # filehandle on the scalar
            open ($fh, '<', \$module_text);

            # and put it into %INC too so that it looks like we loaded the code
            # from the file directly
            $INC{$filename} = $found;
            return $fh;
        } else {
            return ();
        }
    };
jkeenan commented 11 years ago

Dr. Hyde, your workaround worked for me as well. By making the Moose method modifier a thin wrapper around a named subroutine, I was able to get coverage data on that named subroutine. That was my practical objective and it confirmed the results I got by temporarily stuffing the code with say STDERR statements.

Let's hope that we can get coverage of anonymous subroutines into Devel::Cover so that we don't have to resort to such workarounds. A colleague notes that much event-loop-based programming relies heavily on closures, so that programming would also benefit from an expansion of Devel::Cover's capabilities.

The issue remains open.

dolmen commented 11 years ago

@jkeenan You can also use Sub::Name to give a name to your subroutine:

use Sub::Name;

around my_hook => subname _my_hook {
  my $first = shift;
    ...
};
bor commented 8 years ago

Role::Tiny (Class::Method::Modifiers under-hood) also is affected.

Guys, thanks for sharing workarounds!

dolmen commented 8 years ago

See also Sub::Util::set_subname as an alternative to Sub::Name.

balajirama commented 5 years ago

@pjcj @DrHyde @jkeenan etc., if you write a CPAN class using Moose, you might be interested to know that there is a CPAN module named MooseX::CoverableModifiers that can help with this issue. I was able to get the code to cover all unnamed subroutine references in around, after, and before.

But there is one style of code that would still not be covered properly:

my %callback_function = (
  opt1 => sub {
    ## do something
  },
  opt2 => sub {
    ## something else
  },
);

sub run_funcs {
  my $callback = shift;
  $callback_function{$callback}->()
}
pjcj commented 5 years ago

After extensive research by Aaron Crane, it is all but impossible to implement this without core support. So we're going to go for core support.

balajirama commented 5 years ago

Is that coming in Perl 5?

pjcj commented 5 years ago

With luck, it will be in Perl 5.32 in about a year or so

DrHyde commented 5 years ago

That's good news, thanks!

mohawk2 commented 3 years ago

Did this land?

mat813 commented 3 years ago

I tried MooseX::CoverableModifiers and it completely exploded with Perl 5.32. I found out that, on the other hand, replacing before 'update' => sub { ... with before 'update' => *before_update = sub { ... had a similar effect, and it was a nice workaround.

mohawk2 commented 3 years ago

Nice! @mat813 @pjcj Do you think a doc patch would be a good idea?

sirtoobii commented 2 years ago

Just FYI: I wrapped the code from @DrHyde in a perl module: https://metacpan.org/pod/Devel::Deanonymize. Feedback is highly appreciated