実録レガシー Rails アプリ改善ジャーニー / Legacy Rails app improvement journey

by shutooike

Slide 1

Slide 1 text

実録レガシー Rails アプリ改善ジャーニー 2023.3.25 第90回 Ruby関西勉強会 / 舘林秀和 @shutooike

Slide 2

Slide 2 text

自己紹介舘林秀和 / Shuto Tatebayashi 所属株式会社インゲージ各種アカウント @shutooike 途中で名字が変わってややこしい趣味ひとことコミュニティで発表するのが初めてなので緊張してます

Slide 3

Slide 3 text

目次ジョイン当時の状況課題改善ジャーニー自動テスト編バージョンアップ編リファクタリング編得られたものこれからまとめ

Slide 4

Slide 4 text

ジョイン当時の状況 2014年から開発している Rails アプリフロントエンドはSPA バックエンドはAPI+Job サービスはたくさんのユーザーに使われている Ruby のバージョン2.3 RSpec が導入されているが、カバレッジは20%以下テストがないことがほとんど＝レガシーコード QAはステージング環境でのドックフーディングと手動テスト頼み CIはあった（werkerを使用）週に1回本番リリース

Slide 5

Slide 5 text

課題テストが継続的に書かれてない簡単な不具合修正、機能改善、負債返却で他が壊れることがある以前と変わることは手動テストで担保するが以前と変わっていないことを担保できていないステージング環境によるドッグフーディングには限界が変更に対する心理的安全性が低い言語・ライブラリのバージョンアップが継続的に行われていない EOL切れバージョンアップ時の防衛線がステージング環境にしかいない同じようなロジックがあちこちに点在

Slide 6

Slide 6 text

改善ジャーニー次のスライドから取り組んできたことおおよそ時系列で紹介していくこの発表で説明しきれない詳細は今後ブログに書いていくつもり Twitter で @shutooike をフォローして、読んで、拡散して、感想ください（欲張り）

Slide 7

Slide 7 text

自動テスト編

Slide 8

Slide 8 text

実行順序のランダム化ファイル名順での実行になっており順番に依存したテストがちらほら spec ファイルを追加すると突然落ちるようになったり config.order = :random テスト数がそこまで多くなかったのでエイヤで :random にして、ローカルやCIで落ちたら都度修正 Kernel.srand config.seed とすると Array#sample も seed で再現するようになる

Slide 9

Slide 9 text

テストデータが残らないように CIで特定のテストが失敗することが稀によくある他のテストで作ったデータが原因 example ごとにデータを消す config.use_transactional_fixtures = true before(:all) で作りっぱなしのデータ before(:each) に変更 after(:all) で削除

Slide 10

Slide 10 text

自動テストを書いていく（1）日々の開発で自分が関わったコードからテストを書いていくなぜか書きにくいという実感が... 原因突き詰めたところ →「そうか！土台が出来てないんだ！」プロジェクト発足

Slide 11

Slide 11 text

実際の最初の issue

Slide 12

Slide 12 text

スキップしていたモデルのコールバックを解放 User.skip_callback(:create, :after, :foo) 思考停止でのほぼ全てのコールバックがスキップされていたプロダクトコードではデータ作成時にコールバックで行っている処理を、テストデータ作成時は自分で明示的に処理しないといけない状況テスト環境の挙動は出来る限り本番環境に近づけるべき一部残したコールバックもある時間がかかりすぎるもの外部サービスを呼び出すものモック化するという手もある

Slide 13

Slide 13 text

FactoryBot のフル活用お馴染みのテストデータ生成ライブラリシナリオに沿ったテストデータをサクッと作れないとテストを書くハードルが一気に上がる導入はされているが、上手く運用はされていない状況 FactoryBot の定義方針を検討・決定詳しくはブログに書きます方針に沿って FactoryBot を再定義・既存のテストを修正最初は頑張って全モデルでやろうとしたがその必要はなかったよく出てくるスタメンモデルだけでOK

Slide 14

Slide 14 text

自動テスト戦略を立てるここまでテストを書いた感覚でテスティングピラミッドを目指すのは厳しいと判断単体テストを継続的に書くのは開発者の力量に依存するところが大きい良くない単体テストはむしろ開発生産性を下げるプライベートメソッドのテストを書く期待値生成にコードと同じロジックを用いる... などまずは統合テストたくさん書いていくことにする目指すはテスティングトロフィー主な Rails の責務である API の request spec を書いていくとりあえずある程度までカバレッジを上げたい

Slide 15

Slide 15 text

誰でも書ける request spec helper を用意誰でも書けるように薄さにこだわる正常系のみ最低限の条件ログインユーザーパラメーター最低限の検証 APIの権限チェック HTTPステータスコードチェック APIを作ったら絶対書こうねという意識付け →「お約束」というネーミング

Slide 16

Slide 16 text

利用側のイメージ describe 'POST /api/messages' do context ' お約束' do let(:login_user) { create(:normal_user) } let(:minimum_required_params) { { text: 'Hi' } } it_behaves_like(:can_be_accessed_with, 'normal') it_behaves_like(:return_http_status_as, :created) end end どのように実装したかはブログに書きます

Slide 17

Slide 17 text

全APIに正常系テスト（お約束）を書いていく全体カバレッジが60%ほどに APIのカバレッジは90%近くに少なくとも最低限の正常系の動作が変わっていないことが担保できるようになったので安心感が全然違うコスパ最強

Slide 18

Slide 18 text

request spec でAPIの振る舞いを検証するクリティカルユーザージャーニー（CUJ）を定義ユーザーが 1 つの目的を達成するために行うサービスとの一連のインタラクション CUJをもとにサービスの主要な操作に関するAPIに対して正常系・異常系の振る舞いを検証振る舞いとは？データのCRUD ジョブのキック外部サービスの呼び出し... など主要なAPIの動作の担保され安心感がさらに上がった

Slide 19

Slide 19 text

最低限のE2Eテストを書く SPAといいつつ複数画面があるメイン画面編集画面外部連携用画面... など最低限の検証画面が正常に開けることのみ確認ステージング環境でのドックフーディングでは普段開かない画面がひっそり死んでることに気づけるように

Slide 20

Slide 20 text

バージョンアップ編

Slide 21

Slide 21 text

Ruby 2.3 → 2.7.6 警告の対応依存 gem のバージョンアップ開発環境でバージョンアップ自動テスト・手動テスト他の開発者の開発環境に展開ステージング環境バージョンアップ手動テスト本番環境バージョンアップ

Slide 22

Slide 22 text

Rails 5 → 6 警告の対応依存 gem のバージョンアップ開発環境でバージョンアップ rails app:update new_framework_defaults 自動テスト・手動テストステージング環境バージョンアップ手動テスト本番環境バージョンアップ

Slide 23

Slide 23 text

リファクタリング編

Slide 24

Slide 24 text

複数モデルで使える共通の処理を concern に Rails がデフォルトで用意している仕組み上手く使えたら綺麗に抽象化できる上手く使えたら... 例 created_by, updated_by を入れる処理

Slide 25

Slide 25 text

単体テストが書きやすくなるような実装に自動テスト戦略で統合テストに注力したもう1つの理由として単体テストが書きにくい実装になっていたドメインロジックがコントローラー層に Fat Controller より単体テストが書きやすいようにドメインロジックをモデルに寄せていく必要がある Fat Model 自動テストがある程度書けたので振る舞いが変わらないことを担保しながらリファクタリングができる

Slide 26

Slide 26 text

[再掲] 課題テストが継続的に書かれてない簡単な不具合修正、機能改善、負債返却で他が壊れることがある言語・ライブラリのバージョンアップが継続的に行われていない EOL切れバージョンアップ時の防衛線がステージング環境にしかいない同じようなロジックがあちこちに点在

Slide 27

Slide 27 text

得られたものテストが書きやすい環境 APIを作成したらお約束テストを追加する文化ある程度のカバレッジコードの変更やライブラリのバージョンアップなどに対する心理的安全性他に影響があるコード変更がCIで落ちるライブラリのバージョンアップの非互換に気付けたことも

Slide 28

Slide 28 text

[再掲] 課題テストが継続的に書かれてない簡単な不具合修正、機能改善、負債返却で他が壊れることがある言語・ライブラリのバージョンアップが継続的に行われていない EOL切れバージョンアップ時の防衛線がステージング環境にしかいない → 継続してやっていく同じようなロジックがあちこちに点在 → 継続してやっていく

Slide 29

Slide 29 text

これからとりあえず「EOLを切らない」を目標に Ruby, Rails を上げていく dependabot をフル活用してライブラリアップデートを習慣化テスティングピラミッドを目指す単体テストが書きやすくなるよう引き続きリファクタリングステージング環境で行っている手動テストの負荷低減現在の最低限のE2Eから少し検証範囲を広げる

Slide 30

Slide 30 text

まとめ自動テストは書かないと上手くならない勘所を掴むまでクソテストコードを書いて書いて書きまくれ幸いテストコードは書き直しやすいレガシーコード改善に銀の弾丸はないカバレッジも可読性、メンテナビリティも突然上がることはない理想に向けてロードマップを引き地道にやっていくしかないレガシーコードを愛するユーザーの課題を解決し、売上を立てるレガシーコードは偉大なりたくてレガシーコードになった訳じゃないその時々の最適解だったはず恨みではなくリスペクトを持って接する

Slide 31

Slide 31 text

おまけ1 カバレッジは正しく測ろう以下の2つを行ったらカバレッジがグッと上がった # config/envioroments/test.rb config.eager_load = true spring を使わずに RSpec を実行原因ははっきりわかっていないがおそらくコードの分母が正しく測れるようになったから知ってる人いたら教えてください

Slide 32

Slide 32 text

おまけ2 カバレッジを追い求めすぎないカバレッジが高いからバグが出ない訳じゃない効果的なテストをすることが一番重要カバレッジは目標にせず、指標にするべき