-
Notifications
You must be signed in to change notification settings - Fork 44
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
Ampers: Nora Peters #32
base: master
Are you sure you want to change the base?
Conversation
…oning of this app but casually left out by Rails...cool.
…adds get_recipe method for search#show
API MuncherWhat We're Looking For
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most obvious thing is you missed the "monkey" patch which we had to include from the SlackAPi project. That might explain your testing problems.
|
||
require "minitest/rails" | ||
require "minitest/reporters" # for Colorized output | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to have this in here:
# MonkeyPatch for This version of Rails :(
if ActionPack::VERSION::STRING >= "5.2.0"
Minitest::Rails::TestUnit = Rails::TestUnit
end
@@ -0,0 +1,17 @@ | |||
<h3>Results for: "<%= @search_terms %>"</h3> | |||
<section> | |||
<div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this div
is really needed here.
@recipes_list = [] | ||
end | ||
|
||
encoded_search_terms = URI.encode(search_terms) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no checking to ensure that search_terms
is a string and it's non-empty.
|
||
def index | ||
@search_terms = params[:search_terms] | ||
@recipes = RecipeApiWrapper.find_recipes(@search_terms).paginate(page: params[:page], per_page: 10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need some sort of check to ensure that @search_terms
is not null. The same applies to params[:page]
it "can get recipes index" do | ||
|
||
VCR.use_cassette("recipes") do | ||
get recipes_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is failing because you're not setting a param here for this path.
get recipes_path, params: {search_terms: query}
get recipes_path | ||
|
||
query = "chicken" | ||
RecipeApiWrapper.find_recipes(query) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you making API Wrapper calls directly here? This should be only making controller calls.
get recipe_path("7bf4a371c6884d809682a72808da7dc2") | ||
|
||
recipe_id = "7bf4a371c6884d809682a72808da7dc2" | ||
RecipeApiWrapper.get_recipe(recipe_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, this is a controller test, not a test of the wrapper.
VCR.use_cassette("get_recipe") do | ||
RecipeApiWrapper.find_recipes("chicken") | ||
recipe_id = "7bf4a371c6884d809682a72808da7dc2" | ||
@recipe = RecipeApiWrapper.find_recipe(recipe_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be:
@recipe = RecipeApiWrapper.get_recipe(recipe_id)
|
||
it "Returns empty array if there are no query matches" do | ||
VCR.use_cassette("recipes2") do | ||
query = 2429384725434.229384687 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be in quotes?
API Muncher
Congratulations! You're submitting your assignment!
Comprehension Questions
NOTE: I keep getting the error: "uninitialized constant Minitest::Rails::TestUnit (NameError)". I tried all of the things mentioned in Slack and tried troubleshooting a bunch on my own, but I still can't get my tests to run.