Skip to content

Commit 1cae5e1

Browse files
plemarquandplemarquand
committed
Associate comment details on #expect w/ test issue
If a swift-testing #expect macro has preceding comments they are sent along with the expectation failure text as entries in the EventMessage array. Pull them out of the array so they don't appear as their own separate issues and append them to the actual issue text. Also fixes up some of the issue message colorization to be more consistent with how swift-testing behaves on the command line.
1 parent d801d41 commit 1cae5e1

File tree

3 files changed

+177
-35
lines changed

3 files changed

+177
-35
lines changed

Diff for: ‎src/TestExplorer/TestParsers/SwiftTestingOutputParser.ts

+122-18
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ export enum TestSymbol {
148148
difference = "difference",
149149
warning = "warning",
150150
details = "details",
151+
none = "none",
151152
}
152153

153154
export interface EventMessage {
@@ -222,9 +223,29 @@ export class SwiftTestingOutputParser {
222223
* @param chunk A chunk of stdout emitted during a test run.
223224
*/
224225
public parseStdout = (() => {
225-
const values = Object.values(TestSymbol).map(symbol =>
226-
regexEscapedString(SymbolRenderer.eventMessageSymbol(symbol))
227-
);
226+
const values = [
227+
...Object.values(TestSymbol)
228+
.filter(symbol => symbol !== TestSymbol.none)
229+
.map(symbol =>
230+
regexEscapedString(
231+
// Trim the ANSI reset code from the search since some lines
232+
// are fully colorized from the symbol to the end of line.
233+
SymbolRenderer.eventMessageSymbol(symbol).replace(
234+
SymbolRenderer.resetANSIEscapeCode,
235+
""
236+
)
237+
)
238+
),
239+
// It is possible there is no symbol for a line produced by swift-testing,
240+
// for instance if the user has a multi line comment before a failing expectation
241+
// only the first line of the printed comment will have a symbol, but to make the
242+
// indentation consistent the subsequent lines will have three spaces. We don't want
243+
// to treat this as output produced by the user during the test run, so omit these.
244+
// This isn't ideal since this will swallow lines the user prints if they start with
245+
// three spaces, but until we have user output as part of the JSON event stream we have
246+
// this workaround.
247+
" ",
248+
];
228249

229250
// Build a regex of all the line beginnings that come out of swift-testing events.
230251
const isSwiftTestingLineBeginning = new RegExp(`^${values.join("|")}`);
@@ -313,13 +334,47 @@ export class SwiftTestingOutputParser {
313334
testIndex: number | undefined
314335
) {
315336
messages.forEach(message => {
316-
runState.recordOutput(
317-
testIndex,
318-
`${SymbolRenderer.eventMessageSymbol(message.symbol)} ${message.text}\r\n`
319-
);
337+
runState.recordOutput(testIndex, `${MessageRenderer.render(message)}\r\n`);
320338
});
321339
}
322340

341+
/**
342+
* Partitions a collection of messages in to issues and details about the issues.
343+
* This is used to print the issues first, followed by the details.
344+
*/
345+
private partitionIssueMessages(messages: EventMessage[]): {
346+
issues: EventMessage[];
347+
details: EventMessage[];
348+
} {
349+
return messages.reduce(
350+
(buckets, message) => {
351+
const key =
352+
message.symbol === "details" ||
353+
message.symbol === "default" ||
354+
message.symbol === "none"
355+
? "details"
356+
: "issues";
357+
return { ...buckets, [key]: [...buckets[key], message] };
358+
},
359+
{
360+
issues: [],
361+
details: [],
362+
}
363+
);
364+
}
365+
366+
/*
367+
* A multi line comment preceeding an issue will have a 'default' symbol for
368+
* all lines except the first one. To match the swift-testing command line we
369+
* should show no symbol on these lines.
370+
*/
371+
private transformIssueMessageSymbols(messages: EventMessage[]): EventMessage[] {
372+
return messages.map(message => ({
373+
...message,
374+
symbol: message.symbol === "default" ? TestSymbol.none : message.symbol,
375+
}));
376+
}
377+
323378
private parse(item: SwiftTestEvent, runState: ITestRunState) {
324379
if (
325380
item.kind === "test" &&
@@ -391,14 +446,31 @@ export class SwiftTestingOutputParser {
391446
sourceLocation.line,
392447
sourceLocation.column
393448
);
394-
item.payload.messages.forEach(message => {
395-
runState.recordIssue(testIndex, message.text, isKnown, location);
449+
450+
const messages = this.transformIssueMessageSymbols(item.payload.messages);
451+
const { issues, details } = this.partitionIssueMessages(messages);
452+
453+
// Order the details after the issue text.
454+
const additionalDetails = details
455+
.map(message => MessageRenderer.render(message))
456+
.join("\n");
457+
458+
issues.forEach(message => {
459+
runState.recordIssue(
460+
testIndex,
461+
additionalDetails.length > 0
462+
? `${MessageRenderer.render(message)}\n${additionalDetails}`
463+
: MessageRenderer.render(message),
464+
isKnown,
465+
location
466+
);
396467
});
397-
this.recordOutput(runState, item.payload.messages, testIndex);
468+
469+
this.recordOutput(runState, messages, testIndex);
398470

399471
if (item.payload._testCase && testID !== item.payload.testID) {
400472
const testIndex = this.getTestCaseIndex(runState, item.payload.testID);
401-
item.payload.messages.forEach(message => {
473+
messages.forEach(message => {
402474
runState.recordIssue(testIndex, message.text, isKnown, location);
403475
});
404476
}
@@ -439,6 +511,32 @@ export class SwiftTestingOutputParser {
439511
}
440512
}
441513

514+
export class MessageRenderer {
515+
/**
516+
* Converts a swift-testing `EventMessage` to a colorized symbol and message text.
517+
*
518+
* @param message An event message, typically found on an `EventRecordPayload`.
519+
* @returns A string colorized with ANSI escape codes.
520+
*/
521+
static render(message: EventMessage): string {
522+
return `${SymbolRenderer.eventMessageSymbol(message.symbol)} ${MessageRenderer.colorize(message.symbol, message.text)}`;
523+
}
524+
525+
private static colorize(symbolType: TestSymbol, message: string): string {
526+
const ansiEscapeCodePrefix = "\u{001B}[";
527+
const resetANSIEscapeCode = `${ansiEscapeCodePrefix}0m`;
528+
switch (symbolType) {
529+
case TestSymbol.details:
530+
case TestSymbol.skip:
531+
case TestSymbol.difference:
532+
case TestSymbol.passWithKnownIssue:
533+
return `${ansiEscapeCodePrefix}90m${message}${resetANSIEscapeCode}`;
534+
default:
535+
return message;
536+
}
537+
}
538+
}
539+
442540
export class SymbolRenderer {
443541
/**
444542
* Converts a swift-testing symbol identifier in to a colorized unicode symbol.
@@ -450,6 +548,9 @@ export class SymbolRenderer {
450548
return this.colorize(symbol, this.symbol(symbol));
451549
}
452550

551+
static ansiEscapeCodePrefix = "\u{001B}[";
552+
static resetANSIEscapeCode = `${SymbolRenderer.ansiEscapeCodePrefix}0m`;
553+
453554
// This is adapted from
454555
// https://github.com/apple/swift-testing/blob/786ade71421eb1d8a9c1d99c902cf1c93096e7df/Sources/Testing/Events/Recorder/Event.Symbol.swift#L102
455556
public static symbol(symbol: TestSymbol): string {
@@ -469,6 +570,8 @@ export class SymbolRenderer {
469570
return "\u{25B2}"; // Unicode: BLACK UP-POINTING TRIANGLE
470571
case TestSymbol.details:
471572
return "\u{2192}"; // Unicode: RIGHTWARDS ARROW
573+
case TestSymbol.none:
574+
return "";
472575
}
473576
} else {
474577
switch (symbol) {
@@ -486,28 +589,29 @@ export class SymbolRenderer {
486589
return "\u{26A0}\u{FE0E}"; // Unicode: WARNING SIGN + VARIATION SELECTOR-15 (disable emoji)
487590
case TestSymbol.details:
488591
return "\u{21B3}"; // Unicode: DOWNWARDS ARROW WITH TIP RIGHTWARDS
592+
case TestSymbol.none:
593+
return " ";
489594
}
490595
}
491596
}
492597

493598
// This is adapted from
494599
// https://github.com/apple/swift-testing/blob/786ade71421eb1d8a9c1d99c902cf1c93096e7df/Sources/Testing/Events/Recorder/Event.ConsoleOutputRecorder.swift#L164
495600
private static colorize(symbolType: TestSymbol, symbol: string): string {
496-
const ansiEscapeCodePrefix = "\u{001B}[";
497-
const resetANSIEscapeCode = `${ansiEscapeCodePrefix}0m`;
498601
switch (symbolType) {
499602
case TestSymbol.default:
603+
case TestSymbol.details:
500604
case TestSymbol.skip:
501605
case TestSymbol.difference:
502606
case TestSymbol.passWithKnownIssue:
503-
return `${ansiEscapeCodePrefix}90m${symbol}${resetANSIEscapeCode}`;
607+
return `${SymbolRenderer.ansiEscapeCodePrefix}90m${symbol}${SymbolRenderer.resetANSIEscapeCode}`;
504608
case TestSymbol.pass:
505-
return `${ansiEscapeCodePrefix}92m${symbol}${resetANSIEscapeCode}`;
609+
return `${SymbolRenderer.ansiEscapeCodePrefix}92m${symbol}${SymbolRenderer.resetANSIEscapeCode}`;
506610
case TestSymbol.fail:
507-
return `${ansiEscapeCodePrefix}91m${symbol}${resetANSIEscapeCode}`;
611+
return `${SymbolRenderer.ansiEscapeCodePrefix}91m${symbol}${SymbolRenderer.resetANSIEscapeCode}`;
508612
case TestSymbol.warning:
509-
return `${ansiEscapeCodePrefix}93m${symbol}${resetANSIEscapeCode}`;
510-
case TestSymbol.details:
613+
return `${SymbolRenderer.ansiEscapeCodePrefix}93m${symbol}${SymbolRenderer.resetANSIEscapeCode}`;
614+
case TestSymbol.none:
511615
default:
512616
return symbol;
513617
}

Diff for: ‎test/suite/testexplorer/SwiftTestingOutputParser.test.ts

+23-11
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
SourceLocation,
2525
TestSymbol,
2626
SymbolRenderer,
27+
MessageRenderer,
2728
} from "../../../src/TestExplorer/TestParsers/SwiftTestingOutputParser";
2829
import { TestRunState, TestStatus } from "./MockTestRunState";
2930
import { Readable } from "stream";
@@ -114,7 +115,7 @@ suite("SwiftTestingOutputParser Suite", () => {
114115
]);
115116
});
116117

117-
test("Failed test with one issue", async () => {
118+
async function performTestFailure(messages: EventMessage[]) {
118119
const testRunState = new TestRunState(["MyTests.MyTests/testFail()"], true);
119120
const issueLocation = {
120121
_filePath: "file:///some/file.swift",
@@ -124,25 +125,23 @@ suite("SwiftTestingOutputParser Suite", () => {
124125
const events = new TestEventStream([
125126
testEvent("runStarted"),
126127
testEvent("testCaseStarted", "MyTests.MyTests/testFail()"),
127-
testEvent(
128-
"issueRecorded",
129-
"MyTests.MyTests/testFail()",
130-
[{ text: "Expectation failed: bar == foo", symbol: TestSymbol.fail }],
131-
issueLocation
132-
),
128+
testEvent("issueRecorded", "MyTests.MyTests/testFail()", messages, issueLocation),
133129
testEvent("testCaseEnded", "MyTests.MyTests/testFail()"),
134130
testEvent("runEnded"),
135131
]);
136132

137133
await outputParser.watch("file:///mock/named/pipe", testRunState, events);
138134

135+
const renderedMessages = messages.map(message => MessageRenderer.render(message));
136+
const fullFailureMessage = renderedMessages.join("\n");
137+
139138
assert.deepEqual(testRunState.tests, [
140139
{
141140
name: "MyTests.MyTests/testFail()",
142141
status: TestStatus.failed,
143142
issues: [
144143
{
145-
message: "Expectation failed: bar == foo",
144+
message: fullFailureMessage,
146145
location: new vscode.Location(
147146
vscode.Uri.file(issueLocation._filePath),
148147
new vscode.Position(issueLocation.line - 1, issueLocation?.column ?? 0)
@@ -154,11 +153,24 @@ suite("SwiftTestingOutputParser Suite", () => {
154153
timing: {
155154
timestamp: 0,
156155
},
157-
output: [
158-
`\u001b[91m${SymbolRenderer.symbol(TestSymbol.fail)}\u001b[0m Expectation failed: bar == foo\r\n`,
159-
],
156+
output: renderedMessages.map(message => `${message}\r\n`),
160157
},
161158
]);
159+
}
160+
161+
test("Failed with an issue that has a comment", async () => {
162+
await performTestFailure([
163+
{ text: "Expectation failed: bar == foo", symbol: TestSymbol.fail },
164+
{ symbol: TestSymbol.details, text: "// One" },
165+
{ symbol: TestSymbol.details, text: "// Two" },
166+
{ symbol: TestSymbol.details, text: "// Three" },
167+
]);
168+
});
169+
170+
test("Failed test with one issue", async () => {
171+
await performTestFailure([
172+
{ text: "Expectation failed: bar == foo", symbol: TestSymbol.fail },
173+
]);
162174
});
163175

164176
test("Parameterized test", async () => {

Diff for: ‎test/suite/testexplorer/TestExplorerIntegration.test.ts

+32-6
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ import { TestRunProxy } from "../../../src/TestExplorer/TestRunner";
3131
import { Version } from "../../../src/utilities/version";
3232
import { TestKind } from "../../../src/TestExplorer/TestKind";
3333
import { mockNamespace } from "../../unit-tests/MockUtils";
34+
import {
35+
MessageRenderer,
36+
TestSymbol,
37+
} from "../../../src/TestExplorer/TestParsers/SwiftTestingOutputParser";
3438

3539
suite("Test Explorer Suite", function () {
3640
const MAX_TEST_RUN_TIME_MINUTES = 5;
@@ -196,7 +200,12 @@ suite("Test Explorer Suite", function () {
196200
failed: [
197201
{
198202
test: "PackageTests.testWithKnownIssueAndUnknownIssue()",
199-
issues: ["Expectation failed: 2 == 3"],
203+
issues: [
204+
MessageRenderer.render({
205+
symbol: TestSymbol.fail,
206+
text: "Expectation failed: 2 == 3",
207+
}),
208+
],
200209
},
201210
],
202211
});
@@ -387,7 +396,12 @@ suite("Test Explorer Suite", function () {
387396
failed: [
388397
{
389398
test: "PackageTests.topLevelTestFailing()",
390-
issues: ["Expectation failed: 1 == 2"],
399+
issues: [
400+
MessageRenderer.render({
401+
symbol: TestSymbol.fail,
402+
text: "Expectation failed: 1 == 2",
403+
}),
404+
],
391405
},
392406
],
393407
});
@@ -399,13 +413,16 @@ suite("Test Explorer Suite", function () {
399413
runProfile,
400414
"PackageTests.MixedSwiftTestingSuite"
401415
);
416+
402417
assertTestResults(testRun, {
403418
passed: ["PackageTests.MixedSwiftTestingSuite/testPassing()"],
404419
skipped: ["PackageTests.MixedSwiftTestingSuite/testDisabled()"],
405420
failed: [
406421
{
407422
test: "PackageTests.MixedSwiftTestingSuite/testFailing()",
408-
issues: ["testFailing() \u{203A} Expectation failed: 1 == 2"],
423+
issues: [
424+
`testFailing() \u{203A} ${MessageRenderer.render({ symbol: TestSymbol.fail, text: "Expectation failed: 1 == 2" })}`,
425+
],
409426
},
410427
{
411428
issues: [],
@@ -429,7 +446,12 @@ suite("Test Explorer Suite", function () {
429446
],
430447
failed: [
431448
{
432-
issues: ["2 \u{203A} Expectation failed: (arg → 2) != 2"],
449+
issues: [
450+
`2 \u{203A} ${MessageRenderer.render({
451+
symbol: TestSymbol.fail,
452+
text: "Expectation failed: (arg → 2) != 2",
453+
})}`,
454+
],
433455
test: "PackageTests.parameterizedTest(_:)/PackageTests.swift:51:2/argumentIDs: Optional([Testing.Test.Case.Argument.ID(bytes: [50])])",
434456
},
435457
{
@@ -453,7 +475,9 @@ suite("Test Explorer Suite", function () {
453475
failed: [
454476
{
455477
test: "PackageTests.MixedSwiftTestingSuite/testFailing()",
456-
issues: ["testFailing() \u{203A} Expectation failed: 1 == 2"],
478+
issues: [
479+
`testFailing() \u{203A} ${MessageRenderer.render({ symbol: TestSymbol.fail, text: "Expectation failed: 1 == 2" })}`,
480+
],
457481
},
458482
{
459483
issues: [],
@@ -480,7 +504,9 @@ suite("Test Explorer Suite", function () {
480504
failed: [
481505
{
482506
test: "PackageTests.MixedSwiftTestingSuite/testFailing()",
483-
issues: ["testFailing() \u{203A} Expectation failed: 1 == 2"],
507+
issues: [
508+
`testFailing() \u{203A} ${MessageRenderer.render({ symbol: TestSymbol.fail, text: "Expectation failed: 1 == 2" })}`,
509+
],
484510
},
485511
{
486512
issues: [],

0 commit comments

Comments
 (0)