Skip to content

Conversation

@dramanat
Copy link

I'm pretty sure that lib\api.rb & movie_json.rb are correct. I had a lot of trouble with spec\api_spec.rb. When I run the spec, it says "6 examples 0 failures", but there are only 5 "it"s. Will appreciate any comments/suggestions. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

Here you are rescuing the entire search_by_title method, where no matter what exception is raised, you're showing "no_movie_found". That's not REALLY the case though. For example, if the API is down or broken, you still want people to be able to find their "Caddyshack"

@jwo
Copy link
Member

jwo commented Mar 11, 2013

This looks good! I added a comment on having rescues that reach a little too far. Let me know if that makes sense or if we should go into further detail.

@dramanat
Copy link
Author

I understand what you are saying. Would you mind taking a look at
spec/api_spec.rb? I had a lot of trouble with the spec file, and I'm not
sure if I did it right. When I run it, it gives "6 examples, 0 failures"
as output; but there are only 5 "it"s.

Thanks,
Dharini

On Mon, Mar 11, 2013 at 2:46 PM, Jesse Wolgamott
[email protected]:

This looks good! I added a comment on having rescues that reach a little
too far. Let me know if that makes sense or if we should go into further
detail.


Reply to this email directly or view it on GitHubhttps://github.com//pull/14#issuecomment-14737674
.

@jwo
Copy link
Member

jwo commented Mar 13, 2013

The 6th spec is in the spec/movie_spec.rb. If you ran it separately you'd get what you expect.

$ rspec spec/api_spec.rb 
.....

Finished in 0.0053 seconds
5 examples, 0 failures

I'll leave a couple comments on the spec file too :)

Copy link
Member

Choose a reason for hiding this comment

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

I would not add the expect here in the before block. The before block is adding this test to every example.

I do think having this in the before makes sense, because you want every time it executes get_url_as_json it should return that fixture.

Api.stub(:get_url_as_json) { JSON.parse(File.read("spec/fixtures/forrest.json"))

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.

2 participants