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

Mary Ampers #41

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

Mary Ampers #41

wants to merge 15 commits into from

Conversation

mmlamkin
Copy link

@mmlamkin mmlamkin commented May 2, 2018

No description provided.

it "succeeds with new valid username and redirects to root path" do
#not sure why these tests don't work!
new_user = User.new(
provider: 'githib',

Choose a reason for hiding this comment

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

provider githib???

Maybe this is the reason the test fails!

}.must_change 'User.count', 0


must_respond_with :bad_request

Choose a reason for hiding this comment

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

Your controller is redirecting instead of providing bad request.

perform_login(existing_user)
post upvote_path(Work.find(works(:movie).id))
must_respond_with :redirect
must_redirect_to work_path(Work.find(works(:movie).id))
end

it "redirects to the work page after the user has logged out" do

Choose a reason for hiding this comment

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

This doesn't really go here.

end
end

describe "upvote" do

it "redirects to the work page if no user is logged in" do
existing_user = users(:dan)

Choose a reason for hiding this comment

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

A flash notice check would also be appropriate in cases like this.


perform_login(existing_user)

post login_path, params: {

Choose a reason for hiding this comment

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

What is this doing? It's a failed login without enough data.

@@ -1,5 +1,33 @@
require 'test_helper'

describe UsersController do
describe "index" do

Choose a reason for hiding this comment

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

What about tests for guest users?

@CheezItMan
Copy link

MediaRanker Revisted

What We're Looking For

Feature Feedback
General
Appropriate Tests on WorksController Some issues here, but otherwise well done
Appropriate Tests on SessionsController Check with some problems here
Appropriate Tests on UsersController Check, no tests for guest users
Completed OAuth Nicely done!
Overall Overall well done, you've got some issues with testing and need to be through at testing each route with guests and unauthenticated users.

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