Railsチュートリアル 第10章 ユーザーの更新・表示・削除 - ユーザーを削除する

何をするか

• ユーザー削除が可能な権限を持つ管理ユーザーのクラスの実装
• ユーザーを削除するためのリンクの追加
• RDBからユーザーを削除する動作の実装

ここまでの実装が完了すれば、Userリソースに対し、RESTが求めるすべての動作の実装が完了することになります。

Railsチュートリアル本文においては、ユーザーを削除するためのリンクを追加したサイトレイアウトのモックアップは、図 10.13に示されています。

管理ユーザー

Userモデルに、boolean型の値を取るadminという属性を追加します。特権を持つ管理ユーザーを識別するために用いる属性です。

boolean型のadmin属性をUserモデルに追加すると、Railsによって、Userモデルのadmin?というメソッドが自動で追加されます。

マイグレーションの生成と修正

続いて、マイグレーションを生成します。

# rails generate migration add_admin_to_users admin:boolean
Running via Spring preloader in process 12240
      invoke  active_record
      create    db/migrate/[timestamp]_add_admin_to_users.rb

db/migrate/[timestamp]_add_admin_to_users.rbというマイグレーションが生成されました。ただ、生成されたマイグレーションに若干の修正を加える必要があります。

db/migrate/[timestamp]_add_admin_to_users.rb

  class AddAdminToUsers < ActiveRecord::Migration[5.1]
    def change
-     add_column :users, :admin, :boolean
+     add_column :users, :admin, :boolean, default: false
    end
  end

default: falseという引数を与えています。「デフォルトでは管理権限はない」ということを明示するためです。

後はマイグレーションを実行すれば、Userモデルにadmin属性が実装され、admin?メソッドも使えるようになります。

# rails db:migrate
== [timestamp] AddAdminToUsers: migrating ==================================
-- add_column(:users, :admin, :boolean)
   -> 0.0152s
== [timestamp] AddAdminToUsers: migrated (0.0186s) =========================

admin?メソッドを試してみる

# rails console --sandbox

>> user = User.first
  User Load (4.6ms)  SELECT  "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT ?  [["LIMIT", 1]]
=> #<User id: 1, ..., admin: nil>
>> user.admin?
=> false
>> user.toggle!(:admin)
   (0.2ms)  SAVEPOINT active_record_1
  SQL (37.0ms)  UPDATE "users" SET "updated_at" = ?, "admin" = ? WHERE "users"."id" = ?  [["updated_at", "2019-11-28 21:42:04.018070"], ["admin", "t"], ["id", 1]]
   (0.1ms)  RELEASE SAVEPOINT active_record_1
=> true
>> user.admin
=> true

ここでは、toggle!メソッドを使って、id=1のユーザーのadmin属性値をfalseからtrueに反転しています。follow_redirect!やfind_by!等の!とは異なり、この場合の!は「破壊的代入」という意味ですね。

id=1のユーザーのみ、デフォルトで管理者とする

まず、db/seeds.rbの内容を変更し、id=1のユーザーをデフォルトで管理者とするようにサンプルデータ生成タスクの内容を変更します。

db/seeds.rb

  User.create(name:                  "Example User",
              email:                 "[e-mail]",
              password:              "foobar",
-             password_confirmation: "foobar")
+             password_confirmation: "foobar",
+             admin: true)

  99.times do |n|
    name = Faker::Name.name
    email = "example-#{n+1}@railstutorial.org"
    password = "password"
    User.create!( name:                  name,
                  email:                 email,
                  password:              password,
                  password_confirmation: password)
  end

次に、データベースをリセットし、サンプルデータを再生成します。

# rails db:migrate:reset
Dropped database 'db/development.sqlite3'
Dropped database 'db/test.sqlite3'
Created database 'db/development.sqlite3'
Created database 'db/test.sqlite3'
...略

# rails db:seed

rails db:seedは、正常に完了した場合、シェルには何も表示されません。

なお、私はここで少々つまづきました。顛末はdb/seeds.rbの中身を変更したらrails db:seedできない。そんなときにありがちな原因に記述ています。

Strong Parametersにより、admin属性を保護する

特に対策をしていない場合、例えば以下のようなPATCHリクエストにより、任意のユーザーのadmin属性の値を変更することができてしまいます。

