From 89136eb150a4c55fc7482f3721809aeea342c487 Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Sun, 9 Aug 2020 11:58:08 -0400 Subject: [PATCH 1/2] Make diff section contents red in run-tests.php This deliberately uses only one color. Green would be associated with things that were done correctly. But both additional lines and missing lines contribute to the failure, so that may be confusing for something that's an error report instead of a diff that is meant to be applied. Red would make the failure causes stand out visually when scrolling through errors. --- run-tests.php | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/run-tests.php b/run-tests.php index 5839c7e90f76c..8fe09a1dd130f 100755 --- a/run-tests.php +++ b/run-tests.php @@ -2877,6 +2877,7 @@ function count_array_diff( function generate_array_diff(array $ar1, array $ar2, bool $is_reg, array $w): array { + global $colorize; $idx1 = 0; $cnt1 = @count($ar1); $idx2 = 0; @@ -2885,6 +2886,23 @@ function generate_array_diff(array $ar1, array $ar2, bool $is_reg, array $w): ar $old1 = array(); $old2 = array(); + $format_diff_line = function (int $line_number, string $contents, string $diff_character) use ($colorize): string { + $output = sprintf("%03d%s ", $line_number, $diff_character) . $contents; + if ($colorize) { + // Light red for any line of the diff, for each line (e.g. in case post-processing removes lines). + // Using green for the expected lines missing from the output + // would cause some confusion (green could be interpreted as something the test did correctly) + return "\e[1;31m{$output}\e[0m"; + } + return $output; + }; + $format_expected_line = function (int $line_number, string $contents) use ($format_diff_line): string { + return $format_diff_line($line_number, $contents, '-'); + }; + $format_actual_line = function (int $line_number, string $contents) use ($format_diff_line): string { + return $format_diff_line($line_number, $contents, '+'); + }; + while ($idx1 < $cnt1 && $idx2 < $cnt2) { if (comp_line($ar1[$idx1], $ar2[$idx2], $is_reg)) { $idx1++; @@ -2895,12 +2913,12 @@ function generate_array_diff(array $ar1, array $ar2, bool $is_reg, array $w): ar $c2 = @count_array_diff($ar1, $ar2, $is_reg, $w, $idx1, $idx2 + 1, $cnt1, $cnt2, 10); if ($c1 > $c2) { - $old1[$idx1] = sprintf("%03d- ", $idx1 + 1) . $w[$idx1++]; + $old1[$idx1] = $format_expected_line($idx1 + 1, $w[$idx1++]); } elseif ($c2 > 0) { - $old2[$idx2] = sprintf("%03d+ ", $idx2 + 1) . $ar2[$idx2++]; + $old2[$idx2] = $format_actual_line($idx2 + 1, $ar2[$idx2++]); } else { - $old1[$idx1] = sprintf("%03d- ", $idx1 + 1) . $w[$idx1++]; - $old2[$idx2] = sprintf("%03d+ ", $idx2 + 1) . $ar2[$idx2++]; + $old1[$idx1] = $format_expected_line($idx1 + 1, $w[$idx1++]); + $old2[$idx2] = $format_actual_line($idx2 + 1, $ar2[$idx2++]); } } } @@ -2933,11 +2951,11 @@ function generate_array_diff(array $ar1, array $ar2, bool $is_reg, array $w): ar } while ($idx1 < $cnt1) { - $diff[] = sprintf("%03d- ", $idx1 + 1) . $w[$idx1++]; + $diff[] = $format_expected_line($idx1 + 1, $w[$idx1++]); } while ($idx2 < $cnt2) { - $diff[] = sprintf("%03d+ ", $idx2 + 1) . $ar2[$idx2++]; + $diff[] = $format_actual_line($idx2 + 1, $ar2[$idx2++]); } return $diff; From 320588a90bcd5ed227c2bb2f9ae0c502a69e9f18 Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Sun, 9 Aug 2020 16:04:34 -0400 Subject: [PATCH 2/2] Format the + lines as green and - lines as red --- run-tests.php | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/run-tests.php b/run-tests.php index 8fe09a1dd130f..6e4c19aef0988 100755 --- a/run-tests.php +++ b/run-tests.php @@ -2886,21 +2886,23 @@ function generate_array_diff(array $ar1, array $ar2, bool $is_reg, array $w): ar $old1 = array(); $old2 = array(); - $format_diff_line = function (int $line_number, string $contents, string $diff_character) use ($colorize): string { - $output = sprintf("%03d%s ", $line_number, $diff_character) . $contents; + $format_expected_line = function (int $line_number, string $contents) use ($colorize): string { + $output = sprintf("%03d- ", $line_number) . $contents; if ($colorize) { - // Light red for any line of the diff, for each line (e.g. in case post-processing removes lines). - // Using green for the expected lines missing from the output - // would cause some confusion (green could be interpreted as something the test did correctly) + // Reuse the colors used for `-` in other diff tools. + // Here, red should be interpreted as "removed", and not "bad". return "\e[1;31m{$output}\e[0m"; } return $output; }; - $format_expected_line = function (int $line_number, string $contents) use ($format_diff_line): string { - return $format_diff_line($line_number, $contents, '-'); - }; - $format_actual_line = function (int $line_number, string $contents) use ($format_diff_line): string { - return $format_diff_line($line_number, $contents, '+'); + $format_actual_line = function (int $line_number, string $contents) use ($colorize): string { + $output = sprintf("%03d+ ", $line_number) . $contents; + if ($colorize) { + // Reuse the colors used for `+` in other diff tools. + // Here, green should be interpreted as "added", and not "good". + return "\e[1;32m{$output}\e[0m"; + } + return $output; }; while ($idx1 < $cnt1 && $idx2 < $cnt2) {