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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/models/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@

class Task < ApplicationRecord
enum status: { waiting: 1, work_in_progress: 2, completed: 3 }

validates :name, presence: true, length: { maximum: 20 }
end
7 changes: 7 additions & 0 deletions db/migrate/20190531002052_change_column_to_task.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class ChangeColumnToTask < ActiveRecord::Migration[5.2]
def change
change_column :tasks, :name, :string, limit: 20
end
end
4 changes: 2 additions & 2 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20_190_521_053_254) do
ActiveRecord::Schema.define(version: 20_190_531_002_052) do
create_table 'tasks', options: 'ENGINE=InnoDB DEFAULT CHARSET=utf8', force: :cascade do |t|
t.string 'name', null: false
t.string 'name', limit: 20, null: false
t.text 'description'
t.integer 'status', limit: 1, default: 1, null: false, unsigned: true
t.datetime 'created_at', null: false
Expand Down
93 changes: 93 additions & 0 deletions spec/models/task_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe Task, type: :model do
describe '#save' do
let(:task) { build(:task, name: 'a' * 1) }

before 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

it 'creates records in task' do
expect(Task.count).to eq(1)
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

let(:task) { build(:task, name: nil) }

it '必須のエラーメッセージが出ること' do
expect(Task.count).to eq(0)
expect(task.errors[:name]).to include('を入力してください')
end
end

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

it 'creates records in task' do
expect(Task.count).to eq(1)
expect(task.errors.count).to eq(0)
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) { build(:task, name: 'a' * 21) }

it '桁数超過のエラーメッセージが出ること' do
expect(Task.count).to eq(0)
expect(task.errors[:name]).to include('は20文字以内で入力してください')
end
end

context 'statusへ既定値以外(-1)の入力があると' do
it 'statusにマイナスの値で永続化できないこと' do
expect { task.assign_attributes(status: -1) }.to raise_error(ArgumentError)
end
end

context 'statusへ既定値以外(0)の入力があると' do
it 'statusに0の値で永続化できないこと' do
expect { task.assign_attributes(status: 0) }.to raise_error(ArgumentError)
end
end

context 'statusへ1(waiting)の入力があると' do
let(:task) { build(:task, status: :waiting) }

it 'creates records in task' do
expect(Task.count).to eq(1)
expect(task.reload.status).to eq('waiting')
expect(task.errors.count).to eq(0)
end
end

context 'statusへ2(work_in_progress)の入力があると' do
let(:task) { build(:task, status: :work_in_progress) }

it 'creates records in task' do
expect(Task.count).to eq(1)
expect(task.reload.status).to eq('work_in_progress')
expect(task.errors.count).to eq(0)
end
end

context 'statusへ3(completed)の入力があると' do
let(:task) { build(:task, status: :completed) }

it 'creates records in task' do
expect(Task.count).to eq(1)
expect(task.reload.status).to eq('completed')
expect(task.errors.count).to eq(0)
end
end

context 'statusへ既定値以外(4)の入力があると' do
it 'statusに4の値で永続化できないこと' do
expect { task.assign_attributes(status: 4) }.to raise_error(ArgumentError)
end
end
end
end