@matheusbsbs / matheus-bagliano / commits / 7c04aa7390

chainlint: colorize problem annotations and test delimiters

When `chainlint.pl` detects problems in a test definition, it emits the test definition with "?!FOO?!" annotations highlighting the problems it discovered. For instance, given this problematic test: test_expect_success 'discombobulate frobnitz' ' git frob babble && (echo balderdash; echo gnabgib) >expect && for i in three two one do git nitfol $i done >actual test_cmp expect actual ' chainlint.pl will output: # chainlint: t1234-confusing.sh # chainlint: discombobulate frobnitz git frob babble && (echo balderdash ; ?!AMP?! echo gnabgib) >expect && for i in three two one do git nitfol $i ?!LOOP?! done >actual ?!AMP?! test_cmp expect actual in which it may be difficult to spot the "?!FOO?!" annotations. The problem is compounded when multiple tests, possibly in multiple scripts, fail "linting", in which case it may be difficult to spot the "# chainlint:" lines which delimit one problematic test from another. To ameliorate this potential problem, colorize the "?!FOO?!" annotations in order to quickly draw the test author's attention to the problem spots, and colorize the "# chainlint:" lines to help the author identify the name of each script and each problematic test. Colorization is disabled automatically if output is not directed to a terminal or if NO_COLOR environment variable is set. The implementation is specific to Unix (it employs `tput` if available) but works equally well in the Git for Windows development environment which emulates Unix sufficiently. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Eric Sunshine committed Sep 13, 2022 at 04:01 UTC 7c04aa73906b9186c9d46010227d4437fd534d93
1 file changed +43 -3
t/chainlint.pl
+43 -3
@@ -585,12 +585,14 @@ sub check_test {
585 my $parser = TestParser->new(\$body);
586 my @tokens = $parser->parse();
587 return unless $emit_all || grep(/\?![^?]+\?!/, @tokens);
588 + my $c = main::fd_colors(1);
589 my $checked = join(' ', @tokens);
590 $checked =~ s/^\n//;
591 $checked =~ s/^ //mg;
592 $checked =~ s/ $//mg;
593 + $checked =~ s/(\?![^?]+\?!)/$c->{rev}$c->{red}$1$c->{reset}/mg;
594 $checked .= "\n" unless $checked =~ /\n$/;
593 - push(@{$self->{output}}, "# chainlint: $title\n$checked");
595 + push(@{$self->{output}}, "$c->{blue}# chainlint: $title$c->{reset}\n$checked");
596 }
597
598 sub parse_cmd {
@@ -615,6 +617,41 @@ if (eval {require Time::HiRes; Time::HiRes->import(); 1;}) {
617 $interval = sub { return Time::HiRes::tv_interval(shift); };
618 }
619
620 +# Restore TERM if test framework set it to "dumb" so 'tput' will work; do this
621 +# outside of get_colors() since under 'ithreads' all threads use %ENV of main
622 +# thread and ignore %ENV changes in subthreads.
623 +$ENV{TERM} = $ENV{USER_TERM} if $ENV{USER_TERM};
624 +
625 +my @NOCOLORS = (bold => '', rev => '', reset => '', blue => '', green => '', red => '');
626 +my %COLORS = ();
627 +sub get_colors {
628 + return \%COLORS if %COLORS;
629 + if (exists($ENV{NO_COLOR}) ||
630 + system("tput sgr0 >/dev/null 2>&1") != 0 ||
631 + system("tput bold >/dev/null 2>&1") != 0 ||
632 + system("tput rev >/dev/null 2>&1") != 0 ||
633 + system("tput setaf 1 >/dev/null 2>&1") != 0) {
634 + %COLORS = @NOCOLORS;
635 + return \%COLORS;
636 + }
637 + %COLORS = (bold => `tput bold`,
638 + rev => `tput rev`,
639 + reset => `tput sgr0`,
640 + blue => `tput setaf 4`,
641 + green => `tput setaf 2`,
642 + red => `tput setaf 1`);
643 + chomp(%COLORS);
644 + return \%COLORS;
645 +}
646 +
647 +my %FD_COLORS = ();
648 +sub fd_colors {
649 + my $fd = shift;
650 + return $FD_COLORS{$fd} if exists($FD_COLORS{$fd});
651 + $FD_COLORS{$fd} = -t $fd ? get_colors() : {@NOCOLORS};
652 + return $FD_COLORS{$fd};
653 +}
654 +
655 sub ncores {
656 # Windows
657 return $ENV{NUMBER_OF_PROCESSORS} if exists($ENV{NUMBER_OF_PROCESSORS});
@@ -630,6 +667,8 @@ sub show_stats {
667 my $walltime = $interval->($start_time);
668 my ($usertime) = times();
669 my ($total_workers, $total_scripts, $total_tests, $total_errs) = (0, 0, 0, 0);
670 + my $c = fd_colors(2);
671 + print(STDERR $c->{green});
672 for (@$stats) {
673 my ($worker, $nscripts, $ntests, $nerrs) = @$_;
674 print(STDERR "worker $worker: $nscripts scripts, $ntests tests, $nerrs errors\n");
@@ -638,7 +677,7 @@ sub show_stats {
677 $total_tests += $ntests;
678 $total_errs += $nerrs;
679 }
641 - printf(STDERR "total: %d workers, %d scripts, %d tests, %d errors, %.2fs/%.2fs (wall/user)\n", $total_workers, $total_scripts, $total_tests, $total_errs, $walltime, $usertime);
680 + printf(STDERR "total: %d workers, %d scripts, %d tests, %d errors, %.2fs/%.2fs (wall/user)$c->{reset}\n", $total_workers, $total_scripts, $total_tests, $total_errs, $walltime, $usertime);
681 }
682
683 sub check_script {
@@ -656,8 +695,9 @@ sub check_script {
695 my $parser = ScriptParser->new(\$s);
696 1 while $parser->parse_cmd();
697 if (@{$parser->{output}}) {
698 + my $c = fd_colors(1);
699 my $s = join('', @{$parser->{output}});
660 - $emit->("# chainlint: $path\n" . $s);
700 + $emit->("$c->{bold}$c->{blue}# chainlint: $path$c->{reset}\n" . $s);
701 $nerrs += () = $s =~ /\?![^?]+\?!/g;
702 }
703 $ntests += $parser->{ntests};