gap icon indicating copy to clipboard operation
gap copied to clipboard

Inconsistency in code coverage information depending on the number of statements in an if-block

Open zickgraf opened this issue 2 years ago • 2 comments

This might be a dupe of #3841, but I'm not sure. Feel free to close this issue if it is a dupe.

Steps to reproduce

Put the following piece of code in a file:

func := function ( x )
    
    if x = 2 then
        
        Display( "Test1" );
        Display( "Test2" );
        
    else
        
        Display( "else" );
        
    fi;
    
end;

func(1);
  1. Execute the file with GAP including coverage information.

Observed behavior

GAP marks the else as uncovered. If Display( "Test2" ); is commented, else is not marked at all.

Expected behavior

Consistent behavior, i.e. else is handled in the same way in both cases.

Copy and paste GAP banner (to tell us about your setup)

 ┌───────┐   GAP 4.12dev-1407-ge23c6b9 built on 2022-08-08 14:45:44+0200
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-pc-linux-gnu-default64-kv8
 Configuration:  gmp 6.2.1, GASMAN, readline
 Loading the library and packages ...
 Packages:   GAPDoc 1.dev, IO 4.7.0dev, PrimGrp 3.4.0, SmallGrp 1.4.1, TransGrp 2.0.5
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'

zickgraf avatar Aug 17 '22 09:08 zickgraf

This is a seperate issue.

I am aware of what the overall problem is, I'm pondering the best way to fix it.

The else being highlighted ever is a bit misleading -- what's actually going on is your multiple statements are being put in a "sequence of statements" (called a SEQ_STAT internally), and the location of that SEQ_STAT ends up being the line with the else (as that's when GAP realises it's reached the end of the sequence). That's why the else is only highlighted green when the condition is true.

The best option might be to stop highlighting these SEQ_STAT at all -- that would lead to few lines marked as executed, but those lines of code were never really being "executed" anyway.

ChrisJefferson avatar Aug 17 '22 10:08 ChrisJefferson

Thanks for the fast investigation!

From an outside point of view (i.e. without knowing about all the internal quirks involved :D ), your proposed solution sounds very reasonable. At least I prefer a one-time inconsistency because of some fewer lines covered over a continuous inconsistency at various places of our code.

zickgraf avatar Aug 17 '22 14:08 zickgraf