LoginSignup
19
9

More than 5 years have passed since last update.

テストのリファクタリング

Posted at

前世紀のTDD原人です。

異動したばかりなので、開発環境構築から始めて、リハビリ兼ねてテストのリファクタリングをしています。

追加投入された自分の工数ならしばらくリファクタリングに全振りできるでしょう。

Why:テスト・リファクタリング

プロダクトコードを安全にリファクタリングするためにUnitTestを書くわけですが、プロダクトの歴史が長くなるとUnitTest自体もリファクタリングする必要が出てきます。

良いブログ記事をみつけました。

テストをリファクタリングするとき気をつけること 2012

テストは減らすのが一番

プロダクトコードを変更するために、テストコードの変更が発生する。ということは、テストコードは少ない方が、プロダクトコード変更のコストが減ります。ということは、単純にテストを減らせば変更に強くなります。

現プロジェクトだとビルドに1分以上かかるサブプロジェクトだけで11本、合計47分もかかっていました。かなりの時間がJUnitです(あと大きいのはJavaScriptのminify処理)。

ローンチから約5年、機能追加・機能拡充の開発は止まること無く、人数も工数も投下されつづけましたからね。

UnitTestが大事なことはこのチームには伝わってる。今度はテストを減らしていきましょう。

減らすのはやっぱり怖い

ブログからさらに引用すると

このテスト消しても大丈夫だろうか? なんでこんなテストがあるんだっけ? ずっと後でバグ作り込みの原因になったりしないだろうか? よく似たテストがあるけど、ちょっとだけ違うんだよね、どっちを残そうか? などなど。ある意味では、テストを書くより消す方がよっぽど時間と神経を使います。
消したいけれど自信がないなら安全寄りに倒して、消さないでおくようおすすめします。

ブランクもあるし、まずは手を動かしながら対象を観察しなおすためにも、絶対安全なリファクタリングからやっていきます。

アンチパターン:同値テスト

mvn test の出力 surefire-reports を調べて、一番遅いものをみつけて、手元で単体実行して観察します。
アプリケーション層のとあるService class の 1 methodが対象。所要時間 2min18sec!

  • UnitTestの対象method呼出前に、INFO LOGを追加し、回数とパラメータバリエーションをチェック
  • 2324 Calls!, ユニークパラメータは 728!

同じパラメータで3度呼び出しても同じ結果しか返らないのに。返るオブジェクトの3つのfieldをそれぞれに確認するために3度callしていた。
1callで 3値ともチェックするテストコードに変更して、3倍速。

パラメータのうち「実行日時」が違っても期待値が同じパターンが5-7個ずつ見つかった。ビジネスロジックとしては毎月の締日から次締日の間は期待値一定なので、実行日時パラメータは最小・最大の2ケースのみに縮小して、252 Calls さらに3倍速。(この無駄な実行日時バリエーションは他のTestCaseにも何回もコピペされていた)

テストパターン・テストデータについても DRY(Do not Repeat Yourself)原則は意識しておきたい。

アンチパターン:テストデータ肥大

月別配信される教材の取得テストケース。
あとで追加されたテストケースでも前提とするテストデータを共通化するために、6年度分 x 4教科 x 6学年 x (2出版社 + 1) x 2(標準|発展) の教材情報テストデータになっていた。

「テストに必要なテストデータだけをつくるようにするべし」

各テストケースで必要な期間・教科に限定したテストデータを作れるように改修。事前作成するデータ件数が1/10に。

アンチパターン:過眠症

// 5秒待ってDBのnowをずらす
Thread.sleep(5000);

created_at とかの日時情報だけ違うデータをつくりたい場合によくやるコード。単体実行なら5秒でもまあいいか。でもね CIにのっかるとこのsleep()が生涯で何千回、何万回されることになるか想像してくださいな。grepかけて50msecに縮小統一しました。

サブスレッドを起こして、その終了待ちをミスってタイムアウトまで待っていたコードも見つけました。

...
executor.schedule(thread9, 1, TimeUnit.MICROSECONDS);
executor.schedule(thread10, 1, TimeUnit.MICROSECONDS);

executor.shutdown(); // Do not miss it.
executor.awaitTermination(10, TimeUnit.SECONDS);

アンチパターン:過保護なデータベース

UnitTestで使うデータベースは、CIの度にDDLから初期化したり、setUp()/teadDown() で各テストケース実行毎にデータ投入・削除したり。

運用DBのように、クラッシュリカバリに備える必要はまったくありません。それなら、Write Ahead Log(Redoログ)の書き込み保証なんていりません。ロック競合もないのにRDBへの更新が遅いのは主にこいつのせいですから、UnitTest環境、CI環境ではとっぱらいましょう。

PostgreSQLの場合は、postgresql.conf を2カ所書き換えて再起動です。

#-------------------
# WRITE AHEAD LOG
#-------------------
#fsync = on
fsync = off
#synchronous_commit = on
synchronous_commit = off

設定の確認

-bash-4.1$ psql                                                                                                                                                                                                                      
postgres=# show fsync ;                                                                                                                                                                                                              
 fsync 
-------
 off
(1 row)

postgres=# show synchronous_commit;
 synchronous_commit 
--------------------
 off
(1 row)

アンチパターン:増殖テストケース

気持ちはわかります。

初回リリースが終わって機能追加をリリースするとき

既存テストケースには手をつけないですよね。既存機能に影響がないことを確認しながら、追加実装したいので。

既存機能の拡充だったら
やはり既存テストケースには手をつけず、新規にテストケースを追加するでしょう。
この場合、テストの前提データの準備や、拡充機能の前提となる機能のテストもいるでしょうから、既存テストケースをコピペしての、拡充機能対応のテストを追加するでしょう。

これを5年続けると最悪の場合、こうなります。

ある1つのTestClass に入っている TestCase数 343!


To Be Continued. ここから先は安全確実とはいえない世界。

19
9
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
19
9