patch /users/17?admin=1

UNIX系OSのroot権限もそうですが、「任意のユーザーが、任意の属性を変更可能である」というのは非常に危険な状態です。なんとしても「一般ユーザーが編集できてはいけない属性を編集できないようにする」という仕組みを実装する必要があります。

4.0以降のRailsにおいては、「一般ユーザーが編集できてはいけない属性を編集できないようにする」という要求を実現する仕組みとして、Strong Parametersという機能が実装されています。Railsチュートリアル本文では、節7.3.2で言及されていました。

Railsチュートリアルをここまで順番通りに進めていれば、「Strong Parametersによってadmin属性を保護する」という仕組みは、サンプルアプリケーションにすでに実装されているはずです。コードは以下です。

app/controllers/users_controller.rb

class UsersController < ApplicationController
  # ...略
  private

    def user_params
      params.require(:user).permit(:name, :email, :password, :password_confirmation)
    end
    # ...略
end

user_paramsメソッド内のparams.require(:user).permitの引数に:adminが含まれていないことがポイントです。この実装により、「任意のユーザーが自分に管理者権限を与えること」は防止されています。

演習 - 管理ユーザー

1.1. Web経由でadmin属性を変更できないことを確認するテストを実装するために、まずはadminをuser_paramsメソッド内の許可されたパラメータ一覧に追加するところから始めてみましょう。

app/controllers/users_controller.rb

  class UsersController < ApplicationController
    ...略

    private

      def user_params
-       params.require(:user).permit(:name, :email, :password, :password_confirmation)
+       params.require(:user).permit(:name, :email, :password, :password_confirmation, :admin)
      end
      ...略
  end

1.2. Web経由でadmin属性を変更できないことを確認してみましょう。

具体的には、リスト 10.56に示したように、PATCHを直接ユーザーのURL (/users/:id) に送信するテストを作成してみてください。

以下のようなテストをtest/controllers/users_controller_test.rbに追加します。

test "should not allow the admin attribute to be edited via the web" do
  log_in_as(@other_user)
  assert_not @other_user.admin?
  patch user_path(@other_user), params: { user: { password:              'password',
                                                  password_confirmation: 'password',
                                                  admin: true } }
  assert_not @other_user.reload.admin?
end

ポイントは以下です。

• 以下の値は、@other_user.passwordとするとupdateが正常に完了しない
  • params[:user][:password]
  • params[:user][:password_confirmation]
• reloadにより、RDBに保存された@other_userを再度読み込む必要がある

特にparams[:user][:password]およびparams[:user][:password_confirmation]の値については、1時間くらい悩んでしまいました。

このようなときに決め手となるのはやはりdebuggerですね。

app/controllers/users_controller.rb#update

  def update
+   debugger
    if @user.update_attributes(user_params)
      flash[:success] = "Profile updated"
      redirect_to @user
    else
      render 'edit'
    end
  end

(byebug) user_params
<ActionController::Parameters {"password"=>nil, "password_confirmation"=>nil, "admin"=>"true"} permitted: true>

"password"=>nil, "password_confirmation"=>nil

ここを見てようやく気づきました。params[:user][:password]とparams[:user][:password_confirmation]には、生文字列を与えなければダメなのだと…。

テストが正しい振る舞いをしているかどうか確信を得るために、最初のテストの結果はredになるはずです。

