search

rubyworks / minitap

TAP-Y/J Output Formats for MiniTest
http://rubyworks.github.com/minitap
Other
9 stars 4 forks source link

puts statement in tested method causes malformed output #1

Closed cvshepherd closed 12 years ago

cvshepherd commented 12 years ago

If a method which is tested contains a puts statement, piping to tapout will fail because the output of minitap is malformed, due to the fact that the puts seems to get executed and inserted into the output. See https://gist.github.com/1523619

EDIT: Both the YAML and JSON runners are affected.

trans commented 12 years ago

Ah, good catch. I'll need to redirect output.

Okay, I'll work on this hopefully today. Thanks.

trans commented 12 years ago

Working on this, my first thought was that since MiniTest allows standard output to come through then it's really up to the developer to prevent that in their tests. And in general I think that is very true. MiniTest even provides a #capture_io method to help developers handle these cases.

However, thinking about it a bit more, it makes sense for minitap to capture standard output to help ensure it doesn't malform the JSON and YAML formats. Keep in mind, there is no perfect solution here b/c one can always write against the STDOUT/STDERR constants and thus circumvent the capture. Nonetheless, to do this well, the end-user still needs to see the output. So, as it turns out, your issue has led to an extension to the underlying TAP-Y/J format! I will be adding stdout and stderr fields to the next revision. Thus your output will look like this:

{"type":"suite","start":"2011-12-29 08:32:17","count":1,"seed":25263,"rev":4}
{"type":"case","subtype":"","label":"TestThis","level":0}
{"type":"test","subtype":"","status":"pass","label":"test_broken","time":0.000160825,"stdout":"this will break tapout because of malformed output"}
{"type":"final","time":0.00020762,"counts":{"total":1,"pass":1,"fail":0,"error":0,"omit":0,"todo":0}}

What do you think of this solution?

(P.S. You're "why do i have to require this?" might have been caused by a copy-and-paste typo that prevented json from being loaded automatically. Which I fixed too. Thanks.)

cvshepherd commented 12 years ago

Sounds good. I'm new to MiniTest and wasn't even aware of #capture_io. But yeah; with that in mind, the addition of those new fields in the output makes sense, and could be rather helpful too.

P.S. I didn't create a second issue for the 'require problem' I was having, because I hadn't looked into it yet, and wasn't sure who or what was to blame. Good to have that sorted out now as well.

trans commented 12 years ago

Ok. The latest release should have this issue put to bed.

Please, let me know if you run into any other problems. Thanks!