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

Selam Ainalem - Amperes #24

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

Conversation

SelamawitA
Copy link

No description provided.

@CheezItMan
Copy link

MediaRanker Revisted

What We're Looking For

Feature Feedback
General
Appropriate Tests on WorksController You're missing some tests for guests users, like deleting a work, or creating one etc
Appropriate Tests on SessionsController Similarly you need to test logout with guests
Appropriate Tests on UsersController No tests for guests!
Completed OAuth You have the login working, but you need a filter to force users to be logged in for most routes.
Overall You need to work on doing more tests for guest users and put in a filter to prevent guest users from getting where they shouldn't. Those are things I'd like you to work on in Api-Muncher once you get the basic API parts done.

}

# Create mock account with user
OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new(@riley_git_hash)

Choose a reason for hiding this comment

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

You really should DRY this up using the login method in the test_helper.rb file.

end

describe 'Destroy' do
it 'will log out a user' do

Choose a reason for hiding this comment

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

What about guest users?


describe 'Index' do

Choose a reason for hiding this comment

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

No testing for guest users!

flash[:status].must_equal :failure
flash[:result_text].must_equal "Could not create book"
flash[:messages][:title].size.must_equal 1
flash[:messages][:title][0].must_equal "can't be blank"

Choose a reason for hiding this comment

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

Might be more flexible to have:

        flash[:messages][:title].must_contain "can't be blank"

post works_path,
params:
{work:
{rating: "5", creator: "world", description: "foo", publication_year: 2008, category: "album"}}

Choose a reason for hiding this comment

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

How is "album" a bogus category?

Maybe this test would be better named, with bogus data.


flash[:status].must_equal :failure
flash[:result_text].must_equal "Could not create album"
assert_operator flash[:messages][:title].size,:>, 0

Choose a reason for hiding this comment

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

You're missing assert-style and spec-style testing.

You can use

flash[:messages][:title].must_be :>, 0


it "renders 404 not_found for a bogus work ID" do
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 test seems a mishmash of things being tested.


it "succeeds with no media" do

describe 'Guest User-tests' do

Choose a reason for hiding this comment

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

You need to test the other routes with guest users and verify that they're redirected.

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