# rails test test/controllers/users_controller_test.rb:35
Running via Spring preloader in process 12914
Started with run options --seed 32377

 FAIL["test_should_not_allow_the_admin_attribute_to_be_edited_via_the_web", UsersControllerTest, 0.8165606999828015]
 test_should_not_allow_the_admin_attribute_to_be_edited_via_the_web#UsersControllerTest (0.82s)
        Expected true to be nil or false
        test/controllers/users_controller_test.rb:41:in `block in <class:UsersControllerTest>'

  7/7: [===================================] 100% Time: 00:00:00, Time: 00:00:00

Finished in 0.82068s
1 tests, 2 assertions, 1 failures, 0 errors, 0 skips

悩み悩んだ挙げ句、ようやくテストが失敗してくれました。

test/controllers/users_controller_test.rb（41行目）

assert_not @other_user.reload.admin?

現在の私の環境では、test/controllers/users_controller_test.rbの41行目は上記コードです。というわけで、テスト失敗時のメッセージの内容も、:admin属性の値がtrueになっていますという内容です。ここまで長かった。

user_paramsの内容を本来の実装に戻して、もう一度テスト

app/controllers/users_controller.rb

  class UsersController < ApplicationController
    ...略

    private

      def user_params
-       params.require(:user).permit(:name, :email, :password, :password_confirmation, :admin)
+       params.require(:user).permit(:name, :email, :password, :password_confirmation)
      end

      ...略
  end

# rails test test/controllers/users_controller_test.rb:35
Running via Spring preloader in process 12927
Started with run options --seed 30515

  7/7: [===================================] 100% Time: 00:00:00, Time: 00:00:00

Finished in 0.82063s
1 tests, 2 assertions, 0 failures, 0 errors, 0 skips

今度はテストが成功しました。

destroyアクション

indexビュー用のuserパーシャルに、ユーザー削除用のリンクを追加する

続いて編集するのは、indexビュー用のuserパーシャルです。追加するのはユーザー削除用のリンクで、「管理者のみに表示される」という要件を満たす必要があります。

ユーザー削除用のリンクの実体

<% if current_user.admin? && !current_user?(user) %>
  | <%= link_to "delete", user, method: :delete, data: { confirm: "You sure?" } %>
<% end %>

重要と思われるポイントは以下です。

• if文について
  • 「管理者のみに表示される」という要件を満たすために、current_user.admin?という記述がなされている
  • !current_user?(user)というのは、「現在ログイン中のユーザーが自分自身を削除することはできない」ことを意味する
• DELETEリクエストを発行するリンクの生成は、link_toの引数にmethod: :deleteという記述があることがポイントとなる
• 確認メッセージを表示するようにしている

2019年現在のHTMLの仕様では、HTMLのフォームは直接DELETE（あるいはPATCHやPUT）リクエストを発行することはできません。そのため、Railsその他Webフレームワークでは、フレームワーク内部で実装された何らかの仕組みによってDELETEリクエストを発行するようにしています。

実際にindexビュー用のuserパーシャルの内容を変更する

app/views/users/_user.html.erbに、以下の変更を加えていきます。

app/views/users/_user.html.erb

  <li>
    <%= gravatar_for user, size: 50 %>
    <%= link_to user.name, user %>
+   <% if current_user.admin? && !current_user?(user) %>
+     | <%= link_to "delete", user, method: :delete, data: { confirm: "You sure?" } %>
+   <% end %>
  </li>

（管理ユーザーでログインしている場合）deleteリンクがindexに表示されるようになる

ここまで実装が完了すれば、（管理ユーザーでログインしている場合に）deleteリンクがindexに表示されるようになるようになります。

destroyアクションの実装

deleteリンクをクリックして実際にRDBからユーザーが削除されるようにするためには、Usersコントローラーにdestroyアクションを実装する必要があります。

destroyアクションの実装内容は以下のとおりになります。

UsersController#destroy

def destroy
  User.find(params[:id]).destroy
  flash[:success] = "User deleted"
  redirect_to users_url
end

User.find.destroyというメソッドチェーンが「RDBからユーザーを削除する」という動作の実体ですね。以降、「フラッシュメッセージの定義」からの「indexページへのリダイレクト」へと続きます。

destroyアクションをbeforeフィルターの対象にする

destroyアクションも、editやupdateと同様に「ログインユーザーのみが実行できる」ようにする必要があります。コードは以下です。

ログインユーザーでなければdestroyアクションを実行できない

before_action :logged_in_user, only: [:index, :edit, :update, :destroy]

管理ユーザーでなければdestroyアクションを実行できない

さらに、「destroyアクションは管理ユーザーのみが実行できる」という保護も必要となります。「cURLなどのツールを使って、直接DELETEリクエストを対象リソースに送りつける」という想定外のアクセスを防ぐ必要があるためです。コードは以下です。

before_action :admin_user, only: :destroy

:admin_userを第一引数とするbefore_actionにおいて、:onlyをキーとするハッシュに与えているのは配列ではありません。:destroyというシンボルを直接与えています。この点は、:logged_in_userや:correct_userを第一引数とするbefore_actionとは異なっています。

admin_userメソッドそのものの定義

admin_userメソッドそのものの定義は以下です。

def admin_user
  redirect_to(root_url) unless current_user.admin?
end

「管理ユーザーでなければ / にリダイレクトされる」という動作になります。

以上の実装をapp/controllers/users_controller.rbに反映する

app/controllers/users_controller.rb全体の変更内容は以下のとおりです。

app/controllers/users_controller.rb

  class UsersController < ApplicationController
-   before_action :logged_in_user, only: [:index, :edit, :update]
+   before_action :logged_in_user, only: [:index, :edit, :update, :destroy]
    before_action :correct_user,   only: [:edit, :update]
+   before_action :admin_user,     only: :destroy

    ...略
+
+   def destroy
+     User.find(params[:id]).destroy
+     flash[:success] = "User deleted"
+     redirect_to users_url
+   end

    private

      ...略
+
+     # 管理者かどうか確認
+     def admin_user
+       redirect_to(root_url) unless current_user.admin?
+     end
  end

演習 - destroyアクション

1. 管理者ユーザーとしてログインし、試しにサンプルユーザを２〜３人削除してみましょう。ユーザーを削除すると、Railsサーバーのログにはどのような情報が表示されるでしょうか?

Started DELETE "/users/100" ...略
Processing by UsersController#destroy as HTML
  Parameters: {"authenticity_token"=>"rhQj0KvhGPwbu2uDnnux9jBIBpt6RC00ly1hoqSWPiE3EHv2wyrDvy003EsFWoANTfm79aAsGcTQbHQUpIO9ng==", "id"=>"100"}
  User Load (2.5ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = ? LIMIT ?  [["id", 1], ["LIMIT", 1]]
  User Load (2.5ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = ? LIMIT ?  [["id", 100], ["LIMIT", 1]]
   (1.2ms)  begin transaction
  SQL (11.4ms)  DELETE FROM "users" WHERE "users"."id" = ?  [["id", 100]]
   (8.3ms)  commit transaction
Redirected to http://localhost:8080/users
Completed 302 Found in 119ms (ActiveRecord: 39.5ms)


Started GET "/users" ...略
Processing by UsersController#index as HTML
  User Load (2.4ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = ? LIMIT ?  [["id", 1], ["LIMIT", 1]]
  Rendering users/index.html.erb within layouts/application
   (2.5ms)  SELECT COUNT(*) FROM "users"
  User Load (2.3ms)  SELECT  "users".* FROM "users" LIMIT ? OFFSET ?  [["LIMIT", 30], ["OFFSET", 0]]
  ...略
Completed 200 OK in 370ms (Views: 342.7ms | ActiveRecord: 7.2ms)

• /users/100 へのDELETEリクエストに対して
  • idが100であるユーザーを対象として、SQLのDELETE文が発行されている
  • 最終的には /users へのリダイレクトでDELETEリクエストが完了している
• /usersへのGETリクエストに対して
  • 特に変わった動作はない

フラッシュメッセージの内容

UsersController#destroy

  def destroy
    User.find(params[:id]).destroy
    flash[:success] = "User deleted"
+   debugger
    redirect_to users_url
  end

以上のようにして、flash[:success]の内容を確認してみましょう。

(byebug) flash[:success]
"User deleted"

フラッシュメッセージの内容にも問題ないようですね。

ユーザー削除のテスト

fixture内で最初に登場するユーザーを管理ユーザーとする

ユーザー削除、すなわちdestroyアクションの動作をテストするためには、fixtureにも管理ユーザーが必要となります。最初に出てくるユーザー、ここでは:rhakureiさんを管理ユーザーとしましょう。

  rhakurei:
    name: Reimu Hakurei
    email: [e-mail]
    password_digest: <%= User.digest('password') %>
+   admin: true

  mkirisame:
    name: Marisa Kirisame
    email: [e-mail]
    password_digest: <%= User.digest('password') %>

  skomeiji:
    name: Satori Komeiji
    email: [e-mail]
    password_digest: <%= User.digest('password') %>

  rusami:
    name: Renko Usami
    email: [e-mail]
    password_digest: <%= User.digest('password') %>

  <% 30.times do |n| %>
  user_<%= n %>:
    name:  <%= "User #{n}" %>
    email: <%= "user-#{n}@example.com" %>
    password_digest: <%= User.digest('password') %>
  <% end %>

管理ユーザーでないユーザーがdestroyアクションを実行しようとした場合に対するテスト

表題のような操作は、Usersコントローラーによって拒否されます。そのため、テストの実装箇所はtest/controllers/users_controller_test.rbとなります。

非ログインユーザーがdestroyアクションを実行しようとした場合に対するテスト

test "should redirect destroy when not logged in" do
  assert_no_difference 'User.count' do
    delete user_path(@user)
  end
  assert_redirected_to login_url
end

テストの内容は以下となります。

• @userに対してDELETEリクエストを発行し、前後でユーザー数が変わっていなければOK
• @userに対してDELETEリクエストを発行した後に、 /login にリダイレクトされればOK

ログイン済みの非管理ユーザーがdestroyアクションを実行しようとした場合に対するテスト

test "should redirect destroy when logged in as a non-admin" do
  log_in_as(@other_user)
  assert_no_difference 'User.count' do
    delete user_path(@user)
  end
  assert_redirected_to root_url
end

こちらのテストの内容は以下となります。

• @userに対してDELETEリクエストを発行し、前後でユーザー数が変わっていなければOK
• @userに対してDELETEリクエストを発行した後に、 / にリダイレクトされればOK

ここまでのテストを実装する

上記の記述を踏まえ、test/controllers/users_controller_test.rbに実際のテスト内容を実装していきます。

test/controllers/users_controller_test.rb

  require 'test_helper'

  class UsersControllerTest < ActionDispatch::IntegrationTest

    def setup
      @user       = users(:rhakurei)
      @other_user = users(:mkirisame)
    end

    ...略
+
+   test "should redirect destroy when not logged in" do
+     assert_no_difference 'User.count' do
+       delete user_path(@user)
+     end
+     assert_redirected_to login_url
+   end
+
+   test "should redirect destroy when logged in as a non-admin" do
+     log_in_as(@other_user)
+     assert_no_difference 'User.count' do
+       delete user_path(@user)
+     end
+     assert_redirected_to root_url
+   end
  end

実装内容に問題がなければ、この時点でテストは成功するはずです。

# rails test test/controllers/users_controller_test.rb
Running via Spring preloader in process 13026
Started with run options --seed 37178

  9/9: [===================================] 100% Time: 00:00:02, Time: 00:00:02

Finished in 2.68050s
9 tests, 18 assertions, 0 failures, 0 errors, 0 skips

管理ユーザーがdestroyアクションを実行する場合に対するテスト、および、indexビューにおける"delete"リンクの表示に関するテスト

表題のパターンではdestroyアクションが成功します。そのため、テストによる影響範囲は、「Usersコントローラーの動作」のみならず、「Userモデルの内容」にまで及びます。コントローラーとモデルの双方に影響範囲が及ぶテストなので、実装箇所は統合テストとなります。今回は「/index 上のリンクからdestroyアクションを呼び出す」という動作に対するテストなので、より詳細な実装箇所はtest/integration/users_index_test.rbです。

また、test/integration/users_index_test.rbに実装するテストには、「indexビューに削除リンクが表示されるか否か」というテストも含まれます。「管理ユーザーであれば、indexビューで、自身以外の各ユーザーの削除リンクが表示される。管理ユーザーでなければ、indexビューに削除リンクは表示されない。」というのが正しい実装となります。

管理ユーザーに対する統合テスト

以下の動作に対するテストが必要となります。

• 一覧内の、自身以外のユーザーに対して削除リンクが表示される
• @non_adminというユーザーに対してDELETEリクエストが送信された場合、実際にRDBから当該ユーザーが削除される

対応するテストのコードは以下となります。

test "index as admin including pagination and delete links" do
  log_in_as(@admin)
  get users_path
  assert_template 'users/index'
  assert_select 'div.pagination'
  first_page_of_users = User.paginate(page: 1)
  first_page_of_users.each do |user|
    assert_select 'a[href=?]', user_path(user), text: user.name
    unless user == @admin
      assert_select 'a[href=?]', user_path(user), text: 'delete'
    end
  end
  assert_difference 'User.count', -1 do
    delete user_path(@non_admin)
  end
end

なお、このテストには、「indexビューに実装された、ページネーション機能に対するテスト」も含まれます。個人的には、「一つのテストコードで多くの機能に対するテストを詰め込みすぎではないか」という気はします。

非管理ユーザーのindexビューに対するテスト

非管理ユーザーのindexビューに対しては、以下の動作に対するテストが必要となります。

• 一覧内に削除リンクが表示されない

対応するテストのコードは以下になります。

test "index as non-admin" do
  log_in_as(@non_admin)
  get users_path
  assert_select 'a', text: 'delete', count: 0
end

個人的には、テストの粒度はこれくらいが丁度いい気がします。

test/integration/users_index_test.rbの内容

差分ではなく、test/integration/users_index_test.rb全体を丸ごと書き換えてしまいます。コードは以下のとおりです。

`test/integration/users_index_test.rb`

require 'test_helper'

class UsersIndexTest < ActionDispatch::IntegrationTest
  def setup
    @admin = users(:rhakurei)
    @non_admin = users(:mkirisame)
  end

  test "index as admin including pagination and delete links" do
    log_in_as(@admin)
    get users_path
    assert_template 'users/index'
    assert_select 'div.pagination'
    first_page_of_users = User.paginate(page: 1)
    first_page_of_users.each do |user|
      assert_select 'a[href=?]', user_path(user), text: user.name
      unless user == @admin
        assert_select 'a[href=?]', user_path(user), text: 'delete'
      end
    end
    assert_difference 'User.count', -1 do
      delete user_path(@non_admin)
    end
  end

  test "index as non-admin" do
    log_in_as(@non_admin)
    get users_path
    assert_select 'a', text: 'delete', count: 0
  end
end

この時点でテストは成功する

test/integration/users_index_test.rbに対してテストを実行してみましょう。

# rails test test/integration/users_index_test.rb
Running via Spring preloader in process 13117
Started with run options --seed 23568

  2/2: [===================================] 100% Time: 00:00:02, Time: 00:00:02

Finished in 2.94128s
2 tests, 63 assertions, 0 failures, 0 errors, 0 skips

ここまでの実装内容に問題がなければ、テストは成功するはずです。

演習 - ユーザー削除のテスト

1. 試しにリスト 10.59にある管理者ユーザーのbeforeフィルターをコメントアウトしてみて、テストの結果がredに変わることを確認してみましょう。

app/controllers/users_controller.rb

  class UsersController < ApplicationController
    before_action :logged_in_user, only: [:index, :edit, :update, :destroy]
    before_action :correct_user,   only: [:edit, :update]
-   before_action :admin_user,     only: :destroy
+   # before_action :admin_user,     only: :destroy

    ...略
  end

# rails test
Running via Spring preloader in process 13156
Started with run options --seed 29997

 FAIL["test_should_redirect_destroy_when_logged_in_as_a_non-admin", UsersControllerTest, 3.8563491000095382]
 test_should_redirect_destroy_when_logged_in_as_a_non-admin#UsersControllerTest (3.86s)
        "User.count" didn't change by 0.
        Expected: 34
          Actual: 33
        test/controllers/users_controller_test.rb:68:in `block in <class:UsersControllerTest>'

  43/43: [=================================] 100% Time: 00:00:04, Time: 00:00:04

Finished in 4.65755s
43 tests, 182 assertions, 1 failures, 0 errors, 0 skips

テストは確かに失敗します。test/controllers/users_controller_test.rbの「should redirect destroy when logged in as a non-admin」というテストで失敗しているようですね。当該テストのコードは以下です。

test "should redirect destroy when logged in as a non-admin" do
  log_in_as(@other_user)
  assert_no_difference 'User.count' do
    delete user_path(@user)
  end
  assert_redirected_to root_url
end

当該テストの要求は、「非管理ユーザーがDELETEリクエスト（→destroyアクション）を送出しても、RDBのレコード件数は変化してはならない」というものです。コードは以下です。

assert_no_difference 'User.count' do
  delete user_path(@user)
end

しかしながら、管理者ユーザーのbeforeフィルターがコメントアウトされていると、「非管理ユーザーがDELETEリクエスト（→destroyアクション）を送出しても、RDBのレコード件数が変化する」という事態が発生してしまいます。

"User.count" didn't change by 0.
  Expected: 34
    Actual: 33

そのため、テストが失敗するのです。