Skip to content
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

Add file attr if JEST_JUNIT_ADD_FILE_ATTRIBUTE is true #77

Merged
merged 1 commit into from Feb 14, 2019

Conversation

mtsmfm
Copy link
Contributor

@mtsmfm mtsmfm commented Jan 13, 2019

@palmerj3
Copy link
Collaborator

Sweet!

Code looks good. Would you mind just doing two additional things?

I'd like you to link to any documentation in circle-ci in the README so folks know this file attribute is primarily for there. If that doesn't exist then perhaps just expand on the README description of the setting a bit, mentioning that it provides richer details for circle-ci but may break on other CI platforms if enabled.

The other bit I'd like you to do is add another test which tests that the file attribute does not exist without the setting.

Thank you!!

@mtsmfm
Copy link
Contributor Author

mtsmfm commented Jan 13, 2019

@palmerj3

I'd like you to link to any documentation in circle-ci in the README

I can't find any doc about file attr.
I think https://circleci.com/docs/2.0/collect-test-data/ is the most similar one.

I've added a description and test.

@palmerj3
Copy link
Collaborator

@nicolasschabram would you be willing to test out this PR on your jenkins configuration to see if it breaks?

By default it should be just fine - but will break your setup if the JEST_JUNIT_ADD_FILE_ATTRIBUTE exists.

@nicolasschabram
Copy link

Yup, seems to work. Jenkins fails with the same error I had in #74 when setting this in package.json:

"jest-junit": {
  "addFileAttribute": "true"
}

Without it, everything works fine.

(Btw, I noticed it only picks up the setting with "true" but not with true.)

Anyway, thanks a lot for the fix!

@mtsmfm
Copy link
Contributor Author

mtsmfm commented Jan 31, 2019

(Btw, I noticed it only picks up the setting with "true" but not with true.)

I used usePathForSuiteName as a reference:

Or you can also define a jest-junit key in your package.json. All are string values.

{
  ...
  "jest-junit": {
    "suiteName": "jest tests",
    "outputDirectory": ".",
    "outputName": "./junit.xml",
    "classNameTemplate": "{classname}-{title}",
    "titleTemplate": "{classname}-{title}",
    "ancestorSeparator": " › ",
    "usePathForSuiteName": "true"
  }
}

@mtsmfm
Copy link
Contributor Author

mtsmfm commented Feb 13, 2019

@palmerj3 Is there any blocker to merge this PR?

@palmerj3
Copy link
Collaborator

@mtsmfm hey I don't think so. I'm going to go ahead and merge it and push a new version.

@palmerj3 palmerj3 merged commit e031458 into jest-community:master Feb 14, 2019
@palmerj3
Copy link
Collaborator

@mtsmfm v6.3.0 was published with this change

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.

None yet

3 participants