Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pipes - Iuliia - Muncher #46

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

Pipes - Iuliia - Muncher #46

wants to merge 28 commits into from

Conversation

julalam
Copy link

@julalam julalam commented Nov 8, 2017

API Muncher

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How did you go about exploring the Edamam API, how did you try querying the API? I used postman to test different queries after exploring edamam website.
Describe your API Wrapper. How did you decide on the methods you created? In my API Wrapper I used two main methods: search and show recipe in order to fulfill the requirements. The search method is searching Edamam API with given query and using private method creates new instances of class Recipe. Show recipe method doing similar actions for one specific recipe.
Describe an edge case or failure case test you wrote for your API Wrapper. I tested for raising an error if passed invalid app_id and app_key in search recipes method
Explain how VCR aids in testing an API. I used VCR cassettes to record the response from API and use this record in future tests
What is the Heroku URL of your deployed application? https://ic-muncher.herokuapp.com/

@droberts-sea
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions yes
General
Rails fundamentals (RESTful routing, use of named paths) yes
Semantic HTML yes
Errors are reported to the user yes
API Wrapper to handle the API requests yes
Controller testing yes - missing tests around paging
Lib testing yes - could use some expansion
Search Functionality yes
List Functionality yes
Show individual item functionality (link to original recipe opens in new tab) yes
Styling
Foundation Styling for responsive layout yes
List View shows 10 items at a time/pagination yes
The app is styled to create an attractive user interface yes
API Features
The App attributes Edaman yes
The VCR casettes do not contain the API key yes
External Resources
Link to deployed app on Heroku yes
Overall Excellent job overall! This site is attractive and responsive, easy to navigate and for the most part well-tested. Keep up the hard work!


must_respond_with :success
end
end

Choose a reason for hiding this comment

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

There are some interesting test cases around paging that you're missing here:

  1. Does the page still render if paging parameters are passed?
  2. What if the paging parameters are invalid (negative number, etc)?
  3. What if the paging parameters take you past the end of the available search results?

describe "show_recipe" do
it "returns data for a recipe" do
VCR.use_cassette("recipe") do
uri = "637913ec61d9da69eb451818c3293df2"

Choose a reason for hiding this comment

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

I would also like to see a negative test case here - what happens if the URI is invalid?


it "raises an ApiError when the credentials are bad" do
VCR.use_cassette("no recipes") do
proc { EdamamApiWrapper.search_recipes("fish", "invalid_app_id", "invalid_app_key") }.must_raise EdamamApiWrapper::ApiError

Choose a reason for hiding this comment

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

What about for a search with no hits?

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