Skip to content

JSON formatter #609

Merged
merged 4 commits into from Jun 21, 2012

4 participants

@alexch
alexch commented May 1, 2012

I built an RSpec Server to communicate with a JavaScript Client, so I needed a nice way to send back the results. Thus was born JsonFormatter!

I've still got one big bug -- it can't be run more than once in the same process. I suspect an output stream reference is being set in one of the RSpec globals, so I'll have to track that down and make sure it's reset to point to the real StringIO output stream instead of whatever output stream was set on the first run. But it seems to work ok in normal (command-line, one run at a time) use.

I also made the code work more gracefully when run in $SAFE mode. It looks like you've already got some policies of returning nil on failure in the libraries I was working in so I think that's kosher; otherwise a low-level SecurityError will be thrown and ruin your whole day.

alexch added some commits Apr 6, 2012
@alexch alexch formatters run better when $SAFE mode is on
Setting $SAFE=3 causes some file operations to raise a SecurityError. This patch allows you to run in SAFE mode without those SecurityErrors masking the real errors raised by the code under test and/or by RSpec matcher failures.

I extracted the SAFE setting/resetting logic into a helper method called "safely" since it's called from several places.
576b82d
@alexch alexch improve rdoc comments in BaseFormatter f87b634
@alexch alexch basic JsonFormatter 972591b
@dchelimsky
RSpec member

This all looks good, but I'm not sure if you want me to wait for you to address the bug.

@alexch
alexch commented May 1, 2012
@alexch
alexch commented May 1, 2012
@dblock dblock and 1 other commented on an outdated diff May 4, 2012
lib/rspec/core/formatters/base_formatter.rb
@@ -68,7 +86,8 @@ def stop
end
# This method is invoked after all of the examples have executed. The next method
- # to be invoked after this one is #dump_failure (once for each failed example),
+ # to be invoked after this one is #dump_failures
+ # (BaseTextFormtter then calls #dump_failure once for each failed example.)
@dblock
dblock added a note May 4, 2012

typo in "BaseTextFormtter"

@alexch
alexch added a note May 23, 2012

fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dblock dblock commented on an outdated diff May 4, 2012
lib/rspec/core/formatters/json_formatter.rb
+ hash[:exception] = {
+ :class => e.class.name,
+ :message => e.message,
+ :backtrace => e.backtrace,
+ }
+ end
+ end
+ end
+ end
+
+ def close
+ output.write @output_hash.to_json
+ output.close if IO === output && output != $stdout
+ end
+
+ protected
@dblock
dblock added a note May 4, 2012

Remove extra comments below?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dblock dblock and 1 other commented on an outdated diff May 4, 2012
spec/rspec/core/formatters/json_formatter_spec.rb
+ ],
+ :summary => {
+ :duration => formatter.output_hash[:summary][:duration],
+ :example_count => 3,
+ :failure_count => 1,
+ :pending_count => 1,
+ },
+ :summary_line => "3 examples, 1 failure, 1 pending"
+ }
+ formatter.output_hash.should == expected
+ output.string.should == expected.to_json
+ end
+
+ # todo: include full 'execution_result'
+
+ it "relativizes backtrace paths"
@dblock
dblock added a note May 4, 2012

change to pending, same with the todo

@alexch
alexch added a note May 4, 2012

doesn't an empty "it" automatically become pending? or is the preferred style to actually say "pending" instead of "it" at that level now?

@alexch
alexch added a note May 4, 2012

And now that you mention it, I'm not sure what the right pending policy is here. These are features that are nice to have but may never be implemented. Do we want to clutter the test output with permanent pendings?

@dblock
dblock added a note May 4, 2012

@dchelimsky should comment, imho you don't want any permanent comments or pending specs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dblock dblock and 1 other commented on an outdated diff May 4, 2012
spec/rspec/core/formatters/json_formatter_spec.rb
+
+ describe "#dump_summary" do
+ it "adds summary info to the output hash" do
+ duration, example_count, failure_count, pending_count = 1.0, 2, 1, 1
+ formatter.dump_summary(duration, example_count, failure_count, pending_count)
+ summary = formatter.output_hash[:summary]
+ %w(duration example_count failure_count pending_count).each do |key|
+ summary[key.to_sym].should == eval(key)
+ end
+ summary_line = formatter.output_hash[:summary_line]
+ summary_line.should == "2 examples, 1 failure, 1 pending"
+ end
+ end
+end
+
+
@dblock
dblock added a note May 4, 2012

all the commented code below should probably not make it

@alexch
alexch added a note May 23, 2012

removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dblock
dblock commented May 4, 2012

A JSON formatter might be a more generic way of doing #596 maybe? If there was an easy way to feed the JSON-fromatted output back into RSpec and tell it to rerun only failed tasks.

@alexch
alexch commented May 4, 2012

I think a failures-only formatter is perfectly acceptable -- it's focused on one responsibility, whereas the JSON formatter has to do much more, and isn't really meant to be human-readable.

Thanks for the pointer to that other PR though -- I'll follow its lead in adding a cucumber feature and a command-line option to this one.

@alexch
alexch commented May 23, 2012

Pushed a new commit. It's not as featureful as I'd like but I'd say it's ready to ship.

@ibash
ibash commented Jun 20, 2012

Is this not going to get merged in? I'd like it.

@dchelimsky
RSpec member

@ibash thanks for pinging on this - had fallen off my radar.

@alexch really nice work, thanks!

@dchelimsky dchelimsky merged commit 8e41f1a into rspec:master Jun 21, 2012
@dchelimsky dchelimsky added a commit that referenced this pull request Jun 21, 2012
@dchelimsky dchelimsky Changelog for #609 [ci skip] cfb465f
@dchelimsky dchelimsky added a commit that referenced this pull request Jun 21, 2012
@dchelimsky dchelimsky Revert "Merge pull request #609 from alexch/json-formatter"
Failed in ci build for 1.8.7, jruby, ree

This reverts commit 8e41f1a, reversing
changes made to 076d683.
359cd38
@dchelimsky dchelimsky added a commit that referenced this pull request Jun 21, 2012
@dchelimsky dchelimsky Restore "Merge pull request #609 from alexch/json-formatter""
Most of the commits with this merge are fine - only one is causing failures
in some rubies - we can deal with that issue separately.

This reverts commit 359cd38.
afa4250
@dchelimsky
RSpec member

@alexch this fails in Ruby 1.8.7, jruby, and ree: http://travis-ci.org/#!/rspec/rspec-core/jobs/1672994 - all related to supporting a SecurityError. I reverted that one commit (actually reverted the whole merge, then reverted the revert, and then reverted just the one commit): 713ad6f

Any chance you'd like to make those changes work across the diff ruby versions?

@dchelimsky dchelimsky added a commit that referenced this pull request Jun 21, 2012
@dchelimsky dchelimsky Revert "Merge pull request #609 from alexch/json-formatter"
This reverts commit 8e41f1a, reversing
changes made to 076d683.
ed9ec72
@dchelimsky
RSpec member

I take it back - the cukes for the json fail in a few Rubies, so I reverted the whole lot again. Unfortunately github doesn't support reopening a pull request, so I need to ask you to open a new one. Please refer back to this one if/when you do so.

@alexch
@alexch
alexch commented Aug 17, 2012

resubmitted as #661

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.