Skip to content

Commit

Permalink
Use ANSI colors for test results
Browse files Browse the repository at this point in the history 
Also:
* Refactor `TextFormatter` to be more generic (less indirection).
* Adjust test report slightly (no emojis, add more spacing).
  • Loading branch information
@bioball
bioball committed Nov 1, 2024
1 parent 3f38173 commit ac7c0a5
Show file tree
Hide file tree
Showing 15 changed files with 468 additions and 296 deletions.
2 changes: 1 addition & 1 deletion pkl-cli/src/main/kotlin/org/pkl/cli/CliTestRunner.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ constructor(
failed = results.failed()
isExampleWrittenFailure = results.isExampleWrittenFailure.and(isExampleWrittenFailure)
}
SimpleReport().report(results, consoleWriter)
SimpleReport(useColor).report(results, consoleWriter)
if (sources.size > 1 && idx != sources.size - 1) {
consoleWriter.append('\n')
}
Expand Down
30 changes: 18 additions & 12 deletions pkl-cli/src/test/kotlin/org/pkl/cli/CliTestRunnerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ class CliTestRunnerTest {
"""
module test
facts
✅ succeed
✅ 100.0% tests pass [1 passed], 100.0% asserts pass [2 passed]
✔ succeed
✔ 100.0% tests pass [1 passed], 100.0% asserts pass [2 passed]
"""
.trimIndent()
Expand Down Expand Up @@ -101,9 +102,10 @@ class CliTestRunnerTest {
"""
module test
facts
❌ fail
✘ fail
4 == 9 (/tempDir/test.pkl, line xx)
❌ 0.0% tests pass [1/1 failed], 50.0% asserts pass [1/2 failed]
✘ 0.0% tests pass [1/1 failed], 50.0% asserts pass [1/2 failed]
"""
.trimIndent()
Expand Down Expand Up @@ -137,14 +139,15 @@ class CliTestRunnerTest {
"""
module test
facts
❌ fail
✘ fail
–– Pkl Error ––
uh oh
5 | throw("uh oh")
^^^^^^^^^^^^^^
at test#facts["fail"][#1] (/tempDir/test.pkl, line xx)
❌ 0.0% tests pass [1/1 failed], 0.0% asserts pass [1/1 failed]
✘ 0.0% tests pass [1/1 failed], 0.0% asserts pass [1/1 failed]
"""
.trimIndent()
Expand Down Expand Up @@ -178,14 +181,15 @@ class CliTestRunnerTest {
"""
module test
examples
❌ fail
✘ fail
–– Pkl Error ––
uh oh
5 | throw("uh oh")
^^^^^^^^^^^^^^
at test#examples["fail"][#1] (/tempDir/test.pkl, line xx)
❌ 0.0% tests pass [1/1 failed], 0.0% asserts pass [1/1 failed]
✘ 0.0% tests pass [1/1 failed], 0.0% asserts pass [1/1 failed]
"""
.trimIndent()
Expand Down Expand Up @@ -233,14 +237,15 @@ class CliTestRunnerTest {
"""
module test
examples
❌ fail
✘ fail
–– Pkl Error ––
uh oh
5 | throw("uh oh")
^^^^^^^^^^^^^^
at test#examples["fail"][#1] (/tempDir/test.pkl, line xx)
❌ 0.0% tests pass [1/1 failed], 0.0% asserts pass [1/1 failed]
✘ 0.0% tests pass [1/1 failed], 0.0% asserts pass [1/1 failed]
"""
.trimIndent()
Expand Down Expand Up @@ -435,10 +440,11 @@ class CliTestRunnerTest {
"""
module test
examples
❌ nums
✘ nums
(/tempDir/test.pkl, line xx)
Output mismatch: Expected "nums" to contain 1 examples, but found 2
❌ 0.0% tests pass [1/1 failed], 0.0% asserts pass [1/1 failed]
✘ 0.0% tests pass [1/1 failed], 0.0% asserts pass [1/1 failed]
"""
.trimIndent()
Expand Down
4 changes: 3 additions & 1 deletion pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
)
}

protected val useColor: Boolean by lazy { cliOptions.color?.hasColor() ?: false }

private val proxyAddress by lazy {
cliOptions.httpProxy
?: project?.evaluatorSettings?.http?.proxy?.address ?: settings.http?.proxy?.address
Expand Down Expand Up @@ -284,7 +286,7 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
.setEnvironmentVariables(environmentVariables)
.addModuleKeyFactories(moduleKeyFactories(modulePathResolver))
.addResourceReaders(resourceReaders(modulePathResolver))
.setColor(cliOptions.color?.hasColor() ?: false)
.setColor(useColor)
.setLogger(Loggers.stdErr())
.setTimeout(cliOptions.timeout)
.setModuleCacheDir(moduleCacheDir)
Expand Down
13 changes: 12 additions & 1 deletion pkl-core/src/main/java/org/pkl/core/TestResults.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,18 @@ public TestResult build() {
}
}

public record Error(String message, PklException exception) {}
/**
* Indicates that an exception was thrown when evaluating the assertion.
*
* @param message The message of the underlying exception.
* @param coloredException The exception, whose message contains ANSI color codes.
* @param plainException The exception, whose message does not contain ANSI color codes.
*/
public record Error(String message, PklException coloredException, PklException plainException) {
public PklException getException(boolean useColor) {
return useColor ? coloredException : plainException;
}
}

public record Failure(String kind, String message) {}
}
68 changes: 36 additions & 32 deletions pkl-core/src/main/java/org/pkl/core/runtime/StackTraceRenderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import java.util.List;
import java.util.function.Function;
import org.pkl.core.StackFrame;
import org.pkl.core.runtime.TextFormatter.Element;
import org.pkl.core.util.ColorTheme;
import org.pkl.core.util.Nullable;

public final class StackTraceRenderer {
Expand All @@ -29,7 +29,8 @@ public StackTraceRenderer(Function<StackFrame, StackFrame> frameTransformer) {
this.frameTransformer = frameTransformer;
}

public void render(List<StackFrame> frames, @Nullable String hint, TextFormatter out) {
public void render(
List<StackFrame> frames, @Nullable String hint, TextFormattingStringBuilder out) {
var compressed = compressFrames(frames);
doRender(compressed, hint, out, "", true);
}
Expand All @@ -38,7 +39,7 @@ public void render(List<StackFrame> frames, @Nullable String hint, TextFormatter
void doRender(
List<Object /*StackFrame|StackFrameLoop*/> frames,
@Nullable String hint,
TextFormatter out,
TextFormattingStringBuilder out,
String leftMargin,
boolean isFirstElement) {
for (var frame : frames) {
Expand All @@ -48,25 +49,23 @@ void doRender(
doRender(loop.frames, null, out, leftMargin, isFirstElement);
} else {
if (!isFirstElement) {
out.margin(leftMargin).newline();
out.append(ColorTheme.STACK_TRACE_MARGIN, leftMargin).newline();
}
out.margin(leftMargin)
.margin("┌─ ")
.style(Element.STACK_OVERFLOW_LOOP_COUNT)
.append(loop.count)
.style(Element.TEXT)
out.append(ColorTheme.STACK_TRACE_MARGIN, leftMargin)
.append(ColorTheme.STACK_TRACE_MARGIN, "┌─ ")
.append(ColorTheme.STACK_TRACE_LOOP_COUNT, loop.count)
.append(" repetitions of:\n");
var newLeftMargin = leftMargin + "│ ";
doRender(loop.frames, null, out, newLeftMargin, isFirstElement);
if (isFirstElement) {
renderHint(hint, out, newLeftMargin);
isFirstElement = false;
}
out.margin(leftMargin).margin("└─").newline();
out.append(ColorTheme.STACK_TRACE_MARGIN, leftMargin + "└─").newline();
}
} else {
if (!isFirstElement) {
out.margin(leftMargin).newline();
out.append(ColorTheme.STACK_TRACE_MARGIN, leftMargin).newline();
}
renderFrame((StackFrame) frame, out, leftMargin);
}
Expand All @@ -78,19 +77,24 @@ void doRender(
}
}

private void renderFrame(StackFrame frame, TextFormatter out, String leftMargin) {
private void renderFrame(StackFrame frame, TextFormattingStringBuilder out, String leftMargin) {
var transformed = frameTransformer.apply(frame);
renderSourceLine(transformed, out, leftMargin);
renderSourceLocation(transformed, out, leftMargin);
}

private void renderHint(@Nullable String hint, TextFormatter out, String leftMargin) {
private void renderHint(
@Nullable String hint, TextFormattingStringBuilder out, String leftMargin) {
if (hint == null || hint.isEmpty()) return;

out.newline().margin(leftMargin).style(Element.HINT).append(hint).newline();
out.newline()
.append(ColorTheme.STACK_TRACE_MARGIN, leftMargin)
.append(ColorTheme.ERROR_MESSAGE_HINT, hint)
.newline();
}

private void renderSourceLine(StackFrame frame, TextFormatter out, String leftMargin) {
private void renderSourceLine(
StackFrame frame, TextFormattingStringBuilder out, String leftMargin) {
var originalSourceLine = frame.getSourceLines().get(0);
var leadingWhitespace = VmUtils.countLeadingWhitespace(originalSourceLine);
var sourceLine = originalSourceLine.strip();
Expand All @@ -101,28 +105,28 @@ private void renderSourceLine(StackFrame frame, TextFormatter out, String leftMa
: sourceLine.length();

var prefix = frame.getStartLine() + " | ";
out.margin(leftMargin)
.style(Element.LINE_NUMBER)
.append(prefix)
.style(Element.TEXT)
out.append(ColorTheme.STACK_TRACE_MARGIN, leftMargin)
.append(ColorTheme.STACK_TRACE_LINE_NUMBER, prefix)
.append(sourceLine)
.newline()
.margin(leftMargin)
.repeat(prefix.length() + startColumn - 1, ' ')
.style(Element.ERROR)
.repeat(endColumn - startColumn + 1, '^')
.append(ColorTheme.STACK_TRACE_MARGIN, leftMargin)
.append(" ".repeat(prefix.length() + startColumn - 1))
.append(ColorTheme.STACK_TRACE_CARET, "^".repeat(endColumn - startColumn + 1))
.newline();
}

private void renderSourceLocation(StackFrame frame, TextFormatter out, String leftMargin) {
out.margin(leftMargin)
.style(Element.TEXT)
.append("at ")
.append(frame.getMemberName() != null ? frame.getMemberName() : "<unknown>")
.append(" (")
.append(frame.getModuleUri())
.append(")")
.newline();
private void renderSourceLocation(
StackFrame frame, TextFormattingStringBuilder out, String leftMargin) {
out.append(ColorTheme.STACK_TRACE_MARGIN, leftMargin)
.append(
ColorTheme.STACK_FRAME,
() ->
out.append("at ")
.append(frame.getMemberName() != null ? frame.getMemberName() : "<unknown>")
.append(" (")
.append(frame.getModuleUri())
.append(")")
.newline());
}

/**
Expand Down
17 changes: 13 additions & 4 deletions pkl-core/src/main/java/org/pkl/core/runtime/TestRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ public TestResults run(VmTyped testModule) {
checkAmendsPklTest(testModule);
} catch (VmException v) {
var error =
new TestResults.Error(v.getMessage(), v.toPklException(stackFrameTransformer, false));
new TestResults.Error(
v.getMessage(),
v.toPklException(stackFrameTransformer, true),
v.toPklException(stackFrameTransformer, false));
return resultsBuilder.setError(error).build();
}

Expand Down Expand Up @@ -109,7 +112,9 @@ private TestSectionResults runFacts(VmTyped testModule) {
} catch (VmException err) {
var error =
new TestResults.Error(
err.getMessage(), err.toPklException(stackFrameTransformer, false));
err.getMessage(),
err.toPklException(stackFrameTransformer, true),
err.toPklException(stackFrameTransformer, false));
resultBuilder.addError(error);
}
return true;
Expand Down Expand Up @@ -209,7 +214,9 @@ private TestSectionResults doRunAndValidateExamples(
errored.set(true);
testResultBuilder.addError(
new TestResults.Error(
err.getMessage(), err.toPklException(stackFrameTransformer, false)));
err.getMessage(),
err.toPklException(stackFrameTransformer, true),
err.toPklException(stackFrameTransformer, false)));
return true;
}
var expectedValue = VmUtils.readMember(expectedGroup, exampleIndex);
Expand Down Expand Up @@ -306,7 +313,9 @@ private TestSectionResults doRunAndWriteExamples(VmMapping examples, Path output
} catch (VmException err) {
testResultBuilder.addError(
new TestResults.Error(
err.getMessage(), err.toPklException(stackFrameTransformer, false)));
err.getMessage(),
err.toPklException(stackFrameTransformer, true),
err.toPklException(stackFrameTransformer, false)));
allSucceeded.set(false);
success.set(false);
return true;
Expand Down
Loading

0 comments on commit ac7c0a5

Please sign in to comment.