Skip to content

Commit 474ef23

Browse files
committed
fix: route debug output at test run level
Correlate debug sessions with a unique launch marker, mirror non-telemetry DAP output at run level, and preserve structured TestNG runner errors while suppressing control protocol noise. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8db89d6b-bcb8-42d2-9f14-78aab176fcd0
1 parent 211ed2f commit 474ef23

6 files changed

Lines changed: 196 additions & 77 deletions

File tree

src/runners/baseRunner/BaseRunner.ts

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license.
33

44
import * as iconv from 'iconv-lite';
5+
import { randomUUID } from 'crypto';
56
import { AddressInfo, createServer, Server, Socket } from 'net';
67
import * as os from 'os';
78
import { CancellationToken, debug, DebugAdapterTracker, DebugConfiguration, DebugSession, Disposable, ProviderResult } from 'vscode';
@@ -12,6 +13,8 @@ import { ITestRunnerInternal } from '../ITestRunner';
1213
import { RunnerResultAnalyzer } from './RunnerResultAnalyzer';
1314
import { IExecutionConfig, IRunTestContext } from '../../java-test-runner.api';
1415

16+
const JAVA_TEST_RUN_ID: string = '__javaTestRunId';
17+
1518
export abstract class BaseRunner implements ITestRunnerInternal {
1619
protected server: Server;
1720
protected socket: Socket;
@@ -56,37 +59,26 @@ export abstract class BaseRunner implements ITestRunnerInternal {
5659
// So we force to use internal console here to make sure the session is still under debugger's control.
5760
launchConfiguration.console = 'internalConsole';
5861

59-
// The debuggee's stdout/stderr are delivered by java-debug as standard DAP `output` events,
60-
// which by default only surface in the Debug Console. Attach a tracker to the test's own debug
61-
// session and forward those output events into the Test Results view, so the program output shows
62-
// up next to the test results instead of being split across two separate surfaces.
62+
const testRunId: string = randomUUID();
63+
launchConfiguration[JAVA_TEST_RUN_ID] = testRunId;
64+
const isTestSession: (session: DebugSession) => boolean = (session: DebugSession): boolean =>
65+
session.configuration[JAVA_TEST_RUN_ID] === testRunId;
66+
67+
// Mirror the test session's user-visible Debug Console output in Test Results.
6368
this.disposables.push(debug.registerDebugAdapterTrackerFactory('java', {
6469
createDebugAdapterTracker: (session: DebugSession): ProviderResult<DebugAdapterTracker> => {
65-
if (session.name !== launchConfiguration.name) {
70+
if (!isTestSession(session)) {
6671
return undefined;
6772
}
6873
return {
69-
onDidSendMessage: (message: any): void => {
70-
if (message?.type === 'event' && message.event === 'output') {
71-
const category: string | undefined = message.body?.category;
72-
// `telemetry` output events are not meant for the user.
73-
if (category === 'telemetry') {
74-
return;
75-
}
76-
const output: string | undefined = message.body?.output;
77-
if (output) {
78-
// Let the analyzer attribute the output to the running test when possible.
79-
this.runnerResultAnalyzer.appendProgramOutput(output);
80-
}
81-
}
82-
},
74+
onDidSendMessage: (message: any): void => this.handleDebugAdapterMessage(message),
8375
};
8476
},
8577
}));
8678

8779
let debugSession: DebugSession | undefined;
8880
this.disposables.push(debug.onDidStartDebugSession((session: DebugSession) => {
89-
if (session.name === launchConfiguration.name) {
81+
if (!debugSession && isTestSession(session)) {
9082
debugSession = session;
9183
}
9284
}));
@@ -108,7 +100,7 @@ export abstract class BaseRunner implements ITestRunnerInternal {
108100
return await new Promise<void>((resolve: () => void): void => {
109101
this.disposables.push(
110102
debug.onDidTerminateDebugSession((session: DebugSession): void => {
111-
if (launchConfiguration.name === session.name) {
103+
if (session.id === debugSession?.id) {
112104
debugSession = undefined;
113105
this.tearDown();
114106
if (data.length > 0) {
@@ -125,6 +117,19 @@ export abstract class BaseRunner implements ITestRunnerInternal {
125117
}));
126118
}
127119

120+
protected handleDebugAdapterMessage(message: any): void {
121+
if (message?.type !== 'event' || message.event !== 'output' || message.body?.category === 'telemetry') {
122+
return;
123+
}
124+
125+
const output: unknown = message.body?.output;
126+
if (typeof output !== 'string' || output.length === 0) {
127+
return;
128+
}
129+
130+
this.testContext.testRun.appendOutput(output.replace(/\r?\n/g, '\r\n'));
131+
}
132+
128133
public async tearDown(): Promise<void> {
129134
try {
130135
if (this.socket) {

src/runners/baseRunner/RunnerResultAnalyzer.ts

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -10,45 +10,12 @@ export abstract class RunnerResultAnalyzer {
1010
// Track parent test item states to update them when all children complete
1111
protected parentStates: Map<TestItem, ParentItemState> = new Map();
1212

13-
// Test items that are currently executing. Used to attribute program output
14-
// (captured from the debug session) to the running test when it is unambiguous.
15-
private runningItems: Set<TestItem> = new Set();
16-
1713
constructor(protected testContext: IRunTestContext) { }
1814

1915
public abstract analyzeData(data: string): void;
2016
public abstract processData(data: string): void;
2117
protected testMessageLocation: Location | undefined;
2218

23-
/**
24-
* Record that a test item has started executing.
25-
*/
26-
protected markItemStarted(item: TestItem): void {
27-
this.runningItems.add(item);
28-
}
29-
30-
/**
31-
* Record that a test item has finished executing.
32-
*/
33-
protected markItemFinished(item: TestItem): void {
34-
this.runningItems.delete(item);
35-
}
36-
37-
/**
38-
* Forward program output (captured from the test's debug session as DAP `output`
39-
* events) into the Test Results view. When exactly one test is currently running,
40-
* the output is attributed to that test item so it shows up under the test in the
41-
* explorer; otherwise (idle, or several tests running in parallel where attribution
42-
* would only be a guess) it is appended to the run as a whole.
43-
*/
44-
public appendProgramOutput(output: string): void {
45-
const normalized: string = output.replace(/\r?\n/g, '\r\n');
46-
const item: TestItem | undefined = this.runningItems.size === 1
47-
? this.runningItems.values().next().value
48-
: undefined;
49-
this.testContext.testRun.appendOutput(normalized, undefined, item);
50-
}
51-
5219
/**
5320
* Return a string array which contains the stacktraces that need to be filtered.
5421
* All the stacktraces which include the element in the return array will be removed.

src/runners/junitRunner/JUnitRunnerResultAnalyzer.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer {
7676
this.setDurationAtStart(this.getCurrentState(item));
7777
setTestState(this.testContext.testRun, item, this.getCurrentState(item).resultState);
7878
this.updateParentOnChildStart(item);
79-
this.markItemStarted(item);
8079
} else if (data.startsWith(MessageId.TestEnd)) {
8180
const item: TestItem | undefined = this.getTestItem(data.substr(MessageId.TestEnd.length));
8281
if (!item) {
@@ -86,7 +85,6 @@ export class JUnitRunnerResultAnalyzer extends RunnerResultAnalyzer {
8685
this.calcDurationAtEnd(currentState);
8786
this.determineResultStateAtEnd(data, currentState);
8887
setTestState(this.testContext.testRun, item, currentState.resultState, undefined, currentState.duration);
89-
this.markItemFinished(item);
9088
const itemData: ITestItemData | undefined = dataCache.get(item);
9189
if (itemData?.testLevel === TestLevel.Method) {
9290
this.updateParentOnChildComplete(item, currentState.resultState);

src/runners/testngRunner/TestNGRunnerResultAnalyzer.ts

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { IRunTestContext, TestLevel, TestResultState } from '../../java-test-run
1010
const TEST_START: string = 'testStarted';
1111
const TEST_FAIL: string = 'testFailed';
1212
const TEST_FINISH: string = 'testFinished';
13+
const TEST_ERROR: string = 'error';
1314

1415
export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer {
1516

@@ -47,15 +48,25 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer {
4748
try {
4849
this.processData(match[1]);
4950
} catch (error) {
50-
this.testContext.testRun.appendOutput(`[ERROR] Failed to parse output data: ${match[1]}\n`);
51+
this.testContext.testRun.appendOutput(`[ERROR] Failed to parse output data: ${match[1]}\r\n`);
5152
}
5253
}
5354
}
5455

5556
public processData(data: string): void {
5657
const outputData: ITestNGOutputData = JSON.parse(data) as ITestNGOutputData;
5758

58-
const id: string = `${this.projectName}@${outputData.attributes.name}`;
59+
if (outputData.name === TEST_ERROR) {
60+
this.processRunnerError(outputData.attributes);
61+
return;
62+
}
63+
64+
const attributes: ITestNGAttributes | undefined = outputData.attributes;
65+
if (!attributes?.name) {
66+
return;
67+
}
68+
69+
const id: string = `${this.projectName}@${attributes.name}`;
5970
if (outputData.name === TEST_START) {
6071
this.initializeCache();
6172
const item: TestItem | undefined = this.getTestItem(id);
@@ -66,7 +77,6 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer {
6677
this.currentTestState = TestResultState.Running;
6778
this.testContext.testRun.started(item);
6879
this.updateParentOnChildStart(item);
69-
this.markItemStarted(item);
7080
} else if (outputData.name === TEST_FAIL) {
7181
const item: TestItem | undefined = this.getTestItem(id);
7282
if (!item) {
@@ -75,11 +85,11 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer {
7585
this.currentTestState = TestResultState.Failed;
7686
const testMessages: TestMessage[] = [];
7787

78-
if (outputData.attributes.trace) {
88+
if (attributes.trace) {
7989
const markdownTrace: MarkdownString = new MarkdownString();
8090
markdownTrace.isTrusted = true;
8191
markdownTrace.supportHtml = true;
82-
for (const line of outputData.attributes.trace.split(/\r?\n/)) {
92+
for (const line of attributes.trace.split(/\r?\n/)) {
8393
this.processStackTrace(line, markdownTrace, this.currentItem, this.projectName);
8494
}
8595

@@ -92,19 +102,18 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer {
92102
}
93103
testMessages.push(testMessage);
94104
}
95-
const duration: number = Number.parseInt(outputData.attributes.duration, 10);
105+
const duration: number | undefined = this.parseDuration(attributes.duration);
96106
setTestState(this.testContext.testRun, item, this.currentTestState, testMessages, duration);
97107
} else if (outputData.name === TEST_FINISH) {
98-
const item: TestItem | undefined = this.getTestItem(data);
108+
const item: TestItem | undefined = this.getTestItem(id);
99109
if (!item) {
100110
return;
101111
}
102112
if (this.currentTestState === TestResultState.Running) {
103113
this.currentTestState = TestResultState.Passed;
104114
}
105-
const duration: number = Number.parseInt(outputData.attributes.duration, 10);
115+
const duration: number | undefined = this.parseDuration(attributes.duration);
106116
setTestState(this.testContext.testRun, item, this.currentTestState, undefined, duration);
107-
this.markItemFinished(item);
108117
const itemData: ITestItemData | undefined = dataCache.get(item);
109118
if (itemData?.testLevel === TestLevel.Method) {
110119
this.updateParentOnChildComplete(item, this.currentTestState);
@@ -126,6 +135,26 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer {
126135
this.currentItem = undefined;
127136
}
128137

138+
private processRunnerError(attributes: ITestNGAttributes | undefined): void {
139+
let message: string = attributes?.message || 'Failed to run TestNG tests.';
140+
if (attributes?.trace) {
141+
message += `\n${attributes.trace}`;
142+
}
143+
const testMessage: TestMessage = new TestMessage(message);
144+
for (const item of this.testContext.testItems) {
145+
this.testContext.testRun.errored(item, testMessage);
146+
}
147+
}
148+
149+
private parseDuration(duration: string | undefined): number | undefined {
150+
if (!duration) {
151+
return undefined;
152+
}
153+
154+
const parsed: number = Number.parseInt(duration, 10);
155+
return Number.isNaN(parsed) ? undefined : parsed;
156+
}
157+
129158
protected getStacktraceFilter(): string[] {
130159
return [
131160
'com.microsoft.java.test.runner.',
@@ -142,20 +171,14 @@ export class TestNGRunnerResultAnalyzer extends RunnerResultAnalyzer {
142171
}
143172

144173
interface ITestNGOutputData {
145-
attributes: ITestNGAttributes;
146-
type: TestOutputType;
174+
attributes?: ITestNGAttributes;
147175
name: string;
148176
}
149177

150-
enum TestOutputType {
151-
Info,
152-
Error,
153-
}
154-
155178
interface ITestNGAttributes {
156-
name: string;
157-
duration: string;
158-
location: string;
159-
message: string;
160-
trace: string;
179+
name?: string;
180+
duration?: string;
181+
location?: string;
182+
message?: string;
183+
trace?: string;
161184
}

test/suite/TestNGAnalyzer.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license.
3+
4+
'use strict';
5+
6+
import * as assert from 'assert';
7+
import * as sinon from 'sinon';
8+
import { TestController, TestMessage, TestRunRequest, tests, workspace } from 'vscode';
9+
import { TestNGRunnerResultAnalyzer } from '../../src/runners/testngRunner/TestNGRunnerResultAnalyzer';
10+
import { IRunTestContext, TestKind } from '../../src/java-test-runner.api';
11+
import { generateTestItem } from './utils';
12+
13+
// tslint:disable: only-arrow-functions
14+
suite('TestNG Runner Analyzer Tests', () => {
15+
16+
let testController: TestController;
17+
18+
setup(() => {
19+
testController = tests.createTestController('testngTestController', 'Mock TestNG');
20+
});
21+
22+
teardown(() => {
23+
testController.dispose();
24+
});
25+
26+
test('surfaces runner errors as structured test errors', () => {
27+
const testItem = generateTestItem(testController, 'testng@example.SampleTest#test', TestKind.TestNG);
28+
const testRun = testController.createTestRun(new TestRunRequest([testItem], []));
29+
const erroredSpy = sinon.spy(testRun, 'errored');
30+
const runnerContext: IRunTestContext = {
31+
isDebug: false,
32+
kind: TestKind.TestNG,
33+
projectName: 'testng',
34+
testItems: [testItem],
35+
testRun,
36+
workspaceFolder: workspace.workspaceFolders?.[0]!,
37+
};
38+
const analyzer = new TestNGRunnerResultAnalyzer(runnerContext);
39+
const trace = 'java.lang.ClassNotFoundException: example.SampleTest';
40+
41+
analyzer.processData(JSON.stringify({
42+
name: 'error',
43+
attributes: {
44+
message: 'Failed to run TestNG tests',
45+
trace,
46+
},
47+
}));
48+
49+
sinon.assert.calledOnce(erroredSpy);
50+
sinon.assert.calledWith(erroredSpy, testItem, sinon.match.instanceOf(TestMessage));
51+
const testMessage = erroredSpy.firstCall.args[1] as TestMessage;
52+
assert.strictEqual(testMessage.message, `Failed to run TestNG tests\n${trace}`);
53+
});
54+
55+
test('ignores control messages without test attributes', () => {
56+
const testItem = generateTestItem(testController, 'testng@example.SampleTest#test', TestKind.TestNG);
57+
const testRun = testController.createTestRun(new TestRunRequest([testItem], []));
58+
const appendOutputSpy = sinon.spy(testRun, 'appendOutput');
59+
const runnerContext: IRunTestContext = {
60+
isDebug: false,
61+
kind: TestKind.TestNG,
62+
projectName: 'testng',
63+
testItems: [testItem],
64+
testRun,
65+
workspaceFolder: workspace.workspaceFolders?.[0]!,
66+
};
67+
const analyzer = new TestNGRunnerResultAnalyzer(runnerContext);
68+
69+
analyzer.analyzeData('@@<TestRunner-{"name":"reporterAttached"}-TestRunner>');
70+
71+
sinon.assert.notCalled(appendOutputSpy);
72+
});
73+
});

0 commit comments

Comments
 (0)