freshlybakedca.ke / git
Git fork
fork atom

chainlint: don't be fooled by "?!...?!" in test body

As originally implemented, chainlint did not collect structured
information about detected problems. Instead, it merely emitted raw
parse tokens (not the original test text), along with a "?!...?!"
annotation directly into the output stream each time a problem was
discovered. In order to report statistics (in --stats mode) and to
adjust its exit code to indicate success or failure, it merely counts
the number of times "?!...?!" appears in the output stream. An obvious
shortcoming of this approach is that it can be fooled by a legitimate
"?!...?!" sequence in the body of a test (though, only if an actual
problem is detected in the test).

The situation did not improve when 7c04aa7390 (chainlint: colorize
problem annotations and test delimiters, 2022-09-13) colored the
annotations after-the-fact by searching for "?!...?!" in the output
stream and inserting color codes. As above, a shortcoming is that this
approach can incorrectly color a legitimate "?!...?!" sequence in a test
body as if it is an error.

However, when 73c768dae9 (chainlint: annotate original test definition
rather than token stream, 2022-11-08) taught chainlint to output the
original test text verbatim, it started collecting structured
information about detected problems.

Now that it is available, take advantage of the structured problem
information to deterministically count the number of problems detected
and to color the annotations directly, rather than scanning the output
stream for "?!...?!" and performing these operations after-the-fact.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

authored by

Eric Sunshine and committed by
Junio C Hamano 588ef84e 17d4b10a

+8 -5
unified split
+8 -5
t/chainlint.pl
··· 587 my $class = shift @_; 588 my $self = $class->SUPER::new(@_); 589 $self->{ntests} = 0; 590 return $self; 591 } 592 ··· 634 my $parser = TestParser->new(\$body); 635 my @tokens = $parser->parse(); 636 my $problems = $parser->{problems}; 637 return unless $emit_all || @$problems; 638 my $c = main::fd_colors(1); 639 my $start = 0; ··· 641 for (sort {$a->[1]->[2] <=> $b->[1]->[2]} @$problems) { 642 my ($label, $token) = @$_; 643 my $pos = $token->[2]; 644 - $checked .= substr($body, $start, $pos - $start) . " ?!$label?! "; 645 $start = $pos; 646 } 647 $checked .= substr($body, $start); 648 $checked =~ s/^/$lineno++ . ' '/mge; 649 $checked =~ s/^\d+ \n//; 650 - $checked =~ s/(\s) \?!/$1?!/mg; 651 - $checked =~ s/\?! (\s)/?!$1/mg; 652 - $checked =~ s/(\?![^?]+\?!)/$c->{rev}$c->{red}$1$c->{reset}/mg; 653 $checked =~ s/^\d+/$c->{dim}$&$c->{reset}/mg; 654 $checked .= "\n" unless $checked =~ /\n$/; 655 push(@{$self->{output}}, "$c->{blue}# chainlint: $title$c->{reset}\n$checked"); ··· 791 my $c = fd_colors(1); 792 my $s = join('', @{$parser->{output}}); 793 $emit->("$c->{bold}$c->{blue}# chainlint: $path$c->{reset}\n" . $s); 794 - $nerrs += () = $s =~ /\?![^?]+\?!/g; 795 } 796 $ntests += $parser->{ntests}; 797 } 798 return [$id, $nscripts, $ntests, $nerrs]; 799 }
··· 587 my $class = shift @_; 588 my $self = $class->SUPER::new(@_); 589 $self->{ntests} = 0; 590 + $self->{nerrs} = 0; 591 return $self; 592 } 593 ··· 635 my $parser = TestParser->new(\$body); 636 my @tokens = $parser->parse(); 637 my $problems = $parser->{problems}; 638 + $self->{nerrs} += @$problems; 639 return unless $emit_all || @$problems; 640 my $c = main::fd_colors(1); 641 my $start = 0; ··· 643 for (sort {$a->[1]->[2] <=> $b->[1]->[2]} @$problems) { 644 my ($label, $token) = @$_; 645 my $pos = $token->[2]; 646 + $checked .= substr($body, $start, $pos - $start); 647 + $checked .= ' ' unless $checked =~ /\s$/; 648 + $checked .= "$c->{rev}$c->{red}?!$label?!$c->{reset}"; 649 + $checked .= ' ' unless $pos >= length($body) || 650 + substr($body, $pos, 1) =~ /^\s/; 651 $start = $pos; 652 } 653 $checked .= substr($body, $start); 654 $checked =~ s/^/$lineno++ . ' '/mge; 655 $checked =~ s/^\d+ \n//; 656 $checked =~ s/^\d+/$c->{dim}$&$c->{reset}/mg; 657 $checked .= "\n" unless $checked =~ /\n$/; 658 push(@{$self->{output}}, "$c->{blue}# chainlint: $title$c->{reset}\n$checked"); ··· 794 my $c = fd_colors(1); 795 my $s = join('', @{$parser->{output}}); 796 $emit->("$c->{bold}$c->{blue}# chainlint: $path$c->{reset}\n" . $s); 797 } 798 $ntests += $parser->{ntests}; 799 + $nerrs += $parser->{nerrs}; 800 } 801 return [$id, $nscripts, $ntests, $nerrs]; 802 }