-
Notifications
You must be signed in to change notification settings - Fork 479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add filename and line number to phpunit output #2775
Conversation
You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x. |
bae0cbc
to
c5b4df4
Compare
Please link some proof this is part of the JUnit standard. |
I amended the commit to try and justify the inclusion. Seems like there is no agreed upon junit standard 😢 ! no xsd here : discussion of the absence of proper junit xsd here : Doing the same as phpunit should be safe ? |
Some of the attributes I have added are added here in PHPUnit : https://github.com/sebastianbergmann/phpunit/blob/main/src/Logging/JUnit/JunitXmlLogger.php#L428 |
ee5b53a
to
ddf6fd1
Compare
Remove xsd all together, as is the case in phpunit : no xsd here : https://github.com/sebastianbergmann/phpunit/blob/main/src/Logging/JUnit/JunitXmlLogger.php#L298 discussion of the absence of proper junit xsd here : sebastianbergmann/phpunit#2964
@@ -27,7 +27,7 @@ public function formatErrors( | |||
|
|||
$result = '<?xml version="1.0" encoding="UTF-8"?>'; | |||
$result .= sprintf( | |||
'<testsuite failures="%d" name="phpstan" tests="%d" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/junit-team/junit5/r5.5.1/platform-tests/src/test/resources/jenkins-junit.xsd">', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you remove this? Maybe it's important
@@ -1,118 +0,0 @@ | |||
<?xml version="1.0" encoding="UTF-8" ?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you can find a newer version of this schema that includes file and line?
$result = sprintf('<testcase name="%s">', $this->escape($reference)); | ||
$result = sprintf('<testcase name="%s"', $this->escape($reference)); | ||
if ($fileName !== null) { | ||
$result .= sprintf(' file="%s"', $this->escape($fileName)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about absolute vs. relative path to the file? Typically you don't want to print the whole absolute path, but trim the beginning. But I'm not sure what PHPUnit does and what tools that read the output expect.
Anyway, this is usually done with the help of RelativePathHelper
. Check the other formatters.
@ondrejmirtes I'm removing any mention of a XSD because there is no official schema. PHPUnit does not have it in its output as well : https://github.com/sebastianbergmann/phpunit/blob/main/tests/end-to-end/logging/log-junit-to-file.phpt#L26 If I add the My objective is to add file and linenumber so that, in my private repo, I can better annotate failed pull requests with https://github.com/marketplace/actions/junit-report-action (it annotates PR with annotations on offending lines) |
I just realized that you have a github output format ! You can disregard this PR is removing the XSD seems too much |
Yeah, PHPStan provides nice experience out of the box :) Thanks anyway. |
No description provided.