Add file attr

[mtsmfm]

Ref: #70
This PR depends on #70

@palmerj3

Before I merge could you please just provide a rationale for this change? What problem(s) is it solving on your end?

I'd like to add file attr because some tools (ex. circleci) show file path if the report has this attr.
I thought to fix the path and to add attr can be split but actually, this PR is what I need.

sj26/rspec_junit_formatter#22

[palmerj3]

Hey thanks for clarifying.

I'm concerned about merging this because, as far as I can tell, it would not be a valid junit xml file after this change.

You can see from here https://github.com/jest-community/jest-junit/blob/master/__tests__/lib/junit.xsd that testcase elements don't have a "file" attribute or sub-element. We do have an automated test which tests the generated xml against the xsd included in the project so clearly it passed.

But my concern is that it may mess with other CI environments/systems that are possibly more strict?

Let me do a bit of research on this and see if this could cause any potential problems. I would love to extend the junit and add all sorts of things but it's crucial that we maintain compatibility.

[palmerj3]

In my experience the "classname" is the attribute on "testcase" which can represent the path to the file. In a java system it tends to, anyway. This is already configurable in jest-junit and your previous PR would make that a relative file path. Would circle-ci handle "classname" like you are wanting?

[mtsmfm]

Would circle-ci handle "classname" like you are wanting?

It seems no.

Without file attr

https://circleci.com/gh/mtsmfm/jest-junit/7

With file attr

https://circleci.com/gh/mtsmfm/jest-junit/8

[palmerj3]

Ok thanks for the detailed explanation and for being patient on this. I merged the other PR.

Will spend a few days researching if this change would cause any known issues. Regardless, this change + the other PR will likely result in a new major version to be on the safe side since this would potentially be a breaking change.

[mtsmfm]

@palmerj3 Have you found this change breaks something?

[palmerj3]

Hey @mtsmfm I'm doing my best to talk with a few CI vendors about this to see if there is any likelihood if it breaking.

Again, my main concern here is that this is technically not valid junit xml. If it is only circle ci that makes use of this non standard attribute then I would prefer for them to update their systems to support className.

But if you don't mind giving me another week I'll do my best to see if this is likely to actually break anything and possibly talk to the folks at circle.

[palmerj3]

@mtsmfm so after speaking to some folks from circle and a few other folks familiar with this space I think it's safe for me to merge this. I will be even more safe and do a major version bump even though it's probably not necessary. Thanks again for making this change!

[palmerj3]

@mtsmfm 6.0.0 has been published with these changes. Thank you!

https://github.com/jest-community/jest-junit/releases/tag/v6.0.0

[mtsmfm]

@palmerj3 Thanks!
I've updated jest-junit to v6 in my project and confirmed circleci recognizes file name now!

---

speaking to some folks from circle and a few other folks familiar with this space I think it's safe for me to merge this

Just curious though, could you tell me why did you change your mind?

my main concern here is that this is technically not valid junit xml

I researched and I couldn't find any other tools which accepts file attr except circleci.
I think current junit xml specification lacks this important information but yeah, it's invalid as junit xml format.

[palmerj3]

@mtsmfm all the folks I talked to reassured me that there was no, one, standard for junit files. So, although they do all follow a similar structure, something like this should cause no harm to systems that don't recognize the "file" attribute.

[palmerj3]

@mtsmfm so it appears that this change causes a problem with jenkins xunit processor. I feel it's the right move to rollback right now and publish a patch release.

I'll continue looking into this. But, for now, you might consider resubmitting this PR and add a configuration option where this attribute is conditionally added.

[mtsmfm]

you might consider resubmitting this PR and add a configuration option where this attribute is conditionally added.

Okay, sorry for introducing the problem 🙇