Skip to content

Add feature to generate JUnit XML output #720

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jcoleman
Copy link

@jcoleman jcoleman commented Apr 4, 2025

Background

It's common to use the JUnit plugin on Jenkins to consume machine-readable output about the tests in your CI builds. For example, on our Ruby app tests we use the rspec_junit_formatter to output results from RSpec in a format that Jenkins can consume.

What

This PR adds JUnit style XML output as an option to Spoom so that running Sorbet in CI on Jenkins can output error information that can be displayed natively by Jenkins (and then, in turn, passed along to e.g. Github's interface).

With these changes Sorbet failures on a build look like this:

Screenshot 2025-04-03 at 3 04 34 PM

and a clean Sorbet build looks like this:

Screenshot 2025-04-03 at 2 53 32 PM

TODO

I'm happy to add docs for this feature as well, but I wanted to get initial review of the feature going sooner rather than later.

@jcoleman jcoleman requested a review from a team as a code owner April 4, 2025 14:31
Copy link
Collaborator

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

Hey @jcoleman,

I think the feature is okay but the code will need to be adjusted.

Thanks!

@jcoleman
Copy link
Author

@Morriar I think I've responded to all of your feedback; I usually leave clicking "Resolve" for the reviewer so they can confirm the change first; let me know if you have a different preference.

@jcoleman
Copy link
Author

@Morriar Would you be able to trigger workflows and re-review?

jcoleman and others added 3 commits April 23, 2025 20:14
This is particularly useful when integrating Sorbet into CI builds
running on Jenkins so that Jenkins can display the output as specific
failures.
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
@jcoleman
Copy link
Author

@Morriar I think all of your feedback is resolved now, and I've rebased on main.

Copy link
Collaborator

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

Tests are failing because we're missing the type signatures.

@@ -11,6 +13,36 @@ class << self
def sort_errors_by_code(errors)
errors.sort_by { |e| [e.code, e.file, e.line, e.message] }
end

def to_junit_xml(errors, path:)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You'll need to add a signature to this method:

Suggested change
def to_junit_xml(errors, path:)
#: (Array[Error], path: String)
def to_junit_xml(errors, path:)

But thinking about it more I think this method should return the REXML::Document considering it's name and writing to the file should be done in the client.

So:

Suggested change
def to_junit_xml(errors, path:)
#: (Array[Error]) -> REXML::Document
def to_junit_xml(errors)

@@ -153,6 +185,34 @@ def <=>(other)
def to_s
"#{file}:#{line}: #{message} (#{code})"
end

def to_junit_xml_element
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def to_junit_xml_element
#: -> REXML::Element
def to_junit_xml_element

@@ -37,6 +37,7 @@ Gem::Specification.new do |spec|
spec.add_dependency("rbi", ">= 0.3.1")
spec.add_dependency("sorbet-static-and-runtime", ">= 0.5.10187")
spec.add_dependency("thor", ">= 0.19.2")
spec.add_dependency("rexml")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's put a lower bound, possibly 3.2.6?

Suggested change
spec.add_dependency("rexml")
spec.add_dependency("rexml", ">= 3.2.6")

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.

2 participants