current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

Is there a better way of representing this RSpec code on Rails using Capybara?

There are three scenarios only for when the sign up validations fail so is there a better way of representing them rather than having 4 scenarios codes. I don't want to create a model folder so please don't suggest that. 

    feature 'Login' do
      before do
        FactoryGirl.create(:user)
      end

      scenario "success login", js: true do
        # set_speed(:slow)
        visit root_path
        click_link 'Login'

        fill_in 'email', :with => '[email protected]'
        fill_in 'password', :with => 'password'
        click_button 'Login'

        expect(page).to have_content('Logged in successfully')
      end

      scenario "failed login", js: true do
        # set_speed(:slow)
        visit root_path
        click_link 'Login'

        fill_in 'email', :with => '[email protected]'
        fill_in 'password', :with => 'something failed'
        click_button 'Login'

        expect(page).to have_content('Invalid login/password combination')
      end
    end

    feature "Sign Up" do
      scenario "success sign up", js:true do
        visit root_path
        click_link 'Login'
        click_link 'Sign Up'

        fill_in 'user[email]', :with=>'[email protected]'
        fill_in 'user[password]', :with=> 'password'
        fill_in 'user[password_confirmation]', :with=> 'password'
        click_button 'Create User'
        expect(page).to have_content('User successfully added.')
      end


      scenario "failed sign up/Wrong email format", js:true do
        visit root_path
        click_link 'Login'
        click_link 'Sign Up'  
        fill_in 'user[email]', :with=>'signup.example.com'
        fill_in 'user[password]', :with=> 'password'
        fill_in 'user[password_confirmation]', :with=> 'password'
        click_button 'Create User'
        expect(page).to have_content('is invalid')
      end

      scenario "failed sign up/Short Email address", js:true  do
        visit root_path
        click_link 'Login'
        click_link 'Sign Up'
        fill_in 'user[email]', :with=>'sign'
        fill_in 'user[password]', :with=> 'password'
        fill_in 'user[password_confirmation]', :with=> 'password'
        click_button 'Create User'
        expect(page).to have_content('is too short (minimum is 5 characters)')
      end 

      scenario "failed sign up/Long Email address", js:true  do
        visit root_path
        click_link 'Login'
        click_link 'Sign Up'
        fill_in 'user[email]', :with=>'[email protected]'
        fill_in 'user[password]', :with=> 'password'
        fill_in 'user[password_confirmation]', :with=> 'password'
        click_button 'Create User'
        expect(page).to have_content('is too long (maximum is 50 characters)')
      end 

    end
share|improve this question

migrated from stackoverflow.com Dec 3 '14 at 11:14

This question came from our site for professional and enthusiast programmers.

3 Answers 3

up vote 1 down vote accepted

Instead of creating 3 long signup features for 3 diff email cases, you could do something like: 

describe "email is in wrong format" do
  let(:user) {FactoryGirl.create(:user)}
  before {user.email = something.with.wrongformat}
  it {should_not be_valid}
end

describre "too long email" do
  let(:user) {FactoryGirl.create(:user)}
  before {user.email = ("a"*60)+"@gmail.com"}
  it {should_not be_valid}
end

It'll be same as signup process coz in both you deal with user creation.

And also as for @tokland answer.

I think it could be better not to repeat pattern of:

it "......." do
  expect(page).to ......
end

But just add subject {page} after before block on the top. It allows you to write just like:

describe "....." do
  before {visit root_path}
  it {should have_content('Desired content'}
end
share|improve this answer
up vote 3 down vote

Some comments:

  • Rspec expects the concatenation of feature + scenarios strings to form a human readable text.

  • I may be wrong, but I think the orthodox structure of specs is to perform the actions in the before block, the it block should contain only assertions.

  • Use before blocks to keep your code DRY.

  • Personal opinion: in a feature spec, the less app internals you use, the better. So I'd write "/" instead of root_path.

  • The user/password info is in the factory, I'd prefer explicit attributes on the creation.

That's how I'd write the Login feature, the same ideas apply for Signup:

feature 'Login' do
  before do
    FactoryGirl.create(:user, :email => "[email protected]", :password => "password")
    visit root_path
    click_link 'Login'
  end

  scenario "with valid user/password", js: true do
    before do
      fill_in 'email', :with => '[email protected]'
      fill_in 'password', :with => 'password'
      click_button 'Login'
    end

    it "shows the logged in message" do
      expect(page).to have_content('Logged in successfully')
    end
  end

  scenario "with wrong user/password", js: true do
    before do
      fill_in 'email', :with => '[email protected]'
      fill_in 'password', :with => 'something failed'
      click_button 'Login'
    end

    it "shows an error message" do
      expect(page).to have_content('Invalid login/password combination')
    end
  end
end
share|improve this answer
1  
I agree with your use of before. And I think it's a fair point to use "/" instead of root_path, though I wouldn't insist on it. I'd say using route helpers are fair abstraction, esp. if you have good routing specs already. The root path is a bit special, of course, as it's always "/" - no names or words to complicate things. Anyway, if simple, readable path are a feature in your app, it's probably good to hand-write them. But using the helpers is just fine too. –  Flambino Dec 4 '14 at 0:57
    
prefer to define the route with simple readable path. Good mention on the before. Thanks –  Kingsley Simon Dec 5 '14 at 2:09
up vote 3 down vote

I think some of your expectations might lead to brittle tests. I would modify the expectations a little. For example:

  expect(page).to have_content('Logged in successfully')

What happens if you changed "Logged in successfully" to "Welcome, mate!", or "Merry Christmas, Mohamad"? Your tests will fail even though the code is sound.

Is "Logged in successfully" part of your spec? Or is your spec "The user should be signed in and have a session."

I think that looking for a specific CSS class on the page, as well as other classes, (link classes that should or should not be present if the user is signed in) might be more robust. I would even look for link text, like "Sign in" and "Sign out".

Sure, those can change too, even CSS classes. But they're less likely to change than messages to the user.

share|improve this answer
    
Hi Mohammad, this was a basic test. I am new to rspec and was just testing out. I do understand that the testing scenario is not strong enough and i do know i need to aply more complex testing procedures. Thanks for the mention. –  Kingsley Simon Dec 5 '14 at 2:08

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.