11月4日に入社しましたエンジニアの越川です。 本日は、私がコードを書く際に気をつけていることを書こうと思ったのですが、どう書くかに迷った末、ブログ記事駆動ハッカソンを行うことでそのプロセスで考えいたことなどを記事にすることにしました。
私はコードを書く際におもてなしの心が大事だと思っています。おもてなしの心は最終的なアウトプットに触れるユーザーはもちろんのこと、チームメンバー、未来の自身へ向けた思いやりです。
今回題材とするのは、KPTツールです。KPTは、日々の活動を振り返り「Keep」すべきこと「Problem」であること「Try」したいことを上げて、活動を振り返るために使われます。
先日行われた「オレたちが目指す”最強のエンジニアドリブン”」を終えた後に振り返りでKPT使ったので、それをWebアプリにしてみます。
今回のコードはすべて、ppworks/furikaeri/tree/v1.0.2に置いてあります。
- 機能一覧
- 準備
- databaseのお掃除設定を足す
- FactoryGirlを便利に使う設定を足す
- 初めてのテスト実行
- 最初のテストを書く
- 開発の流れ
- Project を作る
- Project#key にユニークな値を設定する処理を追加する
- ProjectsController を作る
- Issue を作る
- IssuesController を作る
- もっとテストしたい
- 残りのテスト
- 気になる点を直す
- 画面は?
- 終わりに
機能一覧
- ユーザーはprojectを作成する事ができる
- システムはprojectに対する一意のURLを発行する
- ユーザーは(Keep, Problem, Try)から選択してissueを登録できる
- ユーザーは登録したissueを削除できる
最低限必要な機能はここまで。
準備
今回は、rspec
でテストを書きます。feature
テストにはcapybara
を使ってみましょう。以下のように最低限必要なgem
を用意します。
Gemfile
source 'https://rubygems.org' gem 'rails', '4.1.8' gem 'pg' gem 'sass-rails', '~> 4.0.3' gem 'uglifier', '>= 1.3.0' gem 'coffee-rails', '~> 4.0.0' gem 'jquery-rails' gem 'jbuilder', '~> 2.0' group :development, :test do gem 'spring' gem 'rspec-rails' gem 'factory_girl_rails' gem 'faker' gem 'database_rewinder' gem 'capybara' end
rspec/rspec-railsにあるように、rspec
の設定ファイルをインストールします。
$ rails g rspec:install
spec/spec_helper.rb
とspec/rails_helper.rb
が生成されるので、不要な箇所を消すとこんな感じになります。
spec/rails_helper.rb
ENV["RAILS_ENV"] ||= 'test' require 'spec_helper' require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' ActiveRecord::Migration.maintain_test_schema! RSpec.configure do |config| config.fixture_path = "#{::Rails.root}/spec/fixtures" config.use_transactional_fixtures = true config.infer_spec_type_from_file_location! end
databaseのお掃除設定を足す
feature testを書く際に便利となる、amatsuda/database_rewinderを使い、データベースを綺麗にする設定を足します。
config.fixture_path = "#{::Rails.root}/spec/fixtures" - config.use_transactional_fixtures = true + config.use_transactional_fixtures = false config.infer_spec_type_from_file_location! + + config.before(:suite) do + DatabaseRewinder.strategy = :truncation + end + + config.before(:each) do + DatabaseRewinder.clean_all + load "#{Rails.root}/db/seeds.rb" + end + + config.after(:each) do + DatabaseRewinder.clean + end end
FactoryGirlを便利に使う設定を足す
thoughtbot/factory_girlでダミーデータを用意します。テストの中で、FactoryGirl.create
と書かずに、create
と書けるようになり便利です。
config.after(:each) do
DatabaseRewinder.clean
end
+ config.include FactoryGirl::Syntax::Methods
初めてのテスト実行
$ rspec
No examples found. Finished in 0.0002 seconds (files took 0.0468 seconds to load) 0 examples, 0 failures
全てはここからです。
最初のテストを書く
まだ何もないので、トップページを作りましょう。トップページに「ふりかえり」と表示されることを確認します。
なんてことないcontrollerとview、それとroutesを設定しましょう。
app/controllers/pages_controller.rb
class PagesController < ApplicationController end
app/views/pages/top.html.haml
ふりかえり
config/routes.rb
Rails.application.routes.draw do root to: 'pages#top' end
テストは以下のように書いてみました。
feature
でテストしたい機能を明記しますscenario
でどんな手続で何をexpect
期待するのかを書くと分かりやすいと思います。
このケースでは、/
にアクセスすると、ふりかえり
という文字列が表示されることを確認してます。
spec/features/root_spec.rb
require 'rails_helper' describe 'Root' do feature 'visit / path' do scenario do visit root_path expect(page).to have_content 'ふりかえり' end end end
rspec spec/features/root_spec.rb
Root visit / path should text "ふりかえり" Finished in 0.37442 seconds (files took 1.4 seconds to load) 1 examples, 0 failures
無事通りました。
開発の流れ
準備はここまで、開発に移りましょう。
- Projectを作る
- ProjectsControllerを作る
- Issueを作る
- IssuesControllerを作る
という流れで作ってみます。
Project を作る
rails g model project name:string key:string
migrationファイルを開いて、null: false
オプションを追加したり、add_index
でインデックスを貼っておきます。
db/migrate/20141126155514_create_projects.rb
class CreateProjects < ActiveRecord::Migration def change create_table :projects do |t| t.string :name, null: false t.string :key, null: false t.timestamps end add_index :projects, :key, unique: true end end
ダミーデータを用意して、
spec/factories/projects.rb
FactoryGirl.define do factory :project, class: 'Project' do name { Faker::Name.title } key 'unique key value' end end
テストはこんな感じ。
spec/models/project_spec.rb
require 'rails_helper' describe Project do describe 'create project' do subject { create(:project) } it { expect(subject).to be_persisted } end end
単体テストでは、subject(対象)
を意識して書くと分かりやすいです。何を対象に何をテストしたいのかを明確にします。 仲間が読んだ時に意図が伝わるようなにコードを書く ことを大切にしましょう。
$ rspec
Root visit / path should text "furikaeri" Project create project should be persisted Finished in 0.28907 seconds (files took 1.45 seconds to load) 2 examples, 0 failures
いいですね。
ここで今回、Project#key
は、ユニークであることが大事なので、その検証をしておきたいと思いました。
require 'rails_helper' describe Project do describe 'create project' do subject { create(:project) } it { expect(subject).to be_persisted } end + describe 'create projects' do + let(:project_count) { 10 } + subject { create_list(:project, project_count) } + it do + expect { + subject + }.not_to raise_error + end + end end
「ある振る舞いの結果、何かを期待する」場合には以下の様な記述が読みやすいかなーと思っております。
subject { cool_object.awesome_method } it 'テストするぞ! do expect { subject }.to change(Hoge, :fugo).from(:piyo).to(:puni) end
同じように、ある振る舞いの結果、あるエラーが発生するというテストは以下のようにかけるので、今回はエラがー起きないことをnot_to
を使い記述しました。
subject { cool_object.awesome_method } it '例外が発生しちゃうテストするぞ! do expect { subject }.to raise_error(RuntimeError) end
テストにおけるnot
は「ある事柄ではない」すべてがマッチするケースが通ってしまうので、慎重に使う必要があります。今回のケースでは例外が発生しないことだけを見ておきたいのであえて使いました。そういった 事情はコメントに書く と良いかもしれません。
さて、実行すると
rspec spec/models/project_spec.rb
Project create project should be persisted create projects should not raise Exception (FAILED - 1) Failures: 1) Project create projects should not raise Exception Failure/Error: expect { expected no Exception, got #<ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "index_projects_on_key" DETAIL: Key (key)=(indentified_key) already exists. : INSERT INTO "projects" ("created_at", "key", "name", "updated_at") VALUES ($1, $2, $3, $4) RETURNING "id"> with backtrace: # ./spec/models/project_spec.rb:11:in `block (3 levels) in <top (required)>' # ./spec/models/project_spec.rb:14:in `block (4 levels) in <top (required)>' # ./spec/models/project_spec.rb:13:in `block (3 levels) in <top (required)>' # ./spec/models/project_spec.rb:13:in `block (3 levels) in <top (required)>' Finished in 0.28738 seconds (files took 1.42 seconds to load) 2 examples, 1 failure
見事にエラーが起きますね、なんせkeyの生成処理を書いていませんからね。そしてこのエラー、ちょっと気になるエラーだなと思いました。
rails
のレイヤーではなくdb
のレイヤーにまで到達しのエラーとなっているので、もうちょい手前でチェックしたいですね。なので、validation
を追加します。
app/models/project.rb
class Project < ActiveRecord::Base validates :key, uniqueness: true end
そうするとエラー内容が変わります。
Project create project should be persisted create projects should not raise Exception (FAILED - 1) Failures: 1) Project create projects should not raise Exception Failure/Error: expect { expected no Exception, got #<ActiveRecord::RecordInvalid: Validation failed: Key has already been taken> with backtrace: # ./spec/models/project_spec.rb:11:in `block (3 levels) in <top (required)>' # ./spec/models/project_spec.rb:14:in `block (4 levels) in <top (required)>' # ./spec/models/project_spec.rb:13:in `block (3 levels) in <top (required)>' # ./spec/models/project_spec.rb:13:in `block (3 levels) in <top (required)>'
rails
のレイヤーで守ってくれている感が出ました。二重で守ることに関しては賛否両論があるかと思いますが、validation
を書くことによるメリットとしてはrails
の分かりやすいメッセージを表示できるという点にあると思います。
validates
に失敗すると、project.errors
の中身にエラーのメッセージが保存されるので、それをユーザーに表示する選択肢が生まれますね。
Project#key にユニークな値を設定する処理を追加する
class Project < ActiveRecord::Base validates :key, uniqueness: true + before_validation :set_key + + private + + def set_key + self.key = SecureRandom.hex + end +end
今回は、rubyの標準ライブラリを使いました。SecureRandom.hex
でランダムな文字列を作ってくれます。
この実装のおかげで、、、
$ rspec
Root visit / path should text "ふりかえり" Project create project should be persisted create projects should not raise Exception Finished in 0.31031 seconds (files took 1.43 seconds to load) 3 examples, 0 failures
久々のオールグリーン。
ProjectsController を作る
地味な工夫としては、インスタンスの生成処理はなるべくbefore_action
にまとめて、アクションで行う処理にフォーカス出来るようにしています。
app/controllers/projects_controller.rb
class ProjectsController < ApplicationController before_action :set_project, only: [:show] before_action :set_new_project, only: [:new, :create] def new end def create if @project.save redirect_to @project, notice: 'created new project' else render 'new' end end def show end private def set_project @project = Project.find_by(key: params[:key]) end def set_new_project @project = Project.new(project_params) end def project_params params.fetch(:project, {}).permit(:name) end end
viewはform処理はpartialを分けるという癖をつけておくと、新規と更新のformを共通化出来て便利です。ただデザインの都合上分かれるということもありうるので、そこは後でリファクタリングでオッケーでしょう。
app/views/projects/_form.html.haml
= form_for(project) do |f| = f.text_field(:name) = f.submit
app/views/projects/new.html.haml
= render('form', project: @project)
app/views/projects/show.html.haml
%h1= @project.name
routes
のポイントは、param
です。今回、project
のURLを/projects/xxxxxxxx
という感じにしたいのでrails
のid
を使いたくないのでresources
が生成するルートにもkey
を使いたいのです。
config/routes.rb
Rails.application.routes.draw do resources :projects, only: [:new, :create, :show], param: :key root to: 'pages#top' end
このおかげでrake routes
すると
projects POST /projects(.:format) projects#create new_project GET /projects/new(.:format) projects#new project GET /projects/:key(.:format) projects#show
普段、:id
となるところが:key
になっていますよね。これで、contoller
にて、実際は:id
じゃないけどparams[:id]
で受け取るみたいなつらいことが避けられます。
さらに、モデルで#to_param
を定義しておくとURLヘルパーでもわざわざkey
を渡さないで済みます(いつもどおりproject_path(@project)
みたいに書ける)。定義しておきましょう。
diff:app/models/project.rb
class Project < ActiveRecord::Base validates :key, uniqueness: true + before_validation :set_key + + def to_param + key + end private def set_key self.key = SecureRandom.hex end end
テストはこんな感じで、feature
テストなのでscenario
にて手続きを明記します。ポイントは、within
で見るDOM
の範囲を絞ることでテストの範囲を明確にするところです。それと、変わりがちな文言よりもcssのclass
などでfind
している点です。この辺はデザイナーさんやコーダーさんとの連携が非常に重要となる点ですね。
spec/features/projects_spec.rb
require 'rails_helper' describe 'Projects' do let(:new_project) { build(:project) } feature 'create new project' do let(:created_project) { Project.last } scenario do visit new_project_path within('#new_project') do find('#project_name').set(new_project.name) find('input[type=submit]').click end expect(page).to have_content('created new project') expect(page).to have_content(new_project.name) expect(current_path).to eq project_path(created_project) end end end
さて、テストを実行。
$ rspec
Projects create new project should eq "/projects/ac97ac5f7733e87451bfab767e30c121" Root visit / path should text "ふりかえり" Project create project should be persisted Finished in 0.45112 seconds (files took 1.63 seconds to load) 3 examples, 0 failures
Issue を作る
rails g model〜
は端折ります。
各project
はissue
を複数持てるようにしたいというのと、Issue#status
でKPT
のkeep
, problem
, try
を示す事がポイントです。そうした処理はbrainspec/enumerizeを使うと便利です。
app/models/issue.rb
class Issue < ActiveRecord::Base extend Enumerize KPT = %w(keep problem try) enumerize :status, in: KPT, scope: true, default: KPT.first belongs_to :project end
app/models/project.rb
class Project < ActiveRecord::Base validates :key, uniqueness: true before_validation :set_key + has_many :issues def to_param key
db/migrate/20141126215936_create_issues.rb
class CreateIssues < ActiveRecord::Migration def change create_table :issues do |t| t.integer :project_id, null: false t.string :title, null: false t.string :status, null: false t.timestamps end add_index :issues, [:project_id] end end
ダミーデータは、FactoryGirl
のtrait
を使うと、「ここではこういう条件のデータを使いたい」「こっちでは、こんな」というのが楽に扱えます。
ここでは、keep
な状態のissue
はcreate(:issue, :keep)
と使う感じです。
spec/factories/issues.rb
FactoryGirl.define do factory :issue, class: 'Issue' do title { Faker::Name.title } trait :keep do status { :keep } end trait :problem do status { :problem } end trait :try do status { :try } end end end
spec/models/issue_spec.rb
require 'rails_helper' RSpec.describe Issue do let(:project) { create(:project) } describe 'create issue' do subject { create(:issue, project: project) } it { expect(subject).to be_persisted } end end
テストはこんな。通ります。
$ rspec
Projects create new project should eq "/projects/a41150e0851e47df8da365f3d3ecf8d8" Root visit / path should text "furikaeri" Issue create issue should be persisted Project create project should be persisted Finished in 0.41658 seconds (files took 1.47 seconds to load) 4 examples, 0 failures
IssuesController を作る
ポイントは、issue
はproject
の詳細ページから作れれば良いので、作成・削除はredirect_to :back
で戻るという点。
app/controller/issues_controller.rb
class IssuesController < ApplicationController before_action :set_project before_action :set_issue, only: [:destroy] before_action :set_new_issue, only: [:create] def create if @issue.save redirect_to :back, notice: 'created new issue' else redirect_to :back, notice: @issue.errors.full_messages.join(',') end end def destroy if @issue.destroy redirect_to :back, notice: 'deleted new issue' else redirect_to :back, notice: @issue.errors.full_messages.join(',') end end private def set_project @project = Project.find_by(key: params[:project_key]) end def set_issue @issue = @project.issues.find(params[:id]) end def set_new_issue @issue = @project.issues.build(issue_params) end def issue_params params.require(:issue).permit(:title, :status) end end
view
はpartial
だけの構成でprojects
から呼ぶイメージです。issues
のpartials
がどこにあるか?を探す際にviews/issues
から探すのは自然の流れです。 ここにあるだろう! という気持ちを妨げないようにしたいです。
app/views/issues/_form.html.haml
= form_for([project, issue]) do |f| = f.text_field(:title) = f.select(:status, Issue.status.options) = f.submit
app/views/issues/_issue.html.haml
= issue.title
app/views/issues/_issues.html.haml
%ul{class: status} = content_tag_for(:li, issues.with_status(status)) do |issue| = render('issues/issue', issue: issue)
app/views/projects/show.html.haml
%h1= @project.name = render('issues/form', project: @project, issue: Issue.new) %h2 Keep = render('issues/issues', issues: @project.issues, status: :keep) %h2 Problem = render('issues/issues', issues: @project.issues, status: :problem) %h2 Try = render('issues/issues', issues: @project.issues, status: :try)
issues
はproject
に紐づくのでリソースをnested
に表します。
config/routes.rb
Rails.application.routes.draw do - resources :projects, only: [:new, :create, :show], param: :key + resources :projects, only: [:new, :create, :show], param: :key do + resources :issues, only: [:create, :destroy] + end root to: 'pages#top' end
テストはとりあえずざっくり、issueが作成出来るように
spec/features/issues_spec.rb
require 'rails_helper' describe 'create new issue' do let(:project) { create(:project) } let(:new_issue) { build(:issue) } feature 'create new issue' do scenario do visit project_path(project) within('#new_issue') do find('#issue_title').set(new_issue.title) select(new_issue.status_text, from: 'issue_status') find('input[type=submit]').click end expect(page).to have_content('created new issue') within("ul.#{new_issue.status}") do expect(page).to have_content(new_issue.title) end expect(current_path).to eq project_path(project) end end end
テストの結果はこんな。
$ rspec
create new issue create new issue should eq "/projects/f7dfb9c9b04099d37412a1737443de6f" Projects create new project should eq "/projects/4b5f4b9012fbac685983b6f44916a6ea" Root visit / path should text "ふりかえり" Issue create issue should be persisted Project create project should be persisted Finished in 0.53655 seconds (files took 1.5 seconds to load) 5 examples, 0 failures
もっとテストしたい
IssuesController
のテストが足りないですね。keep
しかカバーしていないので、他も書きます。
spec/features/issues_spec.rb
describe 'create new issue' do let(:project) { create(:project) } let(:new_issue) { build(:issue, status) } context 'keep' do let(:status) { 'keep' } feature 'create new issue' do scenario do visit project_path(project) within('#new_issue') do find('#issue_title').set(new_issue.title) select(new_issue.status_text, from: 'issue_status') find('input[type=submit]').click end expect(page).to have_content('created new issue') within("ul.#{new_issue.status}") do expect(page).to have_content(new_issue.title) end expect(current_path).to eq project_path(project) end end end context 'problem' do let(:status) { 'problem' } feature 'create new issue' do scenario do visit project_path(project) within('#new_issue') do find('#issue_title').set(new_issue.title) select(new_issue.status_text, from: 'issue_status') find('input[type=submit]').click end expect(page).to have_content('created new issue') within("ul.#{new_issue.status}") do expect(page).to have_content(new_issue.title) end expect(current_path).to eq project_path(project) end end end end
problem
まで書いて気づきます。これはもしや更にコピペする予感。。。 3度同じことをし始めたらまとめる というルールで進むのがお勧めです。3度目がくるぞ、という予感でDRYを意識しましょう。
ここは、shared_examples_for
でまとめると綺麗になります。
spec/features/issues_spec.rb
require 'rails_helper' shared_examples_for 'create new issue' do feature 'create new issue' do scenario do visit project_path(project) within('#new_issue') do find('#issue_title').set(new_issue.title) select(new_issue.status_text, from: 'issue_status') find('input[type=submit]').click end expect(page).to have_content('created new issue') within("ul.#{status}") do expect(page).to have_content(new_issue.title) end expect(current_path).to eq project_path(project) end end end describe 'Issues' do let(:project) { create(:project) } let(:new_issue) { build(:issue, status) } context 'keep' do let(:status) { :keep } it_behaves_like 'create new issue' end context 'problem' do let(:status) { :problem } it_behaves_like 'create new issue' end context 'try' do let(:status) { :try } it_behaves_like 'create new issue' end end
今回はこのテストだけでの利用だったので同じファイル内に書きましたが、他のテストでも使う場合はspec/support
配下に置くとよいですね。
$ rspec
Issues keep behaves like create new issue create new issue should eq "/projects/351c621ef51c404d5d6ddd6cdad74619" problem behaves like create new issue create new issue should eq "/projects/94675fb6922599c0b3d98f0fef72af03" try behaves like create new issue create new issue should eq "/projects/2b5a00b0ec7d438dbd99009f9175147c" Projects create new project should eq "/projects/13f8cf1a6dda2e837d2fbc1a898f2aa9" Root visit / path should text "furikaeri" Issue create issue should be persisted Project create project should be persisted Finished in 0.62318 seconds (files took 1.46 seconds to load) 7 examples, 0 failures
残りのテスト
削除処理を忘れていました。足しましょう。同じくshared_examples_for
でスッキリと。
spec/features/issues_spec.rb
require 'rails_helper' shared_examples_for 'create new issue' do - feature 'create new issue' do + let(:new_issue) { build(:issue, status, project: project) } + + feature do scenario do visit project_path(project) within('#new_issue') do @@ -19,22 +21,38 @@ shared_examples_for 'create new issue' do end end +shared_examples_for 'delete an issue' do + let!(:issue) { create(:issue, project: project) } + + feature do + scenario 'delete an issue' do + visit project_path(project) + within(".issue:last-child") do + find('.delete-link').click + end + expect(page).to have_content('deleted an issue') + end + end +end + describe 'Issues' do let(:project) { create(:project) } - let(:new_issue) { build(:issue, status) } context 'keep' do let(:status) { :keep } it_behaves_like 'create new issue' + it_behaves_like 'delete an issue' end context 'problem' do let(:status) { :problem } it_behaves_like 'create new issue' + it_behaves_like 'delete an issue' end context 'try' do let(:status) { :try } it_behaves_like 'create new issue' + it_behaves_like 'delete an issue' end end
さて、それなりに機能が出揃いました。
$ rspec
Issues keep behaves like create new issue should eq "/projects/dd188d439deb814443c5057408657fa2" behaves like delete an issue delete an issue problem behaves like create new issue should eq "/projects/91c6e9836acbe615a71eec52140e7ce4" behaves like delete an issue delete an issue try behaves like create new issue should eq "/projects/685c2ce50f336a7ec7adc3482c76f764" behaves like delete an issue delete an issue Projects create new project should eq "/projects/ac915e7de04cf5c8d5802ceed92c7bf1" Root visit / path should text "furikaeri" Issue create issue should be persisted Project create project should be persisted Finished in 0.81576 seconds (files took 1.47 seconds to load) 10 examples, 0 failures
気になる点を直す
/projects/xxxxx
=>/xxxxx
にしたい- トップページにdemoプロジェクトを表示したい
といった点を改善したいと思いました。その辺りは、ppworks/furikaeriにあげてあります。
画面は?
ここまでひと通りの実装をする際にウェブブラウザで確認することはありませんでした。コアな機能を実装する際にこのように最低限のテストを書きながら進めると、その後のUIの検討や機能調整の際にコアの部分を壊さないように勧めることが出来るようになります。
さて画面の確認は、
$ brew install postgresql $ git clone git@github.com:ppworks/furikaeri.git $ git checkout v1.0.2
$ bundle $ rake db:create:all $ rake db:migrate
$ rails s
して頂くとよろしいかと思います。もしくは、ppworks/furikaeri/tree/v1.0.2より
Heroku' Buttonであなたのheroku環境へdeploy可能です。heroku buttonからdeployされるコードは随時バージョンアップしていくかもしれないので、今回の記事時点でのコードは、ppworks/furikaeri/tree/v1.0.2のコードを御覧ください:)
これは最低限の機能しか無いですし、画面のデザインもまだです。妄想としてはwebsocket
で同時編集とかheroku buttonを設定すると熱いですね。pull request
ももちろん受付中です。
終わりに
マネーフォワードでは、おもてなしの心を持ってコードを書くエンジニアを募集しています! みなさまのご応募お待ちしております!pull req応募もお待ちしております:)
マネーフォワード採用サイト https://recruit.moneyforward.com/