Skip to content

Fix Controllers test #130

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix Controllers test #130

wants to merge 2 commits into from

Conversation

Wistlap
Copy link
Contributor

@Wistlap Wistlap commented Apr 10, 2025

Controllers test の失敗していたテストを修正した.

具体的には, test/test_helper.rb 内の OmniAuth mock 作成時に,uiduser.id ではなく user.uid を指定することでほとんどのテストが成功した.(それでもなお失敗したテストについては後述する)
失敗までの流れを以下に示す.

  1. ログインしたいユーザ(例 id: 1, uid: 10)で test login したとき, mock の uid に 1 が保存される.
  2. SessionsControllercreate 時に以下のコードが走る.
    user = User.find_by_provider_and_uid(auth["provider"], auth["uid"]) || User.create_with_omniauth(auth)
  3. User.find_by_provider_and_uid で,uid が 1 のエントリを検索するが,存在しないため,新たなユーザを作成する.
  4. 作成された新規ユーザの id を session[:user_id] に保存する.
  5. current_user? で,ログインしたいユーザと新規ユーザの id を比較し,false が返ってくる.
  6. ログインに失敗し,テストも失敗する.

上記変更でも失敗していた should destroy user について

  • 以前の議論で,ユーザは基本的に削除しない方針になったため,model 側で destroy を無視するよう設定されている.
  • そのため,should_destroy_user を含めたユーザ削除の成功を期待するテストを削除した.
  • 代わりに,ユーザ削除の失敗を期待するテスト should_not_destroy_user を追加した.

+ 失敗していたテストのスキップを外した.
+ test/test_helper.rb のmock作成時に uid を user.id
  にしていた箇所を user.uid に変更することで,ほとんどの
  テストが成功するようになった.
+ 上記変更後も,should_destroy_user は失敗する.
  + 以前の議論により,ユーザは基本的に削除しない方針になった.
  + model 側でユーザ削除をスキップするバリデーションがかかる
    ようになったため,テストが失敗する.
  + エラーへの対処として,削除が成功するテストを削除し,
    代わりに削除できるべきではない should_not_destroy_user
    テストを追加した.
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.

1 participant