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

Hrw/master - wave 3 PR #47

Open
wants to merge 30 commits into
base: hrw/master
Choose a base branch
from

Conversation

yourFriendWes
Copy link

l'm having trouble figuring out how to input my specific controller params into a shared rspec. It worked for some of my tests, but not for others. It seems the methods that call my strong params are the ones I'm having trouble with (create and update). This seems to be what's holding my testing coverage at 89%.

I'm going to focus on looking into this issue over the weekend and may submit another PR on Sunday if I'm able to find a solution. I plan on studying up on shared specs, and different types of tests I can write for controllers and models. I felt that being shaky on syntax was holding me back when I was trying to figure out how to write good tests.

Other than those problems with some parts of specs, I think I did ok catching up on the project. I've checked each view and my site looks pretty identical to the heroku site. I took pretty quickly to bootstrap and matching my divs to the example.

@shannachau shannachau self-assigned this Jan 6, 2016
@title = "New Album"
@action = "update"
@method = :patch
@model = Album.find(id)

Choose a reason for hiding this comment

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

Since you're not using the variable id from line 30 again anywhere else, you could pass the params directly into the find method.

@model = Album.find(params[:id])

@shannachau
Copy link

Your code was super clean and easy to read. Also, fantastic commit messages. Kudos for polymorphic paths and using shared examples! I added a couple of suggestions for additional test cases, so you can keep in mind for next time.

Please let me know if you need further clarification on anything! Great job 👍

it "redirect_to :index" do
expect(subject).to redirect_to :index
end
end

Choose a reason for hiding this comment

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

For some additional testing, you could also check the model to see if if the count is equal to 1 after the medium is created. You could also create a set of params that are invalid and check to see if the count in the model is equal to 0.

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