sj26 / rspec_junit_formatter

RSpec results that your CI can read
http://rubygems.org/gems/rspec_junit_formatter
MIT License
302 stars 122 forks source link

Add support for optional path prefix for testcase file property #91

Closed jasonrudolph closed 3 years ago

jasonrudolph commented 3 years ago

Currently, the file property of each testcase element contains the path to the file from RSpec's perspective, but that's not always the full path to the file from the root of the repository. For example, in Homebrew/brew, consider the testcase element emitted for a test in Library/Homebrew/test/dev-cmd/bump_spec.rb:

<testcase 
  classname="test.dev-cmd.bump_spec"
  name="brew bump behaves like parseable arguments can parse arguments"
  file="./test/dev-cmd/bump_spec.rb"
  time="0.015492">
</testcase>

The file property points to ./test/dev-cmd/bump_spec.rb, but the file actually resides at Library/Homebrew/test/dev-cmd/bump_spec.rb. Currently, rspec_junit_formatter has no way to know this full path to the file, so this pull request proposes a way to make that information available to rspec_junit_formatter. πŸ˜…

For tooling that consumes the XML emitted by rspec_junit_formatter, it's useful to be able to use the file property as the full path to the file from the root of the repository, so that the tooling can link to file on github.com (or wherever the repository is hosted).

This pull request proposes adding an optional setting for specifying a path prefix. For example, Homebrew/brew would set this as follows:

RSpec.configure do |config|
  config.junit_formatter_file_path_prefix = "Library/Homebrew"
end

And instead of file property being ./test/dev-cmd/bump_spec.rb, it would be Library/Homebrew/test/dev-cmd/bump_spec.rb.

What do you think?


Note: This is my first time proposing functionality for an RSpec formatter. I see that fuubar uses custom RSpec settings for its configuration, so this pull request uses that same approach. If there's a different approach that would work better for rspec_junit_formatter, please let me know, and I'll update the implementation accordingly.

sj26 commented 3 years ago

Hi @jasonrudolph! Thanks for the contribution.

This is an interesting use case, and I appreciate that this is a useful thing to do in some very specific scenarios. However, I try to keep this formatter as simple as possible. I'd prefer to keep to rspec conventions and emit exactly what rspec says is the path. I'd also prefer to avoid adding any configuration to this formatter, if possible.

If you need to rewrite this path for downstream consumption in your specific case then I'd suggest post-processing the report after rspec has finished running. For example, a ruby script like this:

require "pathname"
require "rexml"
doc = REXML::Document.new(File.read("tmp/rspec.xml"))
doc.root.get_elements("//testcase[@file]").each do |testcase|
  testcase.add_attribute("file", Pathname.new("Library/Homebrew").join(testcase.attribute("file").to_s).to_s)
end
File.write("tmp/rspec.xml", doc.to_s)

Example input from the specs in this repo:

bundle exec rspec --format RspecJunitFormatter --out tmp/rspec.xml
<?xml version="1.0" encoding="UTF-8"?>
<testsuite name="rspec" tests="3" skipped="0" failures="0" errors="0" time="0.560699" timestamp="2021-07-22T11:03:17+10:00" hostname="Ratchet.localdomain">
<properties>
<property name="seed" value="6197"/>
</properties>
<testcase classname="spec.rspec_junit_formatter_spec" name="RSpecJUnitFormatter correctly describes the test results" file="./spec/rspec_junit_formatter_spec.rb" time="0.187049"></testcase>
<testcase classname="spec.rspec_junit_formatter_spec" name="RSpecJUnitFormatter when $TEST_ENV_NUMBER is set includes $TEST_ENV_NUMBER in the testsuite name" file="./spec/rspec_junit_formatter_spec.rb" time="0.187396"></testcase>
<testcase classname="spec.rspec_junit_formatter_spec" name="RSpecJUnitFormatter with a known rspec seed has a property with seed info" file="./spec/rspec_junit_formatter_spec.rb" time="0.185545"></testcase>
</testsuite>

and after running the above ruby snippet:

<?xml version='1.0' encoding='UTF-8'?>
<testsuite errors='0' failures='0' hostname='Ratchet.localdomain' name='rspec' skipped='0' tests='3' time='0.555767' timestamp='2021-07-22T11:05:12+10:00'>
<properties>
<property name='seed' value='56972'/>
</properties>
<testcase classname='spec.rspec_junit_formatter_spec' file='Library/Homebrew/spec/rspec_junit_formatter_spec.rb' name='RSpecJUnitFormatter correctly describes the test results' time='0.190767'/>
<testcase classname='spec.rspec_junit_formatter_spec' file='Library/Homebrew/spec/rspec_junit_formatter_spec.rb' name='RSpecJUnitFormatter when $TEST_ENV_NUMBER is set includes $TEST_ENV_NUMBER in the testsuite name' time='0.182362'/>
<testcase classname='spec.rspec_junit_formatter_spec' file='Library/Homebrew/spec/rspec_junit_formatter_spec.rb' name='RSpecJUnitFormatter with a known rspec seed has a property with seed info' time='0.182048'/>
</testsuite>
jasonrudolph commented 3 years ago

πŸ‘‹ @sj26: Thanks for the super thoughtful reply. :zap:

I try to keep this formatter as simple as possible.

That makes sense to me. It feels like a key strategy for sustainable open source. ✨

Thanks so much for taking the time to suggest a way to attain the same benefits using the existing rspec_junit_formatter functionality. And thanks for the concrete code example along with sample input and output. You've really gone above and beyond here. πŸ₯‡ :bow:

sj26 commented 3 years ago

Thanks for understanding, @jasonrudolph! I hope that post processor or something similar works for your use case.