2

I'm using Rails 6 and minitest with the built-in system tests (which use Capybara I think) and using FactoryBot as well to generate my test records.

I have a pretty standard password rest feature I'm trying to implement.

I've verified that when I go to the pages in the browser and fill out the information it does indeed change the user's password, but for some reason the password is never changed in the test.

It's almost like the @user object is being cached in the tests and won't reload in the test, but I have no idea why that would be.

Anyone know why this test would fail but the functionality works in "real life" when I manually change a password?

# test/system/password_resets_test.rb
require "application_system_test_case"

class PasswordResetsTest < ApplicationSystemTestCase
  test "change password" do
    original_password = "password"
    new_password = "new-password"
    @user = create(:user, password: original_password, password_reset_token_sent_at: Time.current)

    visit password_reset_path(@user.password_reset_token)
    fill_in "user[password]", with: new_password
    click_on "Update Password"

    assert_equal(@user.reload.password, new_password)
  end
end
# app/views/password_resets/show.html.erb
<%= form_with model: @user, url: password_reset_path(@user.password_reset_token), method: :put do |form| %>
  <div class="field">
    <%= form.label :password, "Password" %><br />
    <%= form.password_field :password, autofocus: true, required: true %>
  </div>
  <div class="field">
    <%= form.submit "Update Password" %>
  </div>
<% end %>
# app/controllers/password_resets_controller.rb
class PasswordResetsController < ApplicationController
  def show
    if @user = User.find_by(password_reset_token: params[:id])
      if @user.password_reset_token_expired?
        flash[:error] = "Your password reset has expired"
        redirect_to new_password_reset_path
      end
    else
      flash[:error] = "Invalid password reset token"
      redirect_to new_password_reset_path
    end
  end

  def update
    @user = User.find_by(password_reset_token: params[:id])
    new_password = password_reset_params[:password]

    # Automatically set `#password_confirmation` so user does not have
    # to enter in password twice on reset page.
    if @user&.update(password: new_password, password_confirmation: new_password)
      let_user_in(@user)
    else
      render :show
    end
  end

  private

  def password_reset_params
    params.require(:user).permit(:password)
  end
# app/models/user.rb
class User < ApplicationRecord
  PASSWORD_RESET_TIME_LIMIT_IN_HOURS = 4.freeze

  has_secure_password
  has_secure_token :password_reset_token

  validates :password,
    presence: true,
    length: { minimum: 8 },
    allow_nil: true

  def password_reset_token_expired?
    return true if password_reset_token_sent_at.blank?
    password_reset_token_sent_at < PASSWORD_RESET_TIME_LIMIT_IN_HOURS.hours.ago
  end
end

Dan L
  • 4,319
  • 5
  • 41
  • 74

1 Answers1

1

click_on doesn't guarantee any actions triggered by the click have happened when it returns. This is because Capybara has no way of knowing what (if any) actions would have been triggered by that click. This means your assertion of the new password is probably happening before the page has even submitted. To fix that you need to use one of the Capybara provided retrying assertions (which assert_equal is not) to check for something visible on the page that indicates the update has occurred.

Something along the lines of

click_on "Update Password"
assert_text "Password Updated!" # whatever message your page shows to indicate successful password update

assert_equal(@user.reload.password, new_password)

should fix your issue.

Thomas Walpole
  • 48,548
  • 5
  • 64
  • 78
  • Thanks for the explanation, I think that was absolutely the problem! The test was checking before the update had actually taken place. When I used a retrying assert (`assert_text`) for the text on the page after updating the password, the test passed and was operating like I expected. – Dan L Jan 12 '20 at 21:45