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 captured log msgs to junit xml file #3156

Merged
merged 2 commits into from
Feb 6, 2018

Conversation

twmr
Copy link
Contributor

@twmr twmr commented Jan 26, 2018

For each tests this adds the captured log msgs to the respective
'system-out' tags in the junit xml output file.

Thanks for submitting a PR, your contribution is really appreciated!

Here's a quick checklist that should be present in PRs:

  • Add a new news fragment into the changelog folder
    • name it $issue_id.$type for example (588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the pr
    • ensure type is one of removal, feature, bugfix, vendor, doc or trivial
    • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."
  • Target: for bugfix, vendor, doc or trivial fixes, target master; for removals or features target features;
  • Make sure to include reasonable tests for your change if necessary

Unless your change is a trivial or a documentation fix (e.g., a typo or reword of a small section) please:

  • Add yourself to AUTHORS, in alphabetical order;

@twmr twmr force-pushed the junit_xml_log_fix branch 2 times, most recently from 1ec8860 to b6bb49a Compare January 26, 2018 21:46
@coveralls
Copy link

coveralls commented Jan 26, 2018

Coverage Status

Coverage decreased (-0.006%) to 92.622% when pulling 2d0c1e9 on thisch:junit_xml_log_fix into f2fb841 on pytest-dev:features.

@nicoddemus
Copy link
Member

Thanks @Thisch, looks good so far.

I think we should have an option to control that though, I can imagine use cases where users don't want their JUnitXML files to contain logging information, or want their logging to be directed to stderr instead. How about an ini option: junitxml_logging=no/stdout/stderr, defaulting to no to keep backward compatibility?

@twmr
Copy link
Contributor Author

twmr commented Feb 2, 2018

@nicoddemus, yes, adding an ini flag makes sense. I don't know why one would want to write the captured logs to the <system-err> tag, though. Anyway I've added the junit_logging ini option in my latest commit.

For each test this adds the captured log msgs to a system-* tag in the junit
xml output file. The destination of the system-* tag is specified by
junit_logging ini option.
@twmr
Copy link
Contributor Author

twmr commented Feb 3, 2018

I think we should have an option to control that though, I can imagine use cases where users don't want their JUnitXML files to contain logging information, or want their logging to be directed to stderr instead. How about an ini option: junitxml_logging=no/stdout/stderr, defaulting to no to keep backward compatibility?

@KKoukiou, what do you think?

@KKoukiou
Copy link
Contributor

KKoukiou commented Feb 5, 2018

@Thisch I definitely find it useful. Do you think it might worth to add option about captured log level as well? With my custom fixture for capturing logs, I end up in out memory in my Jenkins when I was capturing all logs including DEBUG. So, without having ability to set default captured log level, unfortunately I can't use this change.

@twmr
Copy link
Contributor Author

twmr commented Feb 5, 2018

We already have the log_level option, which is taken into account in the junitxml plugin. Have you already tried to set the log_level param to INFO or don't specify it at all (then WARNING is used)?

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @Thisch!

@KKoukiou
Copy link
Contributor

KKoukiou commented Feb 6, 2018

@Thisch right, thanks :)

@nicoddemus nicoddemus merged commit 29a074e into pytest-dev:features Feb 6, 2018
@nicoddemus
Copy link
Member

Thanks again @Thisch!

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.

4 participants