Skip to content
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

Updated JUnit reporter #4725

Merged
merged 3 commits into from
Jan 30, 2023
Merged

Updated JUnit reporter #4725

merged 3 commits into from
Jan 30, 2023

Conversation

patricks
Copy link
Contributor

Some JUnit XML parsers require, that the warnings and errors count is set per test suite. Bitbucket Pipelines require this otherwise the errors / warnings are not recognized. I updated to code that these values are also provided. Another important value would be the total tests count (successful count + warnings count + errors count). But I couldn't figure out an easy way to get this value. I also update the format of the output.

Previous generated file output format:

<?xml version="1.0" encoding="utf-8"?>
<testsuites><testsuite>
	<testcase classname='Formatting Test' name='File.swift'>
<failure message='Lines should not have trailing whitespace.'>warning:
Line:15 </failure>	</testcase>
</testsuite></testsuites>

New file output format:

<?xml version="1.0" encoding="utf-8"?>
<testsuites failures="1" errors="0">
	<testsuite failures="1" errors="0">
		<testcase classname='Formatting Test' name='File.swift'>
			<failure message='Lines should not have trailing whitespace'>Warning:Line:15</failure>
		</testcase>
	</testsuite>
</testsuites>

@SwiftLintBot
Copy link

SwiftLintBot commented Jan 27, 2023

18 Messages
📖 Linting Aerial with this PR took 1.1s vs 1.11s on main (0% faster)
📖 Linting Alamofire with this PR took 1.4s vs 1.4s on main (0% slower)
📖 Linting Brave with this PR took 7.53s vs 7.55s on main (0% faster)
📖 Linting DuckDuckGo with this PR took 3.03s vs 3.03s on main (0% slower)
📖 Linting Firefox with this PR took 9.46s vs 9.57s on main (1% faster)
📖 Linting Kickstarter with this PR took 10.28s vs 10.34s on main (0% faster)
📖 Linting Moya with this PR took 0.57s vs 0.58s on main (1% faster)
📖 Linting NetNewsWire with this PR took 3.25s vs 3.25s on main (0% slower)
📖 Linting Nimble with this PR took 0.63s vs 0.63s on main (0% slower)
📖 Linting PocketCasts with this PR took 7.62s vs 7.63s on main (0% faster)
📖 Linting Quick with this PR took 0.24s vs 0.24s on main (0% slower)
📖 Linting Realm with this PR took 11.96s vs 12.03s on main (0% faster)
📖 Linting SourceKitten with this PR took 0.45s vs 0.45s on main (0% slower)
📖 Linting Sourcery with this PR took 2.33s vs 2.33s on main (0% slower)
📖 Linting Swift with this PR took 4.72s vs 4.73s on main (0% faster)
📖 Linting VLC with this PR took 1.41s vs 1.41s on main (0% slower)
📖 Linting Wire with this PR took 9.08s vs 9.12s on main (0% faster)
📖 Linting WordPress with this PR took 11.37s vs 11.4s on main (0% faster)

Generated by 🚫 Danger

Copy link
Collaborator

@SimplyDanny SimplyDanny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only some small remarks ...

violations.map({ testCase(for: $0) }).joined(),
"\n\t</testsuite>\n",
"</testsuites>"
].joined()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use a text block instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be also possible, but I did it this way, because CheckstyleReporter, GitHubActionsLoggingReporter, GitLabJUnitReporter, HTMLReporter and XcodeReporter also used this style.
Should I change it anyway?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine for now. Let me refactor them in a follow-up.

</testcase>
</testsuite>
</testsuites>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks much better! 👍

CHANGELOG.md Outdated
@@ -73,6 +73,10 @@
[SimplyDanny](https://github.com/SimplyDanny)
[#4121](https://github.com/realm/SwiftLint/issues/4121)

* Updated JUnit reporter to output error count and warning count.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to end the line with two spaces:

Suggested change
* Updated JUnit reporter to output error count and warning count.
* Updated JUnit reporter to output error count and warning count.


return [
"<?xml version=\"1.0\" encoding=\"utf-8\"?>\n",
"<testsuites failures=\"\(warningCount)\" errors=\"\(errorCount)\">\n",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warnings are failures? Is that right?

@SimplyDanny SimplyDanny merged commit 651b00e into realm:main Jan 30, 2023
@SimplyDanny
Copy link
Collaborator

Thank you @patricks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants