Skip to content
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

tasks.nameに必須と桁数のバリデーション処理を入れる #339

Merged
merged 5 commits into from Jun 3, 2019

Conversation

masahikokawai
Copy link

ステップ11: バリデーションを設定してみよう

■桁数超過

スクリーンショット 0031-05-31 午前10 14 04

■必須

スクリーンショット 0031-05-31 午前10 14 15

デザインは何もしてないです。
(field_with_errors を自動で差し込んでくれるのに対して)

@masahikokawai masahikokawai self-assigned this May 31, 2019
@masahikokawai masahikokawai changed the title task.nameに必須と桁数のバリデーション処理を入れる tasks.nameに必須と桁数のバリデーション処理を入れる May 31, 2019

RSpec.describe Task, type: :model do
describe '#save' do
let(:task) { described_class.new(name: 'hoge') }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

factorybotを使用すると後々のテストが楽になると思いました。
https://qiita.com/Ushinji/items/522ed01c9c14b680222c

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

factorybot を使用するように修正しました。
8afae2c

expect(task.errors.count).to eq(0)
end

context 'nameへの入力がない' do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

境界値テストとして1文字の場合もテストした方が良さそうです。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context で括ってない正常系のテストを1文字のケースとして変更しました。
8afae2c

end
end

context 'nameへ21文字以上の入力があると' do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

境界値テストとして20文字の場合もテストした方が良さそうです。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#339 (comment)
こちらも上記に合わせてケースを追加しています。

let(:task) { described_class.new(name: 'hoge') }

it 'creates records in task' do
task.save
Copy link

@hoooori hoooori Jun 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7行目の下に
before { task.save }
を定義してsave処理を共通化すれば、各テストからtask.saveを削除できるのでコンパクト化できそうです。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#339 (comment)
こちらも合わせて修正しました。

コンパクト化できそうです。

ちなみに個人的にspecは何もかもDRYにする事を目指すより
可読性など見やすさに重きを置いても良いのかなと思ってます。(あくまで個人的にです・・)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修正ありがとうございます。

可読性など見やすさに重きを置いても良いのかな

おっしゃる通りだと思います。
過度なリファクタリングは避けた方が良さそうです。

end

context 'nameへ21文字以上の入力があると' do
let(:task) { described_class.new(name: '12345678901234567890a') }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nameは実際のデータを入力するよりも
(name: 'a' * 21)
とした方がコンパクト且つ視覚的に分かりやすいですー

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ですね、視覚的にもわかりやすいですね!
#339 (comment)
こちらも合わせて修正しています。

@hoooori
Copy link

hoooori commented Jun 3, 2019

@masahikokawai
コメントしましたので確認をお願いします:bow:

また2点お伝えしたいことがあります。

  1. 初回bundle installしたところ下記エラーが発生し、下のリンクを参考に解決しました。もしかしたらGemfile.lockの修正が必要かもしれません。

Your bundle is locked to ffi (1.11.0), but that version could not be found in any of the sources listed in your Gemfile.

原因:最近ffi-1.11.0がrubygemsから削除されたらしい
対応策:ffiを1.11.1にupdate
参考文献:ffi/ffi#701

  1. Taskモデルのstatusに対するバリデーション設定とモデルテストの追加は、後のステップで実施する予定でしょうか?

・境界値のケースを追加
・it内のtask.saveの記述の修正
@masahikokawai
Copy link
Author

Taskモデルのstatusに対するバリデーション設定とモデルテストの追加は、後のステップで実施する予定でしょうか?

statusのspecも追加しました。
69ef535

@masahikokawai
Copy link
Author

  1. 初回bundle installしたところ下記エラーが発生し、下のリンクを参考に解決しました。もしかしたらGemfile.lockの修正が必要かもしれません。
    Your bundle is locked to ffi (1.11.0), but that version could not be found in any of the sources listed in your Gemfile.
    原因:最近ffi-1.11.0がrubygemsから削除されたらしい
    対応策:ffiを1.11.1にupdate
    参考文献:ffi-1.11.0 ffi/ffi#701

情報ありがとうございます!
私の環境では出てないんですよね。。今回のPRでは一旦無視してもいいですかね・・?
(これまでに安福さん三谷さんにレビューしていただいた事あるんですけど、その時どうだったんだろ・・・)

end

context 'nameへ20文字の入力があると' do
let(:task) { build(:task, name: 'a' * 20, status: 2) }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こちらは、nameに対するテストですので、
letにはnameのみ設定した方が良さそうです。(statusも設定するとテストが読みづらい気がします)

もしよろしければ、参考にしてください
https://github.com/Fablic/training/blob/hoooori/task_app/spec/models/task_spec.rb

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

テストのケースきちんと書いていて素晴らしいです!!
参考にしました!

statusのケースを分けるように修正しています。
b8fdad9

@hoooori
Copy link

hoooori commented Jun 3, 2019

@masahikokawai
statusのspec追加、ありがとうございます!
bundle installの件、一旦スルーで問題ありません:bow:

1点コメントしましたので確認をお願いします。

@hoooori
Copy link

hoooori commented Jun 3, 2019

@masahikokawai
テスト修正の対応ありがとうございます!
validation設定・テスト共に問題なさそうです。
LGTM!!

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants