From f0016cb1daa4f959e9af6f6c4c8a36408810e3c8 Mon Sep 17 00:00:00 2001 From: YuF-9468 Date: Mon, 16 Mar 2026 21:39:44 +0800 Subject: [PATCH] fix: Extract error message from PHPUnit error details when message attribute is missing Fixes #711 When PHPUnit XML output contains or elements without a message attribute, the parser now extracts the error message from the error details text instead of only showing the error type. The improved logic: 1. Prefer message attribute when present (with type prepended) 2. Extract error message from details when it contains a line matching the error type (e.g., "TypeError: ...") 3. Fall back to first line of details 4. Finally, use error type as last resort This provides more informative error messages in test reports when PHPUnit outputs errors with the full stack trace in the element text rather than a message attribute. --- .../phpunit-phpcheckstyle-results.md | 4 +- __tests__/__outputs__/phpunit-test-results.md | 4 +- .../__snapshots__/phpunit-junit.test.ts.snap | 8 +- .../phpunit/phpunit-no-message-attr.xml | 20 ++++ __tests__/phpunit-message-extraction.test.ts | 105 ++++++++++++++++++ .../phpunit-junit/phpunit-junit-parser.ts | 35 +++++- 6 files changed, 165 insertions(+), 11 deletions(-) create mode 100644 __tests__/fixtures/phpunit/phpunit-no-message-attr.xml create mode 100644 __tests__/phpunit-message-extraction.test.ts diff --git a/__tests__/__outputs__/phpunit-phpcheckstyle-results.md b/__tests__/__outputs__/phpunit-phpcheckstyle-results.md index 6966c55..2797b04 100644 --- a/__tests__/__outputs__/phpunit-phpcheckstyle-results.md +++ b/__tests__/__outputs__/phpunit-phpcheckstyle-results.md @@ -62,9 +62,9 @@ ### ❌ OtherTest ``` ❌ testOther - PHPUnit\Framework\ExpectationFailedException + OtherTest::testOther ❌ testException - PHPUnit\Framework\ExpectationFailedException + OtherTest::testException ✅ testEmpty ✅ testSwitchCaseNeedBreak ``` diff --git a/__tests__/__outputs__/phpunit-test-results.md b/__tests__/__outputs__/phpunit-test-results.md index 67f8258..9a599fa 100644 --- a/__tests__/__outputs__/phpunit-test-results.md +++ b/__tests__/__outputs__/phpunit-test-results.md @@ -13,9 +13,9 @@ ### ❌ CLI Arguments ``` ❌ targeting-traits-with-coversclass-attribute-is-deprecated.phpt - PHPUnit\Framework\PhptAssertionFailedError + targeting-traits-with-coversclass-attribute-is-deprecated.phptFailed asserting that string matches format description. ❌ targeting-traits-with-usesclass-attribute-is-deprecated.phpt - PHPUnit\Framework\PhptAssertionFailedError + targeting-traits-with-usesclass-attribute-is-deprecated.phptFailed asserting that string matches format description. ``` ### ✅ PHPUnit\Event\CollectingDispatcherTest ``` diff --git a/__tests__/__snapshots__/phpunit-junit.test.ts.snap b/__tests__/__snapshots__/phpunit-junit.test.ts.snap index d48d941..fbfaf76 100644 --- a/__tests__/__snapshots__/phpunit-junit.test.ts.snap +++ b/__tests__/__snapshots__/phpunit-junit.test.ts.snap @@ -315,7 +315,7 @@ Failed asserting that 19 matches expected 20. /workspace/phpcheckstyle/test/OtherTest.php:24", "line": 12, - "message": "PHPUnit\\Framework\\ExpectationFailedException", + "message": "OtherTest::testOther", "path": undefined, }, "name": "testOther", @@ -330,7 +330,7 @@ Failed asserting that 0 matches expected 1. /workspace/phpcheckstyle/test/OtherTest.php:40", "line": 31, - "message": "PHPUnit\\Framework\\ExpectationFailedException", + "message": "OtherTest::testException", "path": undefined, }, "name": "testException", @@ -572,7 +572,7 @@ TestRunResult { /home/matteo/OSS/phpunit/src/TextUI/TestRunner.php:62 /home/matteo/OSS/phpunit/src/TextUI/Application.php:200", "line": undefined, - "message": "PHPUnit\\Framework\\PhptAssertionFailedError", + "message": "targeting-traits-with-coversclass-attribute-is-deprecated.phptFailed asserting that string matches format description.", "path": undefined, }, "name": "targeting-traits-with-coversclass-attribute-is-deprecated.phpt", @@ -609,7 +609,7 @@ TestRunResult { /home/matteo/OSS/phpunit/src/TextUI/TestRunner.php:62 /home/matteo/OSS/phpunit/src/TextUI/Application.php:200", "line": undefined, - "message": "PHPUnit\\Framework\\PhptAssertionFailedError", + "message": "targeting-traits-with-usesclass-attribute-is-deprecated.phptFailed asserting that string matches format description.", "path": undefined, }, "name": "targeting-traits-with-usesclass-attribute-is-deprecated.phpt", diff --git a/__tests__/fixtures/phpunit/phpunit-no-message-attr.xml b/__tests__/fixtures/phpunit/phpunit-no-message-attr.xml new file mode 100644 index 0000000..d528d3e --- /dev/null +++ b/__tests__/fixtures/phpunit/phpunit-no-message-attr.xml @@ -0,0 +1,20 @@ + + + + + Symfony\Component\VarDumper\Tests\Caster\DOMCasterTest::testCastModernDocumentType +TypeError: Cannot assign DOMNodeList to property Dom\Node::$childNodes of type Dom\NodeList + +/home/runner/work/php-latest-builds/php-latest-builds/src/Symfony/Component/VarDumper/Caster/DOMCaster.php:190 +/home/runner/work/php-latest-builds/php-latest-builds/src/Symfony/Component/VarDumper/Cloner/AbstractCloner.php:379 +/home/runner/work/php-latest-builds/php-latest-builds/src/Symfony/Component/VarDumper/Cloner/VarCloner.php:127 +/home/runner/work/php-latest-builds/php-latest-builds/src/Symfony/Component/VarDumper/Cloner/AbstractCloner.php:318 +/home/runner/work/php-latest-builds/php-latest-builds/src/Symfony/Component/VarDumper/Test/VarDumperTestTrait.php:79 +/home/runner/work/php-latest-builds/php-latest-builds/src/Symfony/Component/VarDumper/Test/VarDumperTestTrait.php:63 +/home/runner/work/php-latest-builds/php-latest-builds/src/Symfony/Component/VarDumper/Tests/Caster/DOMCasterTest.php:235 +/home/runner/work/php-latest-builds/php-latest-builds/.phpunit/phpunit-12-0/phpunit:104 +/home/runner/work/php-latest-builds/php-latest-builds/vendor/symfony/phpunit-bridge/bin/simple-phpunit.php:458 +/home/runner/work/php-latest-builds/php-latest-builds/vendor/symfony/phpunit-bridge/bin/simple-phpunit:13 + + + diff --git a/__tests__/phpunit-message-extraction.test.ts b/__tests__/phpunit-message-extraction.test.ts new file mode 100644 index 0000000..f8ee19a --- /dev/null +++ b/__tests__/phpunit-message-extraction.test.ts @@ -0,0 +1,105 @@ +import * as fs from 'fs' +import * as path from 'path' + +import {PhpunitJunitParser} from '../src/parsers/phpunit-junit/phpunit-junit-parser.js' +import {ParseOptions} from '../src/test-parser.js' +import {normalizeFilePath} from '../src/utils/path-utils.js' + +import {fileURLToPath} from 'url' +import {dirname} from 'path' +const __filename = fileURLToPath(import.meta.url) +const __dirname = dirname(__filename) + +describe('phpunit-junit parser - message extraction', () => { + it('extracts message from first line of error details when message attribute is not present', async () => { + const fixturePath = path.join(__dirname, 'fixtures', 'phpunit', 'phpunit-no-message-attr.xml') + const filePath = normalizeFilePath(path.relative(__dirname, fixturePath)) + const fileContent = fs.readFileSync(fixturePath, {encoding: 'utf8'}) + + const opts: ParseOptions = { + parseErrors: true, + trackedFiles: [] + } + + const parser = new PhpunitJunitParser(opts) + const result = await parser.parse(filePath, fileContent) + + // Find the failed test + const suite = result.suites.find(s => s.name === 'DOMCasterTest') + expect(suite).toBeDefined() + + const tests = suite!.groups.flatMap(g => g.tests) + const failedTest = tests.find(t => t.name === 'testCastModernDocumentType') + expect(failedTest).toBeDefined() + expect(failedTest!.result).toBe('failed') + expect(failedTest!.error).toBeDefined() + + // Verify that the message is extracted from the first line of error details + expect(failedTest!.error!.message).toBe( + 'TypeError: Cannot assign DOMNodeList to property Dom\\Node::$childNodes of type Dom\\NodeList' + ) + + // Verify that full details are still captured + expect(failedTest!.error!.details).toContain('TypeError: Cannot assign DOMNodeList to property Dom\\Node::$childNodes of type Dom\\NodeList') + expect(failedTest!.error!.details).toContain('/home/runner/work/php-latest-builds/php-latest-builds/src/Symfony/Component/VarDumper/Caster/DOMCaster.php:190') + }) + + it('prefers message attribute when present', async () => { + const fixturePath = path.join(__dirname, 'fixtures', 'external', 'phpunit', 'junit-basic.xml') + const filePath = normalizeFilePath(path.relative(__dirname, fixturePath)) + const fileContent = fs.readFileSync(fixturePath, {encoding: 'utf8'}) + + const opts: ParseOptions = { + parseErrors: true, + trackedFiles: [] + } + + const parser = new PhpunitJunitParser(opts) + const result = await parser.parse(filePath, fileContent) + + // Find the failed test that has a message attribute + const authSuite = result.suites.find(s => s.name === 'Tests.Authentication') + expect(authSuite).toBeDefined() + + const tests = authSuite!.groups.flatMap(g => g.tests) + const failedTest = tests.find(t => t.name === 'testCase9') + expect(failedTest).toBeDefined() + expect(failedTest!.result).toBe('failed') + expect(failedTest!.error).toBeDefined() + + // When message attribute is present, use it (with type prepended) + expect(failedTest!.error!.message).toBe('AssertionError: Assertion error message') + }) + + it('uses type as message when neither message attribute nor details are available', async () => { + // Create a minimal XML with only type attribute + const xmlContent = ` + + + + + + +` + + const opts: ParseOptions = { + parseErrors: true, + trackedFiles: [] + } + + const parser = new PhpunitJunitParser(opts) + const result = await parser.parse('test.xml', xmlContent) + + const suite = result.suites.find(s => s.name === 'TestSuite') + expect(suite).toBeDefined() + + const tests = suite!.groups.flatMap(g => g.tests) + const failedTest = tests.find(t => t.name === 'testError') + expect(failedTest).toBeDefined() + expect(failedTest!.result).toBe('failed') + expect(failedTest!.error).toBeDefined() + + // When only type is available, use it as message + expect(failedTest!.error!.message).toBe('RuntimeException') + }) +}) diff --git a/src/parsers/phpunit-junit/phpunit-junit-parser.ts b/src/parsers/phpunit-junit/phpunit-junit-parser.ts index 4886fc1..af479fa 100644 --- a/src/parsers/phpunit-junit/phpunit-junit-parser.ts +++ b/src/parsers/phpunit-junit/phpunit-junit-parser.ts @@ -156,13 +156,42 @@ export class PhpunitJunitParser implements TestParser { } let message: string | undefined + + // First, try to extract message attribute (with type prepended if available) if (typeof failure !== 'string' && failure.$) { - message = failure.$.message - if (failure.$.type) { - message = message ? `${failure.$.type}: ${message}` : failure.$.type + if (failure.$.message) { + message = failure.$.type ? `${failure.$.type}: ${failure.$.message}` : failure.$.message + } else if (failure.$.type && !details) { + // Only use type alone if there's no details to extract from + message = failure.$.type } } + // If no message attribute, try to extract from details + if (!message && details) { + const typePrefix = typeof failure !== 'string' && failure.$?.type ? `${failure.$.type}: ` : '' + const lines = details.split(/\r?\n/) + for (const line of lines) { + const trimmedLine = line.trim() + // Check if line starts with type prefix (e.g., "TypeError: ") + if (typePrefix && trimmedLine.startsWith(typePrefix)) { + message = trimmedLine + break + } + } + + // If no error message pattern found, use first line + if (!message) { + const firstLine = lines[0].trim() + message = firstLine || undefined + } + } + + // If still no message and type is available, use it as last resort + if (!message && typeof failure !== 'string' && failure.$?.type) { + message = failure.$.type + } + return { path: filePath, line,