Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to ignore blocks of an uncoverable condition? #212

Open
FROGGS opened this issue Jun 25, 2018 · 6 comments
Open

How to ignore blocks of an uncoverable condition? #212

FROGGS opened this issue Jun 25, 2018 · 6 comments

Comments

@FROGGS
Copy link

FROGGS commented Jun 25, 2018

How do I let Devel::Cover ignore all three statements in the block?

if (!$ENV{TEST_ACTIVE}) {
    do_something_that_doesnt_work_in_tests();
    again_something();
    ohh_noes();
}

I tried everything that comes to my mind. But how to do it?

@FROGGS
Copy link
Author

FROGGS commented Jul 2, 2018

To be clear, TEST_ACTIVE is always set, so the condition is never true and the block is never executed when coverage analysis is running.

@pjcj
Copy link
Owner

pjcj commented Jul 9, 2018

As discussed on IRC, there is no way to do this currently. But it would be a nice feature.

I wonder whether marking the first statement in a block as uncoverable should automatically mark all statements and other criteria in the block as also uncoverable. Or is that perhaps too much magic? After all, it is possible to jump into the middle of a block with a goto.

Perhaps marking the first statement in a block as uncoverable using the same syntax as for a statement but specifying a block instead would be better. This would leave all options open and would be less magic.

I suspect there is a way that we can hook into the optree to determine where blocks start and end. And in that case we can perhaps fake up the uncoverable statement calls from that. And the uncoverable calls for the other criteria.

Relatedly, We should probably add the uncoverable calls for branches when the statement is uncoverable. And the calls for conditions when the branch is uncoverable. And the blocks when the sub is uncoverable.

I think that would all be a lot nicer than what we have now.

And if you'd like to have a bash at implementing that I'd be happy to offer any help I could.

Thanks a lot for your report!

@pjcj
Copy link
Owner

pjcj commented Jul 9, 2018

See #44

@FROGGS
Copy link
Author

FROGGS commented Jul 9, 2018

I'd also prefer to have statements like uncoverable block and uncoverable subroutine.
After all it reads nicer and the reader knows what should happen. (Not so with an uncoverable statement as the first statement of a series.)

I'd like to tackle it as soon as I've time, though I cannot say when this will happen.

@FROGGS
Copy link
Author

FROGGS commented Jul 9, 2018

Hmmm, I skimmed the code in order to gain some information, but I'm not sure anymore that I'll be of help at all...

sub add_branch {
    my $self = shift;
    my ($cc, $sc, $fc, $uc) = @_;
    my %line;
    for my $i (0 .. $#$fc) {
        my $l = $sc->[$i][0];
        unless (defined $l) {
            warn "Devel::Cover: ignoring extra branch\n";
            return;
        }
        my $n = $line{$l}++;
        if (my $a = $cc->{$l}[$n]) {
            no warnings "uninitialized";
            $a->[0][0] += $fc->[$i][0];
            $a->[0][1] += $fc->[$i][1];
            $a->[0][2] += $fc->[$i][2] if exists $fc->[$i][2];
            $a->[0][3] += $fc->[$i][3] if exists $fc->[$i][3];
        } else {
            $cc->{$l}[$n] = [ $fc->[$i], $sc->[$i][1] ];
        }
        $cc->{$l}[$n][2][$_->[0]] ||= $_->[1] for @{$uc->{$l}[$n]};
    }
}

$cc, $sc, $fc, $uc, $l, $n, $a? Really? There are no comments, no speaking variable names and magic numbers all over the place. I'd need a lot more time than I had expected to get the basics of this Module. :/

@pjcj
Copy link
Owner

pjcj commented Jul 13, 2018

Ah, I did mention that your experience with the code might be variable and I think you've managed to pick perhaps the most opaque subroutine there is ;)

This sub is responsible taking the raw coverage data collected during the run and converting the data for all the different types of branch and all the different types of condition and storing it in the coverage DB. All the add_* methods have the same signature, which is described where the methods are called:

                # $cc - coverage being filled in
                # $sc - structure information
                # $fc - coverage from this file
                # $uc - uncoverable information

Then $l is a line, $n is for the nth instance of the criterion on the line, $i is a loop index, and $s is just a temp. The magic numbers are the indices into the raw branch and condition data and mean different things depending on the type of branch or loop.

All that being said, I don't disagree with you and it would be nice if it were clearer. However, for solving this particular problem I'm not sure that understanding the format in which the data is collected, the format in which it is stored, and the methods which perform that conversion is an absolute necessity. It wouldn't hurt, for sure, but there's probably other difficult code to be understood instead ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants