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

Merged
merged 1 commit into from
Apr 30, 2025

Conversation

jcoleman
Copy link
Contributor

@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
Contributor 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
Contributor Author

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

@jcoleman jcoleman force-pushed the junit-output branch 2 times, most recently from 22ea5fd to 167def1 Compare April 23, 2025 20:15
@jcoleman
Copy link
Contributor 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.

@jcoleman
Copy link
Contributor Author

@Morriar Pushed up final changes (weird double commit since I used the Github review "commit suggestions" feature for the first bit, and then had to manually push up another bit separately).

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.

This looks good, thanks!

@Morriar Morriar enabled auto-merge April 29, 2025 17:34
@jcoleman
Copy link
Contributor Author

Some style issues I'll fix in a minute:

Offenses:

lib/spoom/sorbet/errors.rb:209:13: C: [Correctable] Style/TrailingCommaInArrayLiteral: Put a comma after the last item of a multiline array.
            *more
            ^^^^^
spoom.gemspec:40:3: C: [Correctable] Gemspec/OrderedDependencies: Dependencies should be sorted in an alphabetical order within their section of the gemspec. Dependency rexml should appear before thor.
  spec.add_dependency("rexml", ">= 3.2.6")
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
test/spoom/cli/srb/tc_test.rb:383:19: C: [Correctable] Style/RegexpLiteral: Use %r around regular expression.
            .gsub(/^(\s+)https:\/\/github\.com\/sorbet\/sorbet\/tree\/[0-9a-f]+/, '\1<GIT_LINK>')
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

auto-merge was automatically disabled April 29, 2025 18:22

Head branch was pushed to by a user without write access

@jcoleman
Copy link
Contributor Author

@Morriar I think you'll have to approve the workflow again for the Rubocop fixes.

@Morriar Morriar enabled auto-merge (squash) April 29, 2025 18:49
auto-merge was automatically disabled April 30, 2025 14:00

Head branch was pushed to by a user without write access

@jcoleman
Copy link
Contributor Author

@Morriar I fixed the generated RBI files now (it's unfortunate that CI doesn't run all the checks but only proceeds if the previous step completes 😞).

@Morriar
Copy link
Collaborator

Morriar commented Apr 30, 2025

For some reason CI is not triggering, can you try rebasing on main and pushing again?

This is particularly useful when integrating Sorbet into CI builds
running on Jenkins so that Jenkins can display the output as specific
failures.
@Morriar Morriar enabled auto-merge (squash) April 30, 2025 18:28
@Morriar Morriar merged commit 190c31a into Shopify:main Apr 30, 2025
5 checks passed
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