Skip to content

Commit ffe6773

Browse files
recca0120recca0120
authored
refactor: replace RawOutputObserver with DebugOutputObserver (#403)
* chore: add .worktrees to gitignore * refactor: replace RawOutputObserver with DebugOutputObserver Replace raw teamcity line output with structured diagnostic info to help diagnose issues like Paratest ID mismatches. DebugOutputObserver outputs: - Executed command - Queue keys (testItemById) at run start - Per-test: event name, id, locationHint, file, find() ✓/✗ result * refactor: remove redundant status variable in logResult * refactor: reorder TestResultObserver constructor params to (queue, testRun, testItemById)
1 parent 34ce536 commit ffe6773

10 files changed

Lines changed: 309 additions & 52 deletions

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,4 @@ node_modules/
2222
# Personal / local
2323
.claude/
2424
CLAUDE.md
25+
.worktrees/

‎packages/extension/src/Observers/DebugOutputObserver.test.ts‎

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
import {
2+
Configuration,
3+
type Path,
4+
PathReplacer,
5+
ProcessBuilder,
6+
TeamcityEvent,
7+
type TestFailed,
8+
type TestFinished,
9+
type TestIgnored,
10+
type TestStarted,
11+
} from '@vscode-phpunit/phpunit';
12+
import { beforeEach, describe, expect, it } from 'vitest';
13+
import type { TestItem } from 'vscode';
14+
import * as vscode from 'vscode';
15+
import { DebugOutputObserver } from './DebugOutputObserver';
16+
17+
const createTestItem = (id: string): TestItem => ({ id }) as TestItem;
18+
19+
const createBuilder = (command = 'php') => {
20+
const config = new Configuration({ php: command });
21+
const options = { cwd: '.' };
22+
return new ProcessBuilder(
23+
config,
24+
options,
25+
new PathReplacer(options, config.get('paths') as Path),
26+
);
27+
};
28+
29+
const makeStarted = (id: string, locationHint: string, file: string): TestStarted => ({
30+
event: TeamcityEvent.testStarted,
31+
name: id.split('::').pop() ?? id,
32+
flowId: 1,
33+
id,
34+
file,
35+
locationHint,
36+
});
37+
38+
const makeFinished = (id: string, locationHint: string, file: string): TestFinished => ({
39+
event: TeamcityEvent.testFinished,
40+
name: id.split('::').pop() ?? id,
41+
flowId: 1,
42+
id,
43+
file,
44+
locationHint,
45+
duration: 10,
46+
});
47+
48+
const makeFailed = (id: string, locationHint: string, file: string): TestFailed => ({
49+
event: TeamcityEvent.testFailed,
50+
name: id.split('::').pop() ?? id,
51+
flowId: 1,
52+
id,
53+
file,
54+
locationHint,
55+
message: 'assertion failed',
56+
details: [],
57+
duration: 10,
58+
});
59+
60+
const makeIgnored = (id: string, locationHint: string, file: string): TestIgnored => ({
61+
event: TeamcityEvent.testIgnored,
62+
name: id.split('::').pop() ?? id,
63+
flowId: 1,
64+
id,
65+
file,
66+
locationHint,
67+
message: 'skipped',
68+
details: [],
69+
duration: 0,
70+
});
71+
72+
describe('DebugOutputObserver', () => {
73+
const ID = 'Tests\\Feature\\ExampleTest::test_foo';
74+
const LOCATION_HINT = `php_qn:///var/www/html/tests/Feature/ExampleTest.php::Tests\\Feature\\ExampleTest::test_foo`;
75+
const FILE = '/local/tests/Feature/ExampleTest.php';
76+
77+
// biome-ignore lint/suspicious/noExplicitAny: test mock type
78+
let outputChannel: any;
79+
let configuration: Configuration;
80+
81+
beforeEach(() => {
82+
outputChannel = vscode.window.createOutputChannel('PHPUnit Debug');
83+
configuration = new Configuration({ clearDebugOutputOnRun: true });
84+
});
85+
86+
describe('run()', () => {
87+
it('logs the command', () => {
88+
const observer = new DebugOutputObserver(outputChannel, configuration, new Map());
89+
observer.run(createBuilder('php'));
90+
91+
expect(outputChannel.appendLine).toHaveBeenCalledWith(expect.stringContaining('php'));
92+
});
93+
94+
it('logs queue keys when testItemById is not empty', () => {
95+
const testItemById = new Map([[ID, createTestItem(ID)]]);
96+
const observer = new DebugOutputObserver(outputChannel, configuration, testItemById);
97+
observer.run(createBuilder('php'));
98+
99+
expect(outputChannel.appendLine).toHaveBeenCalledWith(
100+
expect.stringContaining('[Queue]'),
101+
);
102+
expect(outputChannel.appendLine).toHaveBeenCalledWith(expect.stringContaining(ID));
103+
});
104+
105+
it('does not log queue section when testItemById is empty', () => {
106+
const observer = new DebugOutputObserver(outputChannel, configuration, new Map());
107+
observer.run(createBuilder('php'));
108+
109+
expect(outputChannel.appendLine).not.toHaveBeenCalledWith(
110+
expect.stringContaining('[Queue]'),
111+
);
112+
});
113+
114+
it('clears output once per request when clearDebugOutputOnRun is true', () => {
115+
const observer = new DebugOutputObserver(outputChannel, configuration, new Map());
116+
observer.run(createBuilder('cmd-1'));
117+
observer.run(createBuilder('cmd-2'));
118+
119+
expect(outputChannel.clear).toHaveBeenCalledTimes(1);
120+
});
121+
});
122+
123+
describe('testStarted()', () => {
124+
it('logs ✓ found when testItem exists', () => {
125+
const testItemById = new Map([[ID, createTestItem(ID)]]);
126+
const observer = new DebugOutputObserver(outputChannel, configuration, testItemById);
127+
observer.testStarted?.(makeStarted(ID, LOCATION_HINT, FILE));
128+
129+
expect(outputChannel.appendLine).toHaveBeenCalledWith(
130+
expect.stringContaining('[testStarted]'),
131+
);
132+
expect(outputChannel.appendLine).toHaveBeenCalledWith(expect.stringContaining('✓'));
133+
expect(outputChannel.appendLine).toHaveBeenCalledWith(expect.stringContaining(ID));
134+
expect(outputChannel.appendLine).toHaveBeenCalledWith(
135+
expect.stringContaining(LOCATION_HINT),
136+
);
137+
});
138+
139+
it('logs ✗ not found when testItem missing', () => {
140+
const observer = new DebugOutputObserver(outputChannel, configuration, new Map());
141+
observer.testStarted?.(makeStarted(ID, LOCATION_HINT, FILE));
142+
143+
expect(outputChannel.appendLine).toHaveBeenCalledWith(
144+
expect.stringContaining('[testStarted]'),
145+
);
146+
expect(outputChannel.appendLine).toHaveBeenCalledWith(expect.stringContaining('✗'));
147+
});
148+
});
149+
150+
describe('testFinished()', () => {
151+
it('logs ✓ found when testItem exists', () => {
152+
const testItemById = new Map([[ID, createTestItem(ID)]]);
153+
const observer = new DebugOutputObserver(outputChannel, configuration, testItemById);
154+
observer.testFinished?.(makeFinished(ID, LOCATION_HINT, FILE));
155+
156+
expect(outputChannel.appendLine).toHaveBeenCalledWith(
157+
expect.stringContaining('[testFinished]'),
158+
);
159+
expect(outputChannel.appendLine).toHaveBeenCalledWith(expect.stringContaining('✓'));
160+
});
161+
162+
it('logs ✗ not found when testItem missing', () => {
163+
const observer = new DebugOutputObserver(outputChannel, configuration, new Map());
164+
observer.testFinished?.(makeFinished(ID, LOCATION_HINT, FILE));
165+
166+
expect(outputChannel.appendLine).toHaveBeenCalledWith(
167+
expect.stringContaining('[testFinished]'),
168+
);
169+
expect(outputChannel.appendLine).toHaveBeenCalledWith(expect.stringContaining('✗'));
170+
});
171+
});
172+
173+
describe('testFailed()', () => {
174+
it('logs ✗ not found when testItem missing', () => {
175+
const observer = new DebugOutputObserver(outputChannel, configuration, new Map());
176+
observer.testFailed?.(makeFailed(ID, LOCATION_HINT, FILE));
177+
178+
expect(outputChannel.appendLine).toHaveBeenCalledWith(
179+
expect.stringContaining('[testFailed]'),
180+
);
181+
expect(outputChannel.appendLine).toHaveBeenCalledWith(expect.stringContaining('✗'));
182+
});
183+
});
184+
185+
describe('testIgnored()', () => {
186+
it('logs ✗ not found when testItem missing', () => {
187+
const observer = new DebugOutputObserver(outputChannel, configuration, new Map());
188+
observer.testIgnored?.(makeIgnored(ID, LOCATION_HINT, FILE));
189+
190+
expect(outputChannel.appendLine).toHaveBeenCalledWith(
191+
expect.stringContaining('[testIgnored]'),
192+
);
193+
expect(outputChannel.appendLine).toHaveBeenCalledWith(expect.stringContaining('✗'));
194+
});
195+
});
196+
197+
describe('line()', () => {
198+
it('does not implement line handler', () => {
199+
const observer = new DebugOutputObserver(outputChannel, configuration, new Map());
200+
201+
expect('line' in observer).toBe(false);
202+
});
203+
});
204+
});

‎packages/extension/src/Observers/DebugOutputObserver.ts‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import type {
2+
IConfiguration,
3+
ProcessBuilder,
4+
TestFailed,
5+
TestFinished,
6+
TestIgnored,
7+
TestRunnerObserver,
8+
TestStarted,
9+
} from '@vscode-phpunit/phpunit';
10+
import stripAnsi from 'strip-ansi';
11+
import type { OutputChannel, TestItem } from 'vscode';
12+
13+
export class DebugOutputObserver implements TestRunnerObserver {
14+
private hasClearedCurrentRequest = false;
15+
16+
constructor(
17+
private writer: OutputChannel,
18+
private configuration: IConfiguration,
19+
private testItemById: Map<string, TestItem>,
20+
) {}
21+
22+
run(builder: ProcessBuilder): void {
23+
this.clearOutputOnRun();
24+
this.writer.appendLine(stripAnsi(builder.toString()));
25+
26+
if (this.testItemById.size > 0) {
27+
this.writer.appendLine('[Queue]');
28+
for (const id of this.testItemById.keys()) {
29+
this.writer.appendLine(` ${id}`);
30+
}
31+
}
32+
}
33+
34+
error(error: string): void {
35+
this.writer.appendLine(stripAnsi(error));
36+
}
37+
38+
testStarted(result: TestStarted): void {
39+
this.logResult('testStarted', result);
40+
}
41+
42+
testFinished(result: TestFinished): void {
43+
this.logResult('testFinished', result);
44+
}
45+
46+
testFailed(result: TestFailed): void {
47+
this.logResult('testFailed', result);
48+
}
49+
50+
testIgnored(result: TestIgnored): void {
51+
this.logResult('testIgnored', result);
52+
}
53+
54+
private logResult(
55+
event: string,
56+
result: { id: string; name: string; locationHint: string; file?: string },
57+
) {
58+
const found = this.testItemById.has(result.id);
59+
this.writer.appendLine(`[${event}] ${result.name}`);
60+
this.writer.appendLine(` id : ${result.id}`);
61+
this.writer.appendLine(` locationHint : ${result.locationHint}`);
62+
if (result.file) {
63+
this.writer.appendLine(` file : ${result.file}`);
64+
}
65+
this.writer.appendLine(` find() : ${found ? '✓ found' : '✗ not found'}`);
66+
}
67+
68+
private clearOutputOnRun() {
69+
if (this.hasClearedCurrentRequest) {
70+
return;
71+
}
72+
73+
if (this.configuration.get('clearDebugOutputOnRun') === true) {
74+
this.writer.clear();
75+
}
76+
77+
this.hasClearedCurrentRequest = true;
78+
}
79+
}

‎packages/extension/src/Observers/ObserverFactory.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ import { Configuration } from '../Configuration';
1414
import { TestCollection } from '../TestCollection/TestCollection';
1515
import { TYPES } from '../types';
1616
import { DatasetObserver } from './DatasetObserver';
17+
import { DebugOutputObserver } from './DebugOutputObserver';
1718
import { ErrorDialogObserver } from './ErrorDialogObserver';
1819
import { PrinterObserver } from './PrinterObserver';
19-
import { RawOutputObserver } from './RawOutputObserver';
2020
import { TestResultObserver } from './TestResultObserver';
2121
import { TestRunWriter } from './Writers';
2222

@@ -38,8 +38,8 @@ export class ObserverFactory {
3838

3939
return [
4040
new DatasetObserver(this.testCollection, testItemById),
41-
new TestResultObserver(testItemById, queue, testRun),
42-
new RawOutputObserver(this.outputChannel, this.configuration),
41+
new TestResultObserver(queue, testRun, testItemById),
42+
new DebugOutputObserver(this.outputChannel, this.configuration, testItemById),
4343
new PrinterObserver(
4444
new TestRunWriter(testRun, testItemById),
4545
new Printer(this.phpUnitXML, format),

‎packages/extension/src/Observers/RawOutputObserver.test.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,16 @@ import { detectPhpUnitStubs, phpUnitProject } from '@vscode-phpunit/phpunit/test
99
import { beforeEach, describe, expect, it, type Mock } from 'vitest';
1010
import type { OutputChannel } from 'vscode';
1111
import * as vscode from 'vscode';
12-
import { RawOutputObserver } from './RawOutputObserver';
12+
import { DebugOutputObserver } from './DebugOutputObserver';
1313

14-
describe('RawOutputObserver clear behavior', () => {
14+
describe('DebugOutputObserver clear behavior', () => {
1515
const createObserver = (config: Record<string, unknown> = {}) => {
1616
const outputChannel = vscode.window.createOutputChannel('phpunit');
1717
const configuration = new Configuration({
1818
clearDebugOutputOnRun: true,
1919
...config,
2020
});
21-
const observer = new RawOutputObserver(outputChannel, configuration);
21+
const observer = new DebugOutputObserver(outputChannel, configuration, new Map());
2222

2323
return { observer, outputChannel };
2424
};
@@ -45,8 +45,8 @@ describe('RawOutputObserver clear behavior', () => {
4545
it('each observer instance clears independently', () => {
4646
const outputChannel = vscode.window.createOutputChannel('phpunit');
4747
const configuration = new Configuration({ clearDebugOutputOnRun: true });
48-
const observer1 = new RawOutputObserver(outputChannel, configuration);
49-
const observer2 = new RawOutputObserver(outputChannel, configuration);
48+
const observer1 = new DebugOutputObserver(outputChannel, configuration, new Map());
49+
const observer2 = new DebugOutputObserver(outputChannel, configuration, new Map());
5050

5151
observer1.run(createBuilder('command-1'));
5252
observer2.run(createBuilder('command-2'));
@@ -55,7 +55,7 @@ describe('RawOutputObserver clear behavior', () => {
5555
});
5656
});
5757

58-
describe.each(detectPhpUnitStubs())('RawOutputObserver on $name (PHPUnit $phpUnitVersion)', ({
58+
describe.each(detectPhpUnitStubs())('DebugOutputObserver on $name (PHPUnit $phpUnitVersion)', ({
5959
root,
6060
binary,
6161
args: stubArgs,
@@ -72,7 +72,7 @@ describe.each(detectPhpUnitStubs())('RawOutputObserver on $name (PHPUnit $phpUni
7272
});
7373
testRunner = new TestRunner();
7474
const outputChannel = vscode.window.createOutputChannel('phpunit');
75-
const observer = new RawOutputObserver(outputChannel, configuration);
75+
const observer = new DebugOutputObserver(outputChannel, configuration, new Map());
7676
testRunner.observe(observer);
7777
});
7878

‎packages/extension/src/Observers/RawOutputObserver.ts‎

Lines changed: 0 additions & 37 deletions
This file was deleted.

0 commit comments

Comments
 (0)