Option to omit stderr and stdout from passing tests in junitxml

[jmbowman]

I discovered that on a large codebase which I recently helped switch from nose to pytest, the preservation of stderr and stdout even for passing tests is adding more than 100 MB to the size of the XML artifacts for each pull request's CI run. We would like to preserve the stderr and stdout data for failed tests to aid in fixing them, but keeping it for tests that passed is wasting large amounts of disk space and network bandwidth for virtually no benefit.

While I personally see little value in keeping this data for passing tests, there may be people who currently count on it being there. One approach would be to add a command line option to change this behavior, something like --junit-capture-only-fail. I'm up for submitting a PR, but could use some feedback on the preferred way to fix this.

[nicoddemus]

@jmbowman that's a good suggestion.

I personally agree with you that I don't see much value on this, but you are right that probably there are people that depend on this so we shouldn't just remove the functionality.

A PR would be welcome, but I think it should be an ini option instead: you probably want that configured for the test runs permanently, and it is also easy to change that in the command-line by using -o junitxml_capture_only_fail.

Perhaps we should name the option junitxml_capture=all/fail instead of a boolean: we might want to add more options later, and the effort is the same to implement two named options now than later have to deprecate the boolean flag and add a new option.

[robert-cody]

I think that in most cases together with fail logs you will need uxpass (unexpected pass) and maybe even xfail (expected fail), so it's better to have an option like junitxml_capture_mode=pass,fail,xfail,uxpass - as default value, so you can always remove some result types or even all.

[nicoddemus]

junitxml_capture_mode=pass,fail,xfail,uxpass

Great, that was the kind of brainstorm I was aiming for. One small correction: we call uxpass just xpass, but other than that I agree with everything you said. 👍

[RonnyPfannschmidt]

can someone explain to me what those actually mean? now we have some keywords that map to outcomes, hows that relate to capture of outpput, logs and warnings?

[nicoddemus]

Sorry, not sure what you mean by your question @RonnyPfannschmidt, could you please clarify?

[RonnyPfannschmidt]

well - capture is about logs, warnings, stdout, stderr, outcomes is about test results

so we should be carefull about how we name things, becasue to me capture mode makes zero sense when the elements are outcomes

[nicoddemus]

Oh I now see what you mean, thanks.

You got a point, we should make a clear distinction. We are actually configuring for which outcomes we want captured stderr and stdout output that should be included in the XML report.

junitxml_stdout_stderr_outcomes=pass,fail,xfail,xpass

Or go ahead and make two options?

junitxml_stdout_outcomes=pass,fail,xfail,xpass
junitxml_stderr_outcomes=pass,fail,xfail,xpass

[RonnyPfannschmidt]

my impression is that this is turning into a tricky hassle,

which brings me back to the idea of externalizing the junitxml plugin and allowing people to compose ot more

[jmbowman]

I did look for a hook to customize the behavior of junitxml before proposing this change, but the best I could come up with was a monkey-patch. Even the monkey-patch was a little tricky to accomplish; the stderr and stdout data is added to the XML output during the teardown phase, during which it seems to be difficult to determine what the test outcome was. I wasn't sure off hand if it would be valid to move those tags earlier in the file when the test outcome is written, and there's even an issue in the backlog to fix conformance with the format spec. So I agree that a good cleanup of the junitxml code is in order, but have no strong feelings regarding whether it should be moved or not.

[RonnyPfannschmidt]

@jmbowman the idea behind externalizing is to allow us to expose a api to just compose a reporter so that you no longer have t patch around in private modules

[Zac-HD]

Closing due to inactivity; we'd be happy to take a PR but this issue doesn't appear to be a priority for the Pytest community.