Add file information to XML report

[marcomorain]

This PR adds file information to the XML report to make it easier for tools like CircleCI to produce helpful build reports.

[marcomorain]

Hi @sj26 – is this something you are willing to accept into rspec_junit_formatter?

[sj26]

Sorry, I haven't been getting new PR notifications for some reason which I hope has just been rectified.

Yes, this seems like a good idea. Why are you walking up to the example group path though? Should this include line number information as well?

[kylev]

@dwwoelfel or @marcomorain: Do you have an answer for @sj26's question? I think I know the answer, if you care to confirm...

I did some poking (because I'd like this to be merged) and discovered that when dealing with shared examples, the ExampleGroup handed to the before(:each) will be the definition file for the shared examples, not where the shared examples were used. So if you have it_behaves_like 'a shape' inside square_spec.rb, the top level ExampleGroup will refer to shared_examples_for_shapes.rb.

That isn't relevant output for junit-style logging. Where the shared examples were called from is what is relevant (i.e., the actual test file), so recursing into the ExampleGroup metadata reveals the actual spec file the shared examples are being invoked from.

Since you can't run a shared examples file, the time should be assigned to the spec file that uses the shared examples.

[kylev]

Re-ping @dwwoelfel and @marcomorain to see if they can comment on the walking the path.

I'd love to see this folded into an official release so I don't have to refer to a diverged fork at circleci/rspec_junit_formatter.

[marcomorain]

Hi guys - sorry for the delay on this – I get this emails into my personal github email address so they get buried in gmail.

[marcomorain]

@kylev @sj26 TBH I'm not that familair with the internals of rspec – @dwwoelfel wrote this and he has since moved on from CircleCI. My interest is in getting this merged upstream – so that you don;t have to use a fork, @kylev :)

@kylev do you have a better suggestion for how to handle this?

[kylev]

I can confirm that this patch works in my project that uses rspec 3.2 (and builds on CircleCI).

@sj26 can merge as is, or I'm going to generate and test a new version on several rspec variants that also takes CircleCI-Archived/rspec_junit_formatter#3 into account (a deprecation issue).

[kylev]

Actually, ignore CircleCI-Archived/rspec_junit_formatter#3. It seems to be a bad report or something. Maybe they were using a pre-release version? I can't repro their issue.

Merge this pull request as-is. It works as I described above: ascribing the test and runtime to the appropriate spec file (including when shared examples are in play). I tested it with a micro project against latest versions of rspec-core of each point release: 2.14.8, 3.0.4, 3.1.7 and 3.2.1.

😄

[kylev]

Ping? This can be merged. It works on all modern rspec versions.

[cayennes]

Hi (also from CircleCI)

I wasn't around when @dwwoelfel wrote this either and I don't have much experience with rspec, but I believe @kylev's explanation is correct. To elaborate on why we want the file where the test is defined rather than where the shared code is defined: the specific aim is to be able to collect info about how long each file takes to run, in order to distribute them better for running tests in parallel. Our fork tends to get out of date, so it would be great if people could use the official version of the formatter and still get this functionality.

What do you think?

[kylev]

Yup. It's just a little thinger to make sure rspec shared examples are correctly attributed, time-wise.

[BBonifield]

@sj26 Can we ? LGTM

[kylev]

So say we all?

[appplemac]

Hi there (I’m also from CircleCI),

@sj26 Sorry to bother you again, but what do you think about this?

[sj26]

I don't have an active test environment, so I'll trust you guys. :-)

[marcomorain]

Thanks for merging this.

[rahilsondhi]

Is it possible to get line numbers in the output?

[rahilsondhi]

Following up on my previous comment, I added this functionality in #38