Additional error checking: callback scripts, data inconsistency

Signed-off-by: Henry Cox <[email protected]>

Author: henry2cox

bin/genhtml

@@ -2642,8 +2642,25 @@ sub _categorizeLineCov
         my $linedata = $lineDataMap->{$line};
         my $tla;
         if ($linedata->type() eq "insert") {
+            if (!$linedata->in_curr()) {
+                # can get here if the 'diff' file is wrong with respect to
+                # baseline vs. current coverage data - e.g., showing that
+                # an unchanged line has a difference
+                lcovutil::ignorable_error($lcovutil::ERROR_INCONSISTENT_DATA,
+                    "$filename:$line: 'diff' data claims this line is inserted but it is not in 'current' coverage data"
+                );
+                next;
+            }
             $tla = ($linedata->curr_count() > 0) ? "GNC" : "UNC";
         } elsif ($linedata->type() eq "delete") {
+            if (!$linedata->in_base()) {
+                # similarly:  can get here if the diff vs baseline/current data
+                # is inconsistent.
+                lcovutil::ignorable_error($lcovutil::ERROR_INCONSISTENT_DATA,
+                    "$filename:$line: 'diff' data claims this line is deleted but it is not in 'baseline' coverage data"
+                );
+                next;
+            }
             $tla = ($linedata->base_count() > 0) ? "DCB" : "DUB";
         } else {
             $linedata->type() eq "equal" or
@@ -2700,8 +2717,8 @@ sub _categorizeBranchCov
         my $data;
         if (!exists($lineDataMap->{$line})) {
             # we expect the associated line to also have line coverage data
-            lcovutil::ignorable_error($lcovutil::ERROR_UNKNOWN_CATEGORY,
-                   "line $line of $filename has branchcov but no linecov data");
+            lcovutil::ignorable_error($lcovutil::ERROR_INCONSISTENT_DATA,
+                      "line $filename:$line has branchcov but no linecov data");
             # maybe should skip this branch if no line data - rather
             #  than building fake data.
             $data = LineData->new($type);
@@ -2746,7 +2763,8 @@ sub _categorizeBranchCov
         my @baseBlocks = defined($base) ? $base->blocks() : ();
 
         if ($type eq 'insert') {
-            lcovutil::ignorable_error($lcovutil::ERROR_INTERNAL,
+            # can get here if diff data vs baseline/current is not consistent
+            lcovutil::ignorable_error($lcovutil::ERROR_INCONSISTENT_DATA,
                 "baseline branch data should not be defined for inserted line $filename:$line"
             ) if defined($base);
 
@@ -2759,7 +2777,8 @@ sub _categorizeBranchCov
                 }
             }
         } elsif ($type eq 'delete') {
-            lcovutil::ignorable_error($lcovutil::ERROR_INTERNAL,
+            # similarly: get here if diff data vs baseline/current is not consistent
+            lcovutil::ignorable_error($lcovutil::ERROR_INCONSISTENT_DATA,
                 "current branch data should not be defined for deleted line $filename:$line"
             ) if defined($curr);
 
@@ -2916,8 +2935,8 @@ sub _categorizeFunctionCov
         if (!defined($base)) {
             # either this line was inserted or the line hasn't changed but
             #   wasn't recognized as a function before (e.g., unused template)
-            lcovutil::ignorable_error($lcovutil::ERROR_INTERNAL,
-                "unexpected undef baseline function data for deleted $filename:$line"
+            lcovutil::ignorable_error($lcovutil::ERROR_INCONSISTENT_DATA,
+                "$filename:$line: unexpected undef baseline function data for deleted $name"
             ) if $type eq 'delete';
             my $hit = $curr->hit();
             my $tla = ((0 == $hit) ? ($type eq 'insert' ? 'UNC' : 'UIC') :
@@ -2931,8 +2950,8 @@ sub _categorizeFunctionCov
                 $categorized->addAliasDifferential($alias, [$hit, $tla]);
             }
         } elsif (!defined($curr)) {
-            lcovutil::ignorable_error($lcovutil::ERROR_INTERNAL,
-                "unexpected undef current function data for inserted $filename:$line"
+            lcovutil::ignorable_error($lcovutil::ERROR_INCONSISTENT_DATA,
+                "$filename:$line: unexpected undef current function data for inserted $name"
             ) if $type eq 'insert';
             my $hit = $base->hit();
             my $tla = ((0 == $hit) ? ($type eq 'delete' ? 'DUB' : 'EUB') :

man/genhtml.1

@@ -2519,7 +2519,10 @@ Unexpected syntax found in .info file.
 .IP inconsistent: 3
 Files have been moved or repository history presented by
 .B \-\-diff\-file
-data is not consistent with coverage data; for example, an 'inserted' line has baseline coverage data.  These issues are likely to be caused by inconsistent handling in the 'diff' and 'annotate' scripts.
+data is not consistent with coverage data; for example, an 'inserted' line has baseline coverage data.  These issues are likely to be caused by inconsistent handling in the 'diff' data compared to the 'baseline' and 'current' coverage data (e.g., using different source versions to collect the data but incorrectly annotating those differences), or by inconsistent treatment in the 'annotate' script.
+Consider using a
+.B \-\-version\-script
+to guard against version mismatches.
 .PP
 
 .IP internal: 3

scripts/criteria

@@ -47,7 +47,8 @@ use JSON;
 my $obj = criteria->new($0, @ARGV);
 
 my $signoff;
-if (!GetOptions('signoff' => \$signoff)) {
+if (!defined($obj) ||
+    !GetOptions('signoff' => \$signoff)) {
     print(STDERR "usage: name type json-string [--signoff]\n");
     exit(1) if caller;
     return undef;

scripts/criteria.pm

@@ -49,13 +49,18 @@ use constant {SIGNOFF => 0,};
 
 sub new
 {
-    my $class   = shift;
-    my $signoff = 0;
-    my $script  = shift;
+    my $class      = shift;
+    my $signoff    = 0;
+    my $script     = shift;
+    my $standalone = $script eq $0;
+    my @options    = @_;
 
-    if (!GetOptionsFromArray(\@_, ('signoff' => \$signoff))) {
-        print(STDERR "usage: name type json-string [--signoff]\n");
-        exit(1) if ($script eq $0);
+    if (!GetOptionsFromArray(\@_, ('signoff' => \$signoff)) ||
+        (!$standalone && @_)) {
+        print(STDERR "Error: unexpected option:\n  " .
+              join(' ', @options) .
+              "\nusage: name type json-string [--signoff]\n");
+        exit(1) if $standalone;
         return undef;
     }
 

scripts/gitblame.pm

@@ -63,18 +63,20 @@ sub new
     my $prefix;
     my @args = @_;
     my @abbrev;
-    my $exe = basename($script ? $script : $0);
+    my $exe        = basename($script ? $script : $0);
+    my $standalone = $script eq $0;
     my $help;
     if (!GetOptionsFromArray(\@_,
                              ("p4"       => \$mapP4,
                               "prefix:s" => \$prefix,
                               'abbrev:s' => \@abbrev,
                               'help'     => \$help)) ||
+        (scalar(@_) >= 2) ||
         $help
     ) {
         print(STDERR
               "usage: $exe [--p4] [--abbrev regexp]* [domain] pathname\n");
-        exit($help ? 0 : 1) if ($script eq $0);
+        exit(scalar(@_) >= 2 && $help ? 0 : 1) if $standalone;
         return undef;
     }
     my $internal_domain = shift;

scripts/p4annotate.pm

@@ -69,8 +69,9 @@ sub new
     my $script = shift;    # this should be 'me'
                            #other arguments are as passed...
     my $logfile;
-    my $verify = 0;    # if set, check that we merged local changes correctly
-    my $exe    = basename($script ? $script : $0);
+    my $verify     = 0;   # if set, check that we merged local changes correctly
+    my $exe        = basename($script ? $script : $0);
+    my $standalone = $0 eq $script;
 
     if (exists($ENV{LOG_P4ANNOTATE})) {
         $logfile = $ENV{LOG_P4ANNOTATE};
@@ -80,10 +81,11 @@ sub new
                              ("verify" => \$verify,
                               "log=s"  => \$logfile,
                               'help'   => \$help)) ||
+        (!$standalone && scalar(@_)) ||
         $help
     ) {
         print(STDERR "usage: $exe [--log logfile] [--verify] filename\n");
-        exit($help ? 0 : 1) if $script eq $0;
+        exit(scalar(@_) == 0 && $help ? 0 : 1) if $standalone;
         return undef;
     }
 

scripts/select.pm

@@ -73,8 +73,9 @@ sub new
     my $script = shift;
 
     my (@range, @tla, @owner, @sha);
-    my @args = @_;
-    my $exe  = basename($script ? $script : $0);
+    my @args       = @_;
+    my $exe        = basename($script ? $script : $0);
+    my $standalone = $script eq $0;
     my $help;
     if (!GetOptionsFromArray(\@_,
                              ("range:s"  => \@range,
@@ -83,6 +84,7 @@ sub new
                               'sha|cl:s' => \@sha,
                               'help'     => \$help)) ||
         $help ||
+        (!$standalone && 0 != scalar(@_)) ||
         0 == scalar(@args)    # expect at least one selection  criteria
     ) {
         print(STDERR <<EOF);
@@ -96,7 +98,7 @@ usage: $exe
 Line is selected (return true) if any of the criteria match
 EOF
 
-        exit($help ? 0 : 1) if ($script eq $0);
+        exit($help && 0 == scalar(@_) ? 0 : 1) if $standalone;
         return undef;
     }
     # precompile:
@@ -137,29 +139,23 @@ sub select
         # for a contiguous region of context lines (e.g., which are part
         # of some SHA) which are not code and thus have no data
         my $tla = $lineData->tla();
-        return 1 if grep(/$tla/, @{$self->[TLA]});
+        return 1 if grep({ $tla eq $_ } @{$self->[TLA]});
     }
 
     if (defined($annotateData)) {
         my $age = $annotateData->age();
-        foreach my $a (@{$self->[AGE]}) {
-            return 1
-                if ($age >= $a->[0] &&
-                    $age <= $a->[1]);
-        }
-        my $commit = $annotateData->commit();
-        if (defined($commit) &&
-            '' ne $commit) {
+        return 1
+            if grep({ $age >= $_->[0] && $age <= $_->[1] } @{$self->[AGE]});
 
-            foreach my $sha (@{$self->[SHA]}) {
-                # match at head of commit ID string
-                return 1 if $commit =~ /^$sha/;
-            }
-        }
+        my $commit = $annotateData->commit();
+        # match at head of commit ID string
+        return 1
+            if (defined($commit) &&
+                '' ne $commit &&
+                grep({ $commit =~ /^$_/ } @{$self->[SHA]}));
 
-        foreach my $re (@{$self->[OWNER]}) {
-            return 1 if $annotateData->full_name() =~ $re;
-        }
+        my $fullname = $annotateData->full_name();
+        return 1 if grep({ $fullname =~ $_ } @{$self->[OWNER]});
     }
     lcovutil::info(1,
                    "drop "

tests/gendiffcov/errs/msgtest.sh

@@ -112,6 +112,8 @@ GET_VERSION=$SCRIPTS_DIR/getp4version
 SELECT_SCRIPT=$SCRIPTS_DIR/select.pm
 CRITERIA_SCRIPT=$SCRIPTS_DIR/criteria.pm
 ANNOTATE_SCRIPT=$SCRIPTS_DIR/p4annotate.pm
+GITBLAME_SCRIPT=$SCRIPTS_DIR/gitblame.pm
+GITVERSION_SCRIPT=$SCRIPTS_DIR/gitversion.pm
 
 
 # filter out the compiler-generated _GLOBAL__sub_... symbol
@@ -120,7 +122,7 @@ LCOV_OPTS="$LCOV_BASE"
 DIFFCOV_OPTS="--filter line,branch,function --function-coverage --branch-coverage --highlight --demangle-cpp --prefix $PARENT_VERSION $PROFILE "
 
 rm -f test.cpp *.gcno *.gcda a.out *.info *.log *.json diff.txt
-rm -rf select criteria annotate empty unused_src
+rm -rf select criteria annotate empty unused_src scriptErr scriptFixed
 
 if [ "x$COVER" != 'x' ] && [ 0 != $LOCAL_COVERAGE ] ; then
     cover -delete
@@ -315,9 +317,13 @@ if [ 0 != $? ] ; then
     fi
 fi
 
-for arg in "--select-script $SELECT_SCRIPT" \
-               "--criteria-script $CRITERIA_SCRIPT" \
+for arg in "--select-script $SELECT_SCRIPT,--range,0:10" \
+               "--criteria-script $CRITERIA_SCRIPT,--signoff" \
                "--annotate-script $ANNOTATE_SCRIPT" \
+               "--annotate-script $GITBLAME_SCRIPT,mediatek.com,--p4" \
+               "--annotate-script $GITBLAME_SCRIPT,--p4" \
+               "--annotate-script $GITBLAME_SCRIPT" \
+               " --ignore version --version-script $GITVERSION_SCRIPT,--md5,--p4" \
            ; do
     echo genhtml $DIFCOV_OPTS initial.info -o scriptErr ${arg},-x
     $COVER $GENHTML_TOOL $DIFFCOV_OPTS initial.info -o scriptErr ${arg},-x 2>&1 | tee script_err.log
@@ -334,6 +340,15 @@ for arg in "--select-script $SELECT_SCRIPT" \
             exit 1
         fi
     fi
+    # run again  without error
+    echo genhtml $DIFCOV_OPTS initial.info -o scriptFixed ${arg}
+    $COVER $GENHTML_TOOL $DIFFCOV_OPTS initial.info -o scriptFixed ${arg} --ignore annotate 2>&1 | tee script_err.log
+    if [ 0 != ${PIPESTATUS[0]} ] ; then
+        echo "ERROR: genhtml scriptFixed failed"
+        if [ 0 == $KEEP_GOING ] ; then
+            exit 1
+        fi
+    fi
 done
 
 echo genhtml $DIFCOV_OPTS initial.info -o select --select-script ./select.sh --rc compute_file_version=1