Add flag to print all attachments for successful tests

[mtreinish]

This commit adds a new flag, --all-attachments, to the run, load, and
last commands (in addition to a matching user config flag). When set
this flag will tell subunit trace to print all text attachments for a
test whether it was successful or it failed.

Fixes #223

[mtreinish]

@aspiers give this a try and let me know if it's what you're looking for.

[coveralls]

Coverage decreased (-0.2%) to 65.891% when pulling 784e7ab on add-all-attachments into cacc175 on master.

[mtreinish]

@masayukig Do you have any thoughts on this?

[masayukig]

I didn't see this patch deeply (I'll do it later) but, can we have unit tests for this change? Meaningless?

[mtreinish]

Unit testing this is a bit tricky, since the difference when the flag is set is to print text attachments that are not stdout and stderr. The only way I could think of testing it would be to create a fake test case with a text attachment, run it with the flag, and then inspect the captured output from the run. Which seemed like a lot of effort for minimal gain.

[masayukig]

LGTM. This option works in my environment, too. But I'd like to wait for a while for @aspiers 's comment and/or feedback before merging.

[mtreinish]

@aspiers have you had a chance to take a look at this and see if it fixes #223 for you?

[aspiers]

@mtreinish Sorry for the delay. I've now tested this via

.tox/py36/bin/pip install -e 'git+https://github.com/mtreinish/stestr.git@add-all-attachments#egg=stestr'

and it works great! Many thanks :-)

[aspiers]

Just an observation (even though I hardly know this codebase, so treat with a big pinch of salt) - there seems to be a lot of parameter passing required, which makes adding new options like this a pretty heavyweight change for what it is. Maybe worth a refactor so that an "output options" object is passed around instead?

[aspiers]

Wouldn't it make more sense to error in this case?

[mtreinish]

I was just being lazy, doing it this way was slightly easier to implement. In my mind it doesn't matter exactly what the behavior is as long as we're explicit about how it works. That being said, this just made me realize there is a broken edge case with mixed inputs right now. If you set suppress-attachments in the user config file and --all-attachments on the cli that will mean no attachments print. So I'll have to change the behavior here anyway to make sure the cli takes precedence for both. I'll change this to an error at the same time since I'm not attached to any behavior.

[mtreinish]

Just an observation (even though I hardly know this codebase, so treat with a big pinch of salt) - there seems to be a lot of parameter passing required, which makes adding new options like this a pretty heavyweight change for what it is. Maybe worth a refactor so that an "output options" object is passed around instead?

That might be something to investigate in the future. Part of my concern with doing this though is that each most of the places we pass a parameter is an external public interface. So we have to watch out for any backwards compatibility issues. For example, all the the arg parsing just uses the public python interface so it passes all the parsed args just to that. It'll be easier to do a cleanup like this in the lower internal levels (like the _run_tests() function where we don't have to worry about breaking users (hopefully).

[masayukig]

https://ci.appveyor.com/project/mtreinish/stestr/builds/25586943/job/1a0fw6pgbgtuivh6#L196

ERROR: Sphinx requires at least Python 3.5 to run.

This is really annoying... I pushed a patch for this #246 .