diff --git a/packages/extension/src/Observers/TestResultObserver.test.ts b/packages/extension/src/Observers/TestResultObserver.test.ts index adc1f70c..5d9b4415 100644 --- a/packages/extension/src/Observers/TestResultObserver.test.ts +++ b/packages/extension/src/Observers/TestResultObserver.test.ts @@ -203,4 +203,82 @@ describe('TestResultObserver', () => { expect(testRun.failed).toHaveBeenCalled(); diffSpy.mockRestore(); }); + + describe('dataset parent status propagation', () => { + let parentItem: TestItem; + let child1: TestItem; + let child2: TestItem; + let datasetObserver: TestResultObserver; + + const makeFinished = (id: string): TestFinished => + ({ + event: 'testFinished' as unknown as TeamcityEvent, + id, + flowId: 1, + name: id, + file: '/project/tests/Unit/SampleTests.php', + locationHint: '', + duration: 10, + }) as TestFinished; + + const makeFailed = (id: string): TestFailed => createTestFailed({ id, name: id }); + + beforeEach(() => { + parentItem = ctrl.createTestItem( + 'tests/Unit/SampleTests.php::`something` → it should detect OK', + 'it should detect OK', + Uri.file('/project/tests/Unit/SampleTests.php'), + ); + child1 = ctrl.createTestItem( + 'tests/Unit/SampleTests.php::`something` → it should detect OK with data set "(4)"', + 'with data set "(4)"', + Uri.file('/project/tests/Unit/SampleTests.php'), + ); + child2 = ctrl.createTestItem( + 'tests/Unit/SampleTests.php::`something` → it should detect OK with data set "(8)"', + 'with data set "(8)"', + Uri.file('/project/tests/Unit/SampleTests.php'), + ); + parentItem.children.add(child1); + parentItem.children.add(child2); + + const testItemById = buildTestItemById([parentItem, child1, child2]); + datasetObserver = new TestResultObserver(queue, testRun, testItemById); + }); + + it('marks parent as passed when all dataset children pass', () => { + datasetObserver.testFinished(makeFinished(child1.id)); + expect(testRun.passed).not.toHaveBeenCalledWith(parentItem, undefined); + + datasetObserver.testFinished(makeFinished(child2.id)); + expect(testRun.passed).toHaveBeenCalledWith(parentItem, undefined); + }); + + it('marks parent as failed when any dataset child fails', () => { + datasetObserver.testFinished(makeFinished(child1.id)); + datasetObserver.testFailed(makeFailed(child2.id)); + + expect(testRun.failed).toHaveBeenCalledWith( + parentItem, + expect.any(TestMessage), + undefined, + ); + }); + + it('does not double-mark parent when testSuiteFinished was already called', () => { + datasetObserver.testSuiteFinished({ + event: 'testSuiteFinished' as unknown as TeamcityEvent, + id: parentItem.id, + flowId: 1, + name: parentItem.id, + } as never); + + vi.clearAllMocks(); + + datasetObserver.testFinished(makeFinished(child1.id)); + datasetObserver.testFinished(makeFinished(child2.id)); + + expect(testRun.passed).not.toHaveBeenCalledWith(parentItem, undefined); + }); + }); }); diff --git a/packages/extension/src/Observers/TestResultObserver.ts b/packages/extension/src/Observers/TestResultObserver.ts index f3bbc9bf..a4398f37 100644 --- a/packages/extension/src/Observers/TestResultObserver.ts +++ b/packages/extension/src/Observers/TestResultObserver.ts @@ -21,6 +21,10 @@ import { import { URI } from 'vscode-uri'; export class TestResultObserver implements TestRunnerObserver { + private readonly completedItems = new Set(); + private readonly failedItems = new Set(); + private readonly parentMarked = new Set(); + constructor( private queue: Map, private testRun: TestRun, @@ -38,7 +42,10 @@ export class TestResultObserver implements TestRunnerObserver { } testSuiteStarted(result: TestSuiteStarted): void { - this.doRun(result, (test) => this.testRun.started(test)); + this.doRun(result, (test) => { + this.parentMarked.add(test.id); + this.testRun.started(test); + }); } testStarted(result: TestStarted): void { @@ -46,21 +53,64 @@ export class TestResultObserver implements TestRunnerObserver { } testFinished(result: TestFinished): void { - this.doRun(result, (test) => this.testRun.passed(test, result.duration)); + this.doRun(result, (test) => { + this.testRun.passed(test, result.duration); + this.completedItems.add(test.id); + this.propagateToParent(test); + }); } testFailed(result: TestFailed): void { - this.doRun(result, (test) => - this.testRun.failed(test, this.message(result, test), result.duration), - ); + this.doRun(result, (test) => { + this.testRun.failed(test, this.message(result, test), result.duration); + this.completedItems.add(test.id); + this.failedItems.add(test.id); + this.propagateToParent(test); + }); } testIgnored(result: TestIgnored): void { - this.doRun(result, (test) => this.testRun.skipped(test)); + this.doRun(result, (test) => { + this.testRun.skipped(test); + this.completedItems.add(test.id); + this.propagateToParent(test); + }); } testSuiteFinished(result: TestSuiteFinished): void { - this.doRun(result, (test) => this.testRun.passed(test)); + this.doRun(result, (test) => { + this.parentMarked.add(test.id); + this.testRun.passed(test); + }); + } + + private propagateToParent(test: TestItem): void { + const parent = test.parent; + if (!parent || this.parentMarked.has(parent.id)) { + return; + } + + const trackedChildren = [...parent.children] + .map(([, child]) => child) + .filter((child) => this.testItemById.has(child.id)); + + if ( + trackedChildren.length === 0 || + trackedChildren.some((child) => !this.completedItems.has(child.id)) + ) { + return; + } + + this.parentMarked.add(parent.id); + if (trackedChildren.some((child) => this.failedItems.has(child.id))) { + this.testRun.failed( + parent, + new TestMessage('One or more dataset entries failed'), + undefined, + ); + } else { + this.testRun.passed(parent, undefined); + } } private message(result: TestFailed | TestIgnored, test: TestItem) { diff --git a/packages/phpunit/src/TestIdentifier/PestTestIdentifier.test.ts b/packages/phpunit/src/TestIdentifier/PestTestIdentifier.test.ts index 2b255d71..0c8186ba 100644 --- a/packages/phpunit/src/TestIdentifier/PestTestIdentifier.test.ts +++ b/packages/phpunit/src/TestIdentifier/PestTestIdentifier.test.ts @@ -60,6 +60,44 @@ describe('PestTestIdentifier', () => { }); }); + // Pest v4: php_qn:// format with __pest_evaluable__ method names + // locationHint points to TestCaseFactory.php (not the test file), + // so we must derive the file path from the classFQN in the locationHint. + describe('fromLocationHint v4 php_qn:// evaluable format', () => { + const phpQnBase = + "php_qn:///path/to/vendor/pestphp/pest/src/Factories/TestCaseFactory.php(169) : eval()'d code::"; + + it.each([ + // string describe: describe('something', fn() => it('test example')) + [ + `${phpQnBase}\\P\\Tests\\Unit\\DescribeTest::__pest_evaluable__something__→_it_test_example`, + '__pest_evaluable__something__→_it_test_example', + 'tests/Unit/DescribeTest.php::`something` → it test example', + ], + // nested string describe + [ + `${phpQnBase}\\P\\Tests\\Unit\\DescribeTest::__pest_evaluable__something__→__something_else__→_it_test_example`, + '__pest_evaluable__something__→__something_else__→_it_test_example', + 'tests/Unit/DescribeTest.php::`something` → `something else` → it test example', + ], + // FQN describe: describe(PeName::class, fn() => it('should reject...')) + [ + `${phpQnBase}\\P\\Tests\\Unit\\ClassConstantDatasetTest::__pest_evaluable__App_Domain_PeName__→_it_should_reject_not_of_type_Standard_and_Dynamic`, + '__pest_evaluable__App_Domain_PeName__→_it_should_reject_not_of_type_Standard_and_Dynamic', + 'tests/Unit/ClassConstantDatasetTest.php::`App\\Domain\\PeName` → it should reject not of type Standard and Dynamic', + ], + // FQN describe with dataset child (with data set suffix is NOT encoded) + [ + `${phpQnBase}\\P\\Tests\\Unit\\ClassConstantDatasetTest::__pest_evaluable__App_Domain_PeName__→_it_should_reject_not_of_type_Standard_and_Dynamic with data set "(App\\Domain\\PeName Enum (Standard, 'Standard'))"`, + `__pest_evaluable__App_Domain_PeName__→_it_should_reject_not_of_type_Standard_and_Dynamic with data set "(App\\Domain\\PeName Enum (Standard, 'Standard'))"`, + `tests/Unit/ClassConstantDatasetTest.php::\`App\\Domain\\PeName\` → it should reject not of type Standard and Dynamic with data set "(App\\Domain\\PeName Enum (Standard, 'Standard'))"`, + ], + ])('fromLocationHint(%j, %j) → id: %s', (locationHint, name, expectedId) => { + const result = transformer.fromLocationHint(locationHint, name); + expect(result.id).toBe(expectedId); + }); + }); + describe('fromLocationHint v2 evaluable testSuiteStarted', () => { it.each([ [ diff --git a/packages/phpunit/src/TestIdentifier/PestTestIdentifier.ts b/packages/phpunit/src/TestIdentifier/PestTestIdentifier.ts index d520b07b..2d162d69 100644 --- a/packages/phpunit/src/TestIdentifier/PestTestIdentifier.ts +++ b/packages/phpunit/src/TestIdentifier/PestTestIdentifier.ts @@ -31,6 +31,19 @@ export class PestTestIdentifier extends PHPUnitTestIdentifier { } fromLocationHint(locationHint: string, name: string) { + // Pest v4: php_qn:// locationHint points to TestCaseFactory.php (eval'd code), + // not the test file. Derive file from classFQN embedded in the locationHint. + if (locationHint.startsWith('php_qn://')) { + const phpQnMatch = locationHint.match(/::(?:\\)?(?P\\[\w\\]+)::/); + if (phpQnMatch?.groups?.classFQN) { + const classFQN = phpQnMatch.groups.classFQN; + const file = `${uncapitalize(classFQN.replace(/^P\\/, '')).replace(/\\/g, '/')}.php`; + const decoded = PestV2Fixer.decodeEvaluableV4(name); + return { id: `${file}::${decoded}`, file }; + } + return { id: name, file: '' }; + } + const matched = locationHint.match( /(pest_qn|file):\/\/(?(?[\w\s]+)\((?[\w\\]+)\)(::(?.+))?)/, ); diff --git a/packages/phpunit/src/TestIdentifier/PestV2Fixer.ts b/packages/phpunit/src/TestIdentifier/PestV2Fixer.ts index 0eeb3f28..3246658b 100644 --- a/packages/phpunit/src/TestIdentifier/PestV2Fixer.ts +++ b/packages/phpunit/src/TestIdentifier/PestV2Fixer.ts @@ -19,7 +19,76 @@ function decodeEvaluable(encoded: string) { return before + method; } +// Pest v4: each describe name is backtick-wrapped and separated by __→__ +// The final test name is separated by __→_ (single trailing underscore) +// e.g. `something` → `something else` → it test example +// → __pest_evaluable__something__→__something_else__→_it_test_example +function decodeDescribePart(inner: string): string { + // Decode __ → literal _, single _ → segment separator + const segments: string[] = []; + let current = ''; + let i = 0; + while (i < inner.length) { + if (inner[i] === '_' && inner[i + 1] === '_') { + current += '_'; + i += 2; + } else if (inner[i] === '_') { + segments.push(current); + current = ''; + i++; + } else { + current += inner[i]; + i++; + } + } + segments.push(current); + + // FQN heuristic: all parts start with uppercase → namespace separator \ + // otherwise → space (string describe name) + const isFQN = segments.every((p) => /^[A-Z]/.test(p)); + return `\`${segments.join(isFQN ? '\\' : ' ')}\``; +} + +function decodeEvaluableV4(encoded: string): string { + const prefixIdx = encoded.indexOf(PREFIX); + if (prefixIdx === -1) { + return encoded; + } + + const before = encoded.slice(0, prefixIdx); + + // Separate evaluable method from 'with data set' suffix (not encoded by Pest) + const methodFull = encoded.slice(prefixIdx + PREFIX.length); + const datasetIdx = methodFull.search(/\s+with\s+data\s+set\s+/); + const methodPart = datasetIdx >= 0 ? methodFull.slice(0, datasetIdx) : methodFull; + const datasetSuffix = datasetIdx >= 0 ? methodFull.slice(datasetIdx) : ''; + + // Split by _→_ to separate describe segments from test name + const segments = methodPart.split('_\u2192_'); + if (segments.length < 2) { + // No → separator: plain test (no describe), use simple decode + const decoded = methodPart.replace(/__|_/g, (m) => (m === '__' ? '_' : ' ')); + return before + decoded + datasetSuffix; + } + + const describeParts = segments.slice(0, -1); + const testPart = segments[segments.length - 1]; + + // Describe parts: strip outer _ (backtick markers), then decode + const decodedDescribes = describeParts.map((part) => { + const inner = part.replace(/^_/, '').replace(/_$/, ''); + return decodeDescribePart(inner); + }); + + // Test name: decode _ → space + const decodedTest = testPart.replace(/__|_/g, (m) => (m === '__' ? '_' : ' ')); + + return before + [...decodedDescribes, decodedTest].join(' \u2192 ') + datasetSuffix; +} + export const PestV2Fixer = { + decodeEvaluableV4, + fixId(location: string, name: string) { if (!hasPrefix(name)) { return location; diff --git a/packages/phpunit/src/TestIdentifier/TestIdentifierFactory.ts b/packages/phpunit/src/TestIdentifier/TestIdentifierFactory.ts index 0a7e2382..681ce472 100644 --- a/packages/phpunit/src/TestIdentifier/TestIdentifierFactory.ts +++ b/packages/phpunit/src/TestIdentifier/TestIdentifierFactory.ts @@ -1,6 +1,8 @@ import { PestTestIdentifier } from './PestTestIdentifier'; import { PHPUnitTestIdentifier } from './PHPUnitTestIdentifier'; +// Pest v4 eval'd code: php_qn://...eval()'d code...::\P\Tests\...::__pest_evaluable__... +const pestEvalPattern = /::(?:\\)?P\\/; const pestPattern = /^pest|^P\\|^pest_qn:\/\/|^file:\/\//i; const pestIdentifier = new PestTestIdentifier(); const phpunitIdentifier = new PHPUnitTestIdentifier(); @@ -11,6 +13,9 @@ export const TestIdentifierFactory = { }, create(text: string) { - return TestIdentifierFactory.isPest(text) ? pestIdentifier : phpunitIdentifier; + const isPest = + TestIdentifierFactory.isPest(text) || + (text.startsWith('php_qn://') && pestEvalPattern.test(text)); + return isPest ? pestIdentifier : phpunitIdentifier; }, }; diff --git a/packages/phpunit/tests/fixtures/pest-stub/src/Domain/PeName.php b/packages/phpunit/tests/fixtures/pest-stub/src/Domain/PeName.php new file mode 100644 index 00000000..59cdea1e --- /dev/null +++ b/packages/phpunit/tests/fixtures/pest-stub/src/Domain/PeName.php @@ -0,0 +1,10 @@ +toBe(PeName::class); +})->with([ + PeName::class, +]); + +// Test: with(XXX::EnumCase) - enum case as dataset value (wrapped) +it('should handle enum case as dataset', function (PeName $peName) { + expect($peName)->toBeInstanceOf(PeName::class); +})->with([ + [PeName::Standard], + [PeName::Dynamic], + [PeName::Legacy], +]); + +// Test: both inside describe(FQN::class) +describe(PeName::class, function () { + it('should reject not of type Standard and Dynamic', function (PeName $peName) { + expect($peName)->toBeInstanceOf(PeName::class); + })->with([ + [PeName::Standard], + [PeName::Dynamic], + [PeName::Legacy], + ]); +});