Skip to content

Commit 3d7ab62

Browse files
committed
fix: handle Pest v2 describe+dataset testSuiteStarted and Pest v3 truncated suite names
- PestV2Fixer: extend decodeEvaluable to handle → separator in describe block evaluable names (e.g. __pest_evaluable__something__→_it_should_detect_OK_but_does_not) - TestResultObserver: add prefix fallback for Pest v3 testSuiteStarted/Finished whose names are truncated due to mb_strrpos+substr byte/char mismatch; restricted to suite events only to avoid false positives
1 parent a8ef29d commit 3d7ab62

5 files changed

Lines changed: 163 additions & 14 deletions

File tree

packages/extension/src/Observers/TestResultObserver.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import type {
33
TestDefinition,
44
TestFailed,
55
TestFinished,
6+
TestSuiteFinished,
7+
TestSuiteStarted,
68
} from '@vscode-phpunit/phpunit';
79
import { beforeEach, describe, expect, it, vi } from 'vitest';
810
import {
@@ -189,6 +191,67 @@ describe('TestResultObserver', () => {
189191
expect(message.stackTrace).toBeUndefined();
190192
});
191193

194+
// Pest v3 bug: Str::beforeLast uses mb_strrpos (char offset) with substr (byte offset).
195+
// The → character (U+2192) is 3 UTF-8 bytes but 1 char, so testSuiteStarted/Finished names
196+
// are truncated by 2 bytes per → character.
197+
it('should find parent item via prefix match when Pest v3 truncates testSuiteStarted name', () => {
198+
const parentItem = ctrl.createTestItem(
199+
'tests/Unit/SampleTests.php::`something` \u2192 it should detect OK but does not',
200+
'it should detect OK but does not',
201+
Uri.file('/project/tests/SampleTests.php'),
202+
);
203+
const testItemById = buildTestItemById([testItem, parentItem]);
204+
const obs = new TestResultObserver(queue, testRun, testItemById);
205+
206+
obs.testSuiteStarted({
207+
event: 'testSuiteStarted' as unknown as TeamcityEvent,
208+
id: 'tests/Unit/SampleTests.php::`something` \u2192 it should detect OK but does n',
209+
flowId: 1,
210+
name: '`something` \u2192 it should detect OK but does n',
211+
} as unknown as TestSuiteStarted);
212+
213+
expect(testRun.started).toHaveBeenCalledWith(parentItem);
214+
});
215+
216+
it('should mark parent passed via prefix match when Pest v3 truncates testSuiteFinished name', () => {
217+
const parentItem = ctrl.createTestItem(
218+
'tests/Unit/SampleTests.php::`something` \u2192 it should detect OK but does not',
219+
'it should detect OK but does not',
220+
Uri.file('/project/tests/SampleTests.php'),
221+
);
222+
const testItemById = buildTestItemById([testItem, parentItem]);
223+
const obs = new TestResultObserver(queue, testRun, testItemById);
224+
225+
obs.testSuiteFinished({
226+
event: 'testSuiteFinished' as unknown as TeamcityEvent,
227+
id: 'tests/Unit/SampleTests.php::`something` \u2192 it should detect OK but does n',
228+
flowId: 1,
229+
name: '`something` \u2192 it should detect OK but does n',
230+
} as unknown as TestSuiteFinished);
231+
232+
expect(testRun.passed).toHaveBeenCalledWith(parentItem);
233+
});
234+
235+
it('should not use prefix match for regular testStarted events with → in id', () => {
236+
const parentItem = ctrl.createTestItem(
237+
'tests/Unit/ArchTest.php::preset \u2192 php ',
238+
'preset \u2192 php ',
239+
Uri.file('/project/tests/ArchTest.php'),
240+
);
241+
const testItemById = buildTestItemById([testItem, parentItem]);
242+
const obs = new TestResultObserver(queue, testRun, testItemById);
243+
244+
// runtime id has no trailing space — should NOT match via prefix fallback for testStarted
245+
obs.testStarted({
246+
event: 'testStarted' as unknown as TeamcityEvent,
247+
id: 'tests/Unit/ArchTest.php::preset \u2192 php',
248+
flowId: 1,
249+
name: 'preset \u2192 php',
250+
} as never);
251+
252+
expect(testRun.started).not.toHaveBeenCalledWith(parentItem);
253+
});
254+
192255
it('should not use TestMessage.diff when expected/actual are missing', () => {
193256
const diffSpy = vi.spyOn(TestMessage, 'diff');
194257

packages/extension/src/Observers/TestResultObserver.ts

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export class TestResultObserver implements TestRunnerObserver {
3838
}
3939

4040
testSuiteStarted(result: TestSuiteStarted): void {
41-
this.doRun(result, (test) => this.testRun.started(test));
41+
this.doRun(result, (test) => this.testRun.started(test), true);
4242
}
4343

4444
testStarted(result: TestStarted): void {
@@ -60,7 +60,7 @@ export class TestResultObserver implements TestRunnerObserver {
6060
}
6161

6262
testSuiteFinished(result: TestSuiteFinished): void {
63-
this.doRun(result, (test) => this.testRun.passed(test));
63+
this.doRun(result, (test) => this.testRun.passed(test), true);
6464
}
6565

6666
private message(result: TestFailed | TestIgnored, test: TestItem) {
@@ -100,20 +100,45 @@ export class TestResultObserver implements TestRunnerObserver {
100100
return message;
101101
}
102102

103-
private doRun(result: TestResult, updateTestRun: (testItem: TestItem) => void) {
104-
const testItem = this.find(result);
103+
private doRun(
104+
result: TestResult,
105+
updateTestRun: (testItem: TestItem) => void,
106+
usePrefixFallback = false,
107+
) {
108+
const testItem = this.find(result, usePrefixFallback);
105109
if (!testItem) {
106110
return;
107111
}
108112

109113
updateTestRun(testItem);
110114
}
111115

112-
private find(result: TestResult) {
116+
private find(result: TestResult, usePrefixFallback = false) {
113117
if (!('id' in result) || typeof result.id !== 'string') {
114118
return undefined;
115119
}
116120

117-
return this.testItemById.get(result.id);
121+
const exact = this.testItemById.get(result.id);
122+
if (exact) {
123+
return exact;
124+
}
125+
126+
// Pest v3 has a bug: Str::beforeLast uses mb_strrpos (char offset) with substr (byte offset).
127+
// The → character (U+2192) is 3 UTF-8 bytes but 1 char, so testSuiteStarted/Finished names
128+
// are truncated by 2 bytes per → character. The truncated ID is a prefix of the full parent ID.
129+
// Only apply to suite events to avoid false positives on regular test events.
130+
if (
131+
usePrefixFallback &&
132+
result.id.includes('\u2192') &&
133+
!result.id.includes(' with data set ')
134+
) {
135+
for (const [key, item] of this.testItemById) {
136+
if (key.startsWith(result.id) && !key.includes(' with data set ')) {
137+
return item;
138+
}
139+
}
140+
}
141+
142+
return undefined;
118143
}
119144
}

packages/phpunit/src/TestIdentifier/PestTestIdentifier.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ describe('PestTestIdentifier', () => {
7777
'Users\\path\\to\\tests\\Unit\\DatasetTest::__pest_evaluable_it_business_closed',
7878
'tests/Unit/DatasetTest.php::it business closed',
7979
],
80+
// describe('something', fn() => it('test', fn())->with([...]))
81+
// v2 testSuiteStarted uses evaluable encoding with → separator
82+
[
83+
'file:///path/to/tests/Unit/SampleTests.php',
84+
'Users\\path\\to\\tests\\Unit\\SampleTests::__pest_evaluable__something__\u2192_it_should_detect_OK_but_does_not',
85+
'tests/Unit/SampleTests.php::`something` \u2192 it should detect OK but does not',
86+
],
8087
])('fromLocationHint(%j, %j) → id: %s', (locationHint, name, expectedId) => {
8188
const result = transformer.fromLocationHint(locationHint, name);
8289
expect(result.id).toBe(expectedId);

packages/phpunit/src/TestIdentifier/PestV2Fixer.ts

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,58 @@ function hasPrefix(id?: string) {
44
return id?.includes(PREFIX) ?? false;
55
}
66

7-
function decodeEvaluable(encoded: string) {
8-
const idx = encoded.indexOf(PREFIX);
9-
if (idx === -1) {
7+
function decodeDescribePart(inner: string): string {
8+
const segments: string[] = [];
9+
let current = '';
10+
let i = 0;
11+
while (i < inner.length) {
12+
if (inner[i] === '_' && inner[i + 1] === '_') {
13+
current += '_';
14+
i += 2;
15+
} else if (inner[i] === '_') {
16+
segments.push(current);
17+
current = '';
18+
i++;
19+
} else {
20+
current += inner[i];
21+
i++;
22+
}
23+
}
24+
segments.push(current);
25+
const isFQN = segments.every((p) => /^[A-Z]/.test(p));
26+
return `\`${segments.join(isFQN ? '\\' : ' ')}\``;
27+
}
28+
29+
function decodeEvaluable(encoded: string): string {
30+
const prefixIdx = encoded.indexOf(PREFIX);
31+
if (prefixIdx === -1) {
1032
return encoded;
1133
}
1234

13-
const before = encoded.slice(0, idx);
14-
let method = encoded.slice(idx + PREFIX.length);
35+
const before = encoded.slice(0, prefixIdx);
36+
const methodFull = encoded.slice(prefixIdx + PREFIX.length);
37+
38+
const datasetIdx = methodFull.search(/\s+with\s+data\s+set\s+/);
39+
const methodPart = datasetIdx >= 0 ? methodFull.slice(0, datasetIdx) : methodFull;
40+
const datasetSuffix = datasetIdx >= 0 ? methodFull.slice(datasetIdx) : '';
41+
42+
const segments = methodPart.split('_\u2192_');
43+
if (segments.length < 2) {
44+
const decoded = methodPart.replace(/__|_/g, (m) => (m === '__' ? '_' : ' '));
45+
return before + decoded + datasetSuffix;
46+
}
47+
48+
const describeParts = segments.slice(0, -1);
49+
const testPart = segments[segments.length - 1];
50+
51+
const decodedDescribes = describeParts.map((part) => {
52+
const inner = part.replace(/^_/, '').replace(/_$/, '');
53+
return decodeDescribePart(inner);
54+
});
1555

16-
// reverse: single _ → space, double __ → literal _
17-
method = method.replace(/__|_/g, (m) => (m === '__' ? '_' : ' '));
56+
const decodedTest = testPart.replace(/__|_/g, (m) => (m === '__' ? '_' : ' '));
1857

19-
return before + method;
58+
return before + [...decodedDescribes, decodedTest].join(' \u2192 ') + datasetSuffix;
2059
}
2160

2261
export const PestV2Fixer = {
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
describe('something', function () {
4+
it('should detect OK but does not', function (int $number) {
5+
expect($number)->toBeGreaterThan(3);
6+
})->with([
7+
4,
8+
8,
9+
]);
10+
it('still does not detect OK', function () {
11+
expect(6)->toBeGreaterThan(3);
12+
})->with([
13+
4,
14+
]);
15+
});

0 commit comments

Comments
 (0)