0
votes

I wrote the following feature spec to test sign up, sign in, password recovery, and account locking behavior in a Rails 4.2.8 application with Devise.

The tests work. However, I'm relatively new to Capybara, so I'd like your suggestions regarding how to improve their readability, maintainability, DRYness, speed, reduce brittleness, etc.

Validations such email uniqueness, password strength and so on are performed at the User model spec. Hence, I did not re-test those behaviors here.

# spec/features/user_registration_spec.rb 
require 'rails_helper'

def login_via_form(email, password)
  expect(page).to have_current_url("/users/sign_in")
  fill_in "Email", with: email
  fill_in "Password", with: password
  click_button "Sign in"
end

RSpec::Matchers.define :be_logged_in do |user_first_name|
  match do |page|
    expect(page).to have_current_path "/"
    expect(page).to have_content "Hello #{user_first_name}"
  end
  failure_message do |actual|
    "expected the current path to be /, actual path is #{page.current_path}" \
    "\nexpected to find a 'Hello #{user_first_name}' in '#{page.text}'."
  end
end

describe "User self-management", type: :feature, js: true do
  let!(:user) { FactoryGirl.create(:user) }
  let(:valid_attributes) { FactoryGirl.attributes_for(:user) }

  it "registers a new user" do
    visit "/"

    click_link "Sign up"

    fill_in "Email", with: valid_attributes[:email]
    fill_in "Password", with: valid_attributes[:password]
    fill_in "Password confirmation", with: valid_attributes[:password]
    fill_in "First name", with: valid_attributes[:first_name]
    fill_in "Last name", with: valid_attributes[:last_name]
    select "United States", from: "Country"
    select "New York", from: "State"
    select "New York", from: "City"
    fill_in "Sangha", with: "My Sangha"
    fill_in "Phone number", with: "+1 212-555-0187"
    fill_in "Facebook url", with: "http://www.facebook.com/johndoe"
    click_button "Sign up"

    # The user may only login *after* confirming her e-mail
    expect(page).to have_content "A message with a confirmation link has been" \
      " sent to your email address. Please follow the link to activate your" \
      " account."
    expect(page).to have_current_path "/users/sign_in"

    # Find email sent to the given recipient and set the current_email variable
    # Implemented by https://github.com/DockYard/capybara-email
    open_email(valid_attributes[:email])
    expect(current_email.subject).to eq "Confirmation instructions"
    current_email.click_link "Confirm my account"

    expect(page).to have_content "Your email address has been successfully " \
                                 "confirmed."

    login_via_form(valid_attributes[:email],
                   valid_attributes[:password])
    expect(page).to have_content "Signed in successfully."
    expect(page).to be_logged_in(valid_attributes[:first_name])
  end

  it "performs password recovery (creates a new password)" do
    visit "/users/sign_in"
    click_link "Forgot your password?"
    fill_in "Email", with: user.email
    click_button "Send me reset password instructions"

    expect(page).to have_text "You will receive an email with instructions " \
                              "on how to reset your password in a few minutes."

    # Find email sent to the given recipient and set the current_email variable
    open_email(user.email)
    expect(current_email.subject).to eq "Reset password instructions"
    current_email.click_link "Change my password"

    fill_in "New password", with: valid_attributes[:password]
    fill_in "Confirm new password", with: valid_attributes[:password]
    click_button "Change my password"

    expect(page).to have_content "Your password has been changed " \
                                 "successfully. You are now signed in."
    expect(page).to be_logged_in(user.first_name)

    open_email(user.email)
    expect(current_email.subject).to eq "Password Changed"
    expect(current_email.body). to have_text "We're contacting you to notify " \
      "you that your password has been changed."
  end

  describe "resend confirmation e-mail" do
    context "with an already confirmed e-mail address" do
      it "warns the user and does not send a new confirmation e-mail" do
        # Our factory creates users with confirmed e-mails
        visit "/users/sign_in"
        click_link "Didn't receive confirmation instructions?"
        fill_in "Email", with: user.email
        expect {
          click_button "Resend confirmation instructions"
        }.not_to change(ActionMailer::Base.deliveries, :count)
        expect(page).to have_text "Email was already confirmed"
      end
    end

    context "with an unconfirmed e-mail address" do
      it "sends a new confirmation e-mail" do
        # Unconfirm user (our factory creates users with confirmed e-mails)
        user.update(confirmed_at: nil)

        visit "/users/sign_in"
        click_link "Didn't receive confirmation instructions?"
        fill_in "Email", with: user.email
        click_button "Resend confirmation instructions"

        expect(page).to have_text "You will receive an email with " \
          "instructions for how to confirm your email address in a few minutes."

        open_email(user.email)
        expect(current_email.subject).to eq "Confirmation instructions"
        current_email.click_link "Confirm my account"

        expect(page).to have_content "Your email address has been " \
          "successfully confirmed."

        login_via_form(user.email, user.password)
        expect(page).to have_content "Signed in successfully."
        expect(page).to be_logged_in(user.first_name)
      end
    end
  end

  it "locks the account after 5 failed login attempts" do
    visit "/users/sign_in"

    3.times do
      login_via_form(user.email, "bogus")
      expect(page).to have_text "Invalid Email or password."
    end

    login_via_form(user.email, "bogus")
    expect(page).to have_text "You have one more attempt before your " \
      "account is locked."

    login_via_form(user.email, "bogus")
    expect(page).to have_text "Your account is locked."

    open_email(user.email)
    expect(current_email.subject).to eq "Unlock instructions"
    current_email.click_link "Unlock my account"

    expect(page).to have_content "Your account has been unlocked " \
      "successfully. Please sign in to continue."

    login_via_form(user.email, user.password)
    expect(page).to have_content "Signed in successfully."
    expect(page).to be_logged_in(user.first_name)
  end

  context "account is locked, didn't receive unlocking instructions" do
    it "sends a new unlocking instructions e-mail" do
      user.update(locked_at: DateTime.now)

      visit "/users/sign_in"
      click_link "Didn't receive unlock instructions?"
      fill_in "Email", with: user.email
      click_button "Resend unlock instructions"

      expect(page).to have_text "You will receive an email with instructions " \
        "for how to unlock your account in a few minutes."

      open_email(user.email)
      expect(current_email.subject).to eq "Unlock instructions"
      current_email.click_link "Unlock my account"

      expect(page).to have_content "Your account has been unlocked " \
        "successfully. Please sign in to continue."

      login_via_form(user.email, user.password)
      expect(page).to have_content "Signed in successfully."
      expect(page).to be_logged_in(user.first_name)
    end
  end

  context "account is not locked, attempts to re-send unlocking instructions" do
    it "warns the user and does not send a new confirmation e-mail" do
      # Our factory creates users with confirmed e-mails
      visit "/users/sign_in"
      click_link "Didn't receive unlock instructions?"
      fill_in "Email", with: user.email
      expect {
        click_button "Resend unlock instructions"
      }.not_to change(ActionMailer::Base.deliveries, :count)
      expect(page).to have_text "Email was not locked"
    end
  end
end

Thank you.

1
Note: All your expect { ... }.not_to change ... may not actually be doing anything. This is because click_button is not guaranteed to wait for actions triggered by the button click to finish. To fix this move the following have_text expectation inside the expect { ...}.not_to change.. block.Thomas Walpole

1 Answers

1
votes

This might be better received at https://codereview.stackexchange.com/

That said, I'm not wild about how open_email somehow magically sets current_email. I would prefer that method returned it, and you checked against that return value if possible; this can be a problem with multithreaded apps or test suites.

Your expectations of text on page are pretty brittle. If any of that copy changes, the test will also need to be reworked. Personally, I'm OK with this, but it could potentially save you some effort if you either matched against a semi-lax regular expression, or put those strings into a translation file, and checked against I18n.t('email.confirmation.message').

To improve performance, you may want to use a login method that doesn't use login_via_form, I think Devise test helpers still include login_as that directly sets a logged-in session for you.

Overall, I think this is great work. Keep it up!