From dcc8ce53c95466d12821902ba36f1ae22c88a2e8 Mon Sep 17 00:00:00 2001 From: Fred Emmott Date: Fri, 1 Nov 2019 16:11:22 -0700 Subject: [PATCH 1/4] Pretty-print context on failures --- src/HackTestCLI.hack | 4 +- src/_Private/CLIOutputHandler.hack | 78 +++++++++++++++++++++++++++++- src/_Private/ConciseCLIOutput.hack | 11 +++-- src/_Private/VerboseCLIOutput.hack | 7 +++ 4 files changed, 93 insertions(+), 7 deletions(-) diff --git a/src/HackTestCLI.hack b/src/HackTestCLI.hack index 4cd83f3..9d3f844 100644 --- a/src/HackTestCLI.hack +++ b/src/HackTestCLI.hack @@ -87,8 +87,8 @@ final class HackTestCLI extends CLIWithRequiredArguments { $cf = $this->classFilter; $mf = $this->methodFilter; $output = $this->verbose - ? new _Private\VerboseCLIOutput() - : new _Private\ConciseCLIOutput(); + ? new _Private\VerboseCLIOutput($this->getTerminal()) + : new _Private\ConciseCLIOutput($this->getTerminal()); $stdout = $this->getStdout(); await HackTestRunner::runAsync( diff --git a/src/_Private/CLIOutputHandler.hack b/src/_Private/CLIOutputHandler.hack index 9658b16..3bcd163 100644 --- a/src/_Private/CLIOutputHandler.hack +++ b/src/_Private/CLIOutputHandler.hack @@ -9,7 +9,7 @@ namespace Facebook\HackTest\_Private; -use namespace HH\Lib\{Dict, Math, Str}; +use namespace HH\Lib\{C, Dict, Math, Str, Vec}; use namespace HH\Lib\Experimental\IO; use namespace Facebook\HackTest; @@ -17,6 +17,11 @@ abstract class CLIOutputHandler { <<__LateInit>> private dict $resultCounts; <<__LateInit>> private vec $errors; + const int CONTEXT_LINES = 3; + + public function __construct(private \Facebook\CLILib\ITerminal $terminal) { + } + final public async function writeProgressAsync( <<__AcceptDisposable>> IO\WriteHandle $handle, \Facebook\HackTest\ProgressEvent $e, @@ -110,4 +115,75 @@ abstract class CLIOutputHandler { $result_counts[HackTest\TestResult::ERROR] ?? 0, )); } + + final protected function getPrettyContext( + \Throwable $ex, + string $file, + ): ?string { + $frame = $ex->getTrace() + |> Vec\filter( + $$, + $row ==> + (($row as KeyedContainer<_, _>)['file'] ?? null) as ?string === $file, + ) + |> C\last($$); + + if (!$frame is KeyedContainer<_, _>) { + return null; + } + $colors = $this->terminal->supportsColors(); + $c_light = $colors ? "\e[2m" : ''; + $c_bold = $colors ? "\e[1m" : ''; + $c_red = $colors ? "\e[31m" : ''; + $c_reset = $colors ? "\e[0m" : ''; + + $line = $frame['line'] as int; + $line_number_width = Str\length((string)$line) + 2; + + $first_line = Math\maxva(1, $line - self::CONTEXT_LINES); + $all_lines = \file_get_contents($file) + |> Str\split($$, "\n"); + + $context_lines = Vec\slice( + $all_lines, + $first_line - 1, + ($line - $first_line), + ) + |> Vec\map_with_key( + $$, + ($n, $content) ==> Str\format( + "%s| %s%s%s", + Str\pad_left((string)($n + $first_line), $line_number_width, ' '), + $c_light, + $content, + $c_reset, + ), + ); + + $blame_line = $all_lines[$line - 1]; + $fun = $frame['function'] as string; + $fun_offset = Str\search($blame_line, $fun) as nonnull; + + $context_lines[] = Str\format( + "%s%s>%s %s%s%s%s%s%s", + Str\pad_left((string)$line, $line_number_width), + $c_red, + $c_reset.$c_bold, + Str\slice($blame_line, 0, $fun_offset), + $c_red, + $fun, + $c_reset.$c_bold, + Str\slice($blame_line, $fun_offset + Str\length($fun)), + $c_reset, + ); + + $context_lines[] = Str\format( + "%s%s%s%s", + Str\repeat(' ', $line_number_width + $fun_offset + 2), + $c_red, + Str\repeat('^', Str\length($fun)), + $c_reset, + ); + return $file.':'.$line."\n".Str\join($context_lines, "\n"); + } } diff --git a/src/_Private/ConciseCLIOutput.hack b/src/_Private/ConciseCLIOutput.hack index 2cfeb44..3e45176 100644 --- a/src/_Private/ConciseCLIOutput.hack +++ b/src/_Private/ConciseCLIOutput.hack @@ -11,12 +11,13 @@ namespace Facebook\HackTest\_Private; -use namespace HH\Lib\{Str, Vec}; +use namespace HH\Lib\{Math, Str, Vec}; use namespace HH\Lib\Experimental\IO; use namespace Facebook\HackTest; use type Facebook\HackTest\TestResult; final class ConciseCLIOutput extends CLIOutputHandler { + <<__Override>> protected async function writeProgressImplAsync( <<__AcceptDisposable>> IO\WriteHandle $handle, @@ -63,14 +64,16 @@ final class ConciseCLIOutput extends CLIOutputHandler { $message = 'Skipped: '.$ex->getMessage(); } else if ($event is HackTest\FileProgressEvent) { $file = $event->getPath(); - $trace = $ex->getTraceAsString() + + $context = $this->getPrettyContext($ex, $file) ?? + $ex->getTraceAsString() |> Str\split($$, '#') |> Vec\filter($$, $line ==> Str\contains($line, $file)) |> Vec\map($$, $line ==> Str\strip_prefix($line, ' ')) |> Str\join($$, "\n"); - if ($trace !== '') { - $message .= "\n\n".$trace; + if ($context !== '') { + $message .= "\n\n".$context; } } await $handle->writeAsync($header.$message); diff --git a/src/_Private/VerboseCLIOutput.hack b/src/_Private/VerboseCLIOutput.hack index 274bf9e..195217b 100644 --- a/src/_Private/VerboseCLIOutput.hack +++ b/src/_Private/VerboseCLIOutput.hack @@ -102,6 +102,13 @@ final class VerboseCLIOutput extends CLIOutputHandler { $message .= "\n\nPrevious exception:\n\n"; } } + if ($event is HackTest\FileProgressEvent) { + $file = $event->getPath(); + $context = $this->getPrettyContext($ex, $file); + if ($context is nonnull) { + $message .= "\n\n".$context; + } + } await $handle->writeAsync($header.$message); } } From 43e709ea3efd2dfc776ddfdd70e98ddc347b59e6 Mon Sep 17 00:00:00 2001 From: Fred Emmott Date: Mon, 4 Nov 2019 09:27:09 -0800 Subject: [PATCH 2/4] Handle function names that do not appear verbatim in the caller - trace contains fully-qualified names. Namespaced functions may be autoimported, or explicitly used - `invariant()` shows up in backtrace as `invariant_violation` - if we don't find it at all, report the whole line --- src/_Private/CLIOutputHandler.hack | 65 ++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/src/_Private/CLIOutputHandler.hack b/src/_Private/CLIOutputHandler.hack index 3bcd163..918d918 100644 --- a/src/_Private/CLIOutputHandler.hack +++ b/src/_Private/CLIOutputHandler.hack @@ -162,28 +162,49 @@ abstract class CLIOutputHandler { $blame_line = $all_lines[$line - 1]; $fun = $frame['function'] as string; - $fun_offset = Str\search($blame_line, $fun) as nonnull; - - $context_lines[] = Str\format( - "%s%s>%s %s%s%s%s%s%s", - Str\pad_left((string)$line, $line_number_width), - $c_red, - $c_reset.$c_bold, - Str\slice($blame_line, 0, $fun_offset), - $c_red, - $fun, - $c_reset.$c_bold, - Str\slice($blame_line, $fun_offset + Str\length($fun)), - $c_reset, - ); - - $context_lines[] = Str\format( - "%s%s%s%s", - Str\repeat(' ', $line_number_width + $fun_offset + 2), - $c_red, - Str\repeat('^', Str\length($fun)), - $c_reset, - ); + $fun_offset = Str\search($blame_line, $fun.'('); + if ($fun_offset is null && Str\contains($fun, '\\')) { + $fun = Str\split($fun, '\\') |> C\lastx($$); + $fun_offset = Str\search($blame_line, $fun.'('); + } + if ( + $fun_offset is null && $frame['function'] === "HH\\invariant_violation" + ) { + $fun = 'invariant'; + $fun_offset = Str\search($blame_line, 'invariant('); + } + + if ($fun_offset is null) { + $context_lines[] = Str\format( + "%s%s>%s %s%s", + Str\pad_left((string)$line, $line_number_width), + $c_red, + $c_reset.$c_bold, + $blame_line, + $c_reset, + ); + } else { + $context_lines[] = Str\format( + "%s%s>%s %s%s%s%s%s%s", + Str\pad_left((string)$line, $line_number_width), + $c_red, + $c_reset.$c_bold, + Str\slice($blame_line, 0, $fun_offset), + $c_red, + $fun, + $c_reset.$c_bold, + Str\slice($blame_line, $fun_offset + Str\length($fun)), + $c_reset, + ); + + $context_lines[] = Str\format( + "%s%s%s%s", + Str\repeat(' ', $line_number_width + $fun_offset + 2), + $c_red, + Str\repeat('^', Str\length($fun)), + $c_reset, + ); + } return $file.':'.$line."\n".Str\join($context_lines, "\n"); } } From a53171214c0b59ed9d86f53319cc6757461c6d10 Mon Sep 17 00:00:00 2001 From: Fred Emmott Date: Mon, 4 Nov 2019 09:35:36 -0800 Subject: [PATCH 3/4] linter --- src/_Private/ConciseCLIOutput.hack | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_Private/ConciseCLIOutput.hack b/src/_Private/ConciseCLIOutput.hack index 3e45176..3ad3dbf 100644 --- a/src/_Private/ConciseCLIOutput.hack +++ b/src/_Private/ConciseCLIOutput.hack @@ -11,7 +11,7 @@ namespace Facebook\HackTest\_Private; -use namespace HH\Lib\{Math, Str, Vec}; +use namespace HH\Lib\{Str, Vec}; use namespace HH\Lib\Experimental\IO; use namespace Facebook\HackTest; use type Facebook\HackTest\TestResult; From 1ec416cdd09e38d32bb9af9cf6e8667a6691e3a3 Mon Sep 17 00:00:00 2001 From: Fred Emmott Date: Mon, 4 Nov 2019 14:26:23 -0800 Subject: [PATCH 4/4] support repo-auth mode --- src/_Private/CLIOutputHandler.hack | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/_Private/CLIOutputHandler.hack b/src/_Private/CLIOutputHandler.hack index 918d918..546c601 100644 --- a/src/_Private/CLIOutputHandler.hack +++ b/src/_Private/CLIOutputHandler.hack @@ -120,6 +120,11 @@ abstract class CLIOutputHandler { \Throwable $ex, string $file, ): ?string { + if (!\file_exists($file)) { + // Possibly running in repo-authoritative mode + return null; + } + $frame = $ex->getTrace() |> Vec\filter( $$,