レガシーなプロダクトにテストで向き合う話

2014/02/20

gong023

はじめまして。荻原といいます。グリーのプラットフォーム部門で、サーバーサイドのエンジニアをしています。

昨年末ぐらいまで業務の空き時間にテスト周りでごにょごにょと動いていたので、今日はそのことについて書かせて頂きます。

こんな人は読むと役に立つかもしれません。

レガシーなプロダクトになんとかして突破口を開きたい
PHPUnit の書き方で参考になりそうなものを探している
Ruby でスマートフォンのブラウザ操作を自動化したい

経緯

こちらでも言及されている通り、サービスを運営している以上、時には技術的負債に向き合わなければなりません。GREEも歴史が長いプロダクトなので、日々コードをリリースしていく中でそういった問題に頭を抱える場面もありました。

技術的負債による副作用はたくさんありますが、どういう点に不安を感じていたのか、実際に開発の現場に立って感じたことをいくつか書いてみたいと思います。

コードが読めない・・・

私がチームにアサインされた当初、担当範囲のコードを読み解くのに非常に苦労しました。もちろんこれは私のリーディング力が低かったからというのもあると思いますが、それを差し引いてもコード以外に理解しなければならない業務知識も多く、チームにジョインしていきなり仕事をするのはほとんど不可能なように思われました。

しかし、そんな状況でもタスクは存在します。業務に慣れないうちは、中途半端な理解でだましだましコードを書くしかありませんでした。お恥ずかしい話ですが、しばらくはそんな状態で仕事をしていて、先輩に「あーそれとそれの順番かえちゃダメだから。」という類の指摘を受けたこともありました。この時言われてなければ大障害を起こしていたと思うと今でもぞっとします。

それから時間が経ち、ある程度業務知識も増え、コードの全体像も理解できるようになったころ、かつて作った部分をもう一度メンテする機会がありました。以前やった箇所だから簡単だろうと思ってファイルを開くと、コードには無邪気に if や for が積み上げられていて愕然としました。今だったら業務背景も理解しているしコードの全体像もわかる。このディレクトリのこんなクラスにこんなコード書く必要ないのにと思いつつ、あんなに苦労させられたコードの加害者の一員に、知らぬ間に加わっていたのだということに気づいてショックを受けました。
プロダクトがこういう状態になってしまうと

コードが読めない
よくわからないから適当に注ぎ足す（故意・過失に関わらず）
さらにコード読めなくする

というサイクルに入ってしまい、もはや割れ窓理論状態です。

リリース前テストが終わらない・・・

私のチームでは、コードが完成し、レビューも完了となると、次はステージング環境での受け入れテストというフローになっていました。

一つ補足しておくと、私が今回言及する「受け入れテスト」とは「ユーザーから見て、対象のソフトウェアが受け入れられるものかどうかテストする」という意味です。もっと根も葉もない言い方をすると、ポチポチ端末をいじってテストすることです（少なくとも今回の記事ではそういう意味で使っています）。近い意味としてよく用いられる言葉は、ブラックボックステスト、統合テストなどでしょうか。

受け入れテストは、デベロッパー目線ではなくユーザー目線であることに注意して下さい。私のチームではこれを徹底するため、受け入れテストは専門のチームに依頼していました。開発者自身がテストする場合、修正箇所だけを確かめがちになりますが、そうではなく、第三者にユーザー目線で広くテストしてもらうことは品質担保という意味で非常に妥当だと言えます。

しかしそれゆえに、時間もかかります。開発者からすれば修正箇所以外のテストも行われるので無駄なような気がしてしまうのですが、実際に予期していない箇所から不具合が検出されることもありました。思わぬところで影響が出てしまうというのは技術的負債を背負ってしまったプロダクトならではという感じですが、だからこそしっかり確認していかなければなりません。

また、時間がかかる要因として、非常に多くのテストケースをカバーしなければならなかったことも挙げられます。基本的に歴史の長いプロダクトであるほど満たさなければならない仕様が多く、色々な条件を掛けあわせていくと場合分けがどんどん発散していきます。さらにそれを様々な端末・OSでテストするので、テストケースは膨大になってしまいます。テストケースが膨大になると、今度はテストを依頼するチームとのコミュニケーションコストも無視できない問題になってきます。

しかしいくら時間がかかるとはいえ、これは決してスキップすべきフローではないように思いました。ミッションクリティカルな部分を触ることも多いチームだったので、些細なものでも不具合を看過すべきではありませんでした。

そんなこんなで、私はコードレビューが完了してから全ての修正を出すまでに二ヶ月あいたということも経験したことがあります（非常に極端な例でしたが）。確かにテストはスキップすべきではありませんが、同じような端末操作が行われていることも多かったので、自動化できればリリースを効率化できるのではと考えていました。

どうしよう

勢いで色々と書きましたが、一旦感じていたことをまとめてみようと思います。

コードの汚染度が高く、必要とされる業務ドメインをコードから読み解くのに苦労するし、その状況がまた汚染を加速させる
端末を操作してテストする作業がリリースのボトルネックになっており、自動化したい

大胆にリファクタリングを行っていくという選択肢もあるのかもしれませんが、多くの方々に利用して頂いているプロダクトに大幅な修正をしていくのは大変です。また、リファクタリングを加えるにせよ、それの前と後で挙動に差がでていないか確認する手段が必要です。そういった経緯で、テスト周りの強化に目が向きました。考えていたのは以下の二点です。

単体テストを強化し、ドキュメントとする
端末操作を自動化し、業務を効率化する

単体テストを強化するモチベーションは様々ですが、私の場合は特にドキュメントとしての効果を期待しました。テストを強化する前からドキュメントはあったのですが、どうしても wiki のような形式だとリリースするそばから古くなっていってしまいます。日々書き換わるコードに対して正しさを保証するためには、単体テストをドキュメントとして作っていくのが良いと思いました。イメージとしてはコードの翻訳という形に近いかもしれません。

受け入れテストに関しては、同じような端末操作を自動化することで直接的な効果を期待出来ました。ただし、専門のチームとコミュニケーションを取るためにも、非エンジニアの方でもどのようなテストをしているのかわかりやすいようにすることと、より広い範囲をカバーできるよう多端末でテストできるような仕組みにすることを念頭に置きました。

ドキュメントとしての単体テスト

単体テストにはドキュメントとしての効果を期待していましたので、可読性には特に気を払っていました。ここでは実際にサンプルを挙げて、どのような書き方をしていたかを紹介したいと思います。なお、テスティングフレームワークは PHPUnit を想定しています。

今回は以下のようなコードをテストターゲットととして考えてみます。

// テスト対象のコード
function isOverTen($num)
{
     return $num >= 10;
}

// テスト対象のコード

function isOverTen($num)

{

return $num >= 10;

}

よく見かけるのは以下のようなテストコードです。

function testIsOverTen()
{
     $this->assertTrue(isOverTen(10));
}

function testIsOverTen()

{

$this->assertTrue(isOverTen(10));

}

残念ですがこれは、可読性うんぬん以前に異常系をカバーできていません。意外に書かれていない場合が多いです。とりあえず異常系を書き足してみましょう。

function testIsOverTen()
{
     $this->assertTrue(isOverTen(10));
     $this->assertFalse(isOverTen(9));
}

function testIsOverTen()

{

$this->assertTrue(isOverTen(10));

$this->assertFalse(isOverTen(9));

}

というわけで書き足しましたが、同じ関数の中に複数のテストケースが混在しているのは頂けません。テストコードでは、関数を振る舞いごとに分けましょう。余談ですがこういうことを繰り返して、たまにテストコードなのに100行や200行もあるモンスター級の function になっているのを見かけたりします。こうなってしまうと何をテストしたいのかわからなくなってしまいます。もっと酷い場合 if や foreach などのロジックがふんだんに入ってテストが間違っているのか元のコードが間違っているのかの区別もつけられない時もあります。

function testIsOverTen_Normal()
{
     $this->assertTrue(isOverTen(10));
}
function testIsOverTen_Abnomal()
{
     $this->assertFalse(isOverTen(9));
}

function testIsOverTen_Normal()

{

$this->assertTrue(isOverTen(10));

}

function testIsOverTen_Abnomal()

{

$this->assertFalse(isOverTen(9));

}

異常系、正常系を分けました。私の場合、正常系・異常系ごとにテストケースを @group アノテーションで振り分ける場合が多いです。こうすることでテストの意図を明確にできます。

/*
 * @group normal
 */
function testIsOverTen_Normal()
{
     $this->assertTrue(isOverTen(10));
}
/*
 * @group abnormal
 */
function testIsOverTen_Abnomal()
{
     $this->assertFalse(isOverTen(9));
}

* @group normal

function testIsOverTen_Normal()

{

$this->assertTrue(isOverTen(10));

}

* @group abnormal

function testIsOverTen_Abnomal()

{

$this->assertFalse(isOverTen(9));

}

良くなってきましたが、もう少し読みやすく書けます。振る舞いで関数を分けたのなら、関数名もそれに合わせてしまいましょう。ついでに @test アノテーションをつけて、関数の頭の test という prefix を削ってしまったほうが読みやすいでしょう。

/*
 * @test
 * @group normal
 */
function isOverTen_ReturnTrue_With10()
{
     $this->assertTrue(isOverTen(10));
}
/*
 * @test
 * @group abnormal
 */
function isOverTen_ReturnFalse_With9()
{
     $this->assertFalse(isOverTen(9));
}

* @test

* @group normal

function isOverTen_ReturnTrue_With10()

{

$this->assertTrue(isOverTen(10));

}

* @test

* @group abnormal

function isOverTen_ReturnFalse_With9()

{

$this->assertFalse(isOverTen(9));

}

いかがでしょうか。全て同じコードに対するテストですが、書き方の工夫次第でかなり可読性が違うと思います。サンプルは isOverTen という prefix ではじめていますが、実際はテスト対象の関数ごとにテストクラスを一つ作っていたので、この部分は省略していました。テスト対象のクラスがかなりファットになってしまっている場合、テストクラスの方を細かくしないと対応していけません。

fixture の扱いも気をつけたいところです。何も考えずに一つの array で管理していたりすると、どのテストデータがどういう意図で設けられているのか全く読めないです（というか既存のテストがそうなっていて苦労しました）。今は PHP にも fixture をうまく管理するライブラリが作られているので、積極的に導入していって良いと思います。そもそもDBアクセスさせないという方針もアリで、DBデータは極力 mock を使うようにしていました。私はテストが失敗した時にDBの状態とロジック両方を疑わなければならない状態はあまり好きではないです。

また、弊社はフレームワークに Ethna を使っていますが、標準のテストクラスが SimpleTest の利用を前提としていたため、こちらでヘルパーを作りました。ついでにヘルパーにはスーパグローバル変数を操作して iPhone や Android の UserAgent をエミュレーションする機能を乗っけたりしました。

Ruby 周辺のツールで受け入れテストを自動化する

冒頭で述べたように、受け入れテストも効率よく行っていかないとリリースのボトルネックになってしまいます。今回は RSpec + turnip + selenium-webdriver + Appium という構成で自動化を実現しています。これに関しては実際にコードをみて確かめてもらったほうが早いので、今回は「ブラウザを立ち上げgoogleにアクセス > 検索ボックスにワードを入れて submit > 検索結果をスクリーンショットで保存」というブラウザ操作を自動化したサンプルを用意しました。是非一度動かしてみてください。

サンプルの動作方法は以下です。なお、サンプルは Ruby2.0 以降で動くので、事前にインストールしておいて下さい。

git clone git@github.com:gong023/acceptance_test_sample.git
cd acceptance_test_sample
bundle install
rake

git clone git@github.com:gong023/acceptance_test_sample.git

cd acceptance_test_sample

bundle install

rake

いかがでしょうか。うまくいっていれば Firefox が立ち上がりブラウザ操作が走ったかと思います。またインストールしたディレクトリの pic 以下に、検索結果のスクリーンショットが保存されているので確認してみてください。

せっかくなので、iOS でも動かしてみましょう。実機でも動かすことはできますが、サンプルではお手軽に iPhone Simulator で動くようになっています。Ruby に加え、事前に以下の環境を整えておいて下さい。

node.js v0.10以降
- Xcode 4.6.3以降
- command line tools
Appium 0.14.2以降
- 手元では、npm から取れる CLI ではなくこちらの GUI 版を使っています

環境が整ったら、Appium を launch して、以下のコマンドを実行してみてください。うまくいけば iPhone Simulator が立ち上がって先ほどと同じ操作が行われると思います。スクリーンショットが pic 以下に保存されるのも Firefox の時と同様です。

rake device=iphonesimulator

1	rake device=iphonesimulator

さて、iOS で動かせたら、次は Android です。Android は実機で動かすハードルが低いので、サンプルでも実機をターゲットにしてあります。環境面では、Appium を利用していないので node.js や Xcode は不要です。代わりに、以下のものをセットアップしておいて下さい。

Android SDK
- adb
android-webserver
- こちらからダウンロード
adb install android-server-2.21.0.apk で対象の端末に apk をインストールしておいて下さい。

セットアップが終わったら、以下のコマンドで試せます。

rake device=android

1	rake device=android

以上のサンプルをベースとして、チームではよくテストするブラウザ操作を jenkins の「ビルド実行」をクリックすれば行えるようにしてあります。リグレッションテストで役立つのはもちろんですが、定期的に動かして監視ツールとしても使えます。（ただ、現実は端末が wifi をつかめずテスト失敗したりするので、監視ツールとしての運用は難しい部分も多いですが・・・。）

さて、ここまで試して頂けたらコードもあることですし説明することは実はあまりないのですが、最後にいくつかこの構成を選択した理由を挙げておきます。

1.feature ファイルが魅力的

以下はサンプルの spec/acceptance/firefox/google_search.feature の抜粋です。

Feature: 受け入れテストの練習
  Scenario: グーグル検索とその結果の表示
    Given 実行環境は 'firefox'
    Then 'https://google.com' へアクセス
    Then 検索フォームに 'happy hacking!' と入力する
    Then フォームをsubmitする
    Then 検索結果に遷移したことを確認する
    Then 検索結果をスクリーンショットに保存する

Feature: 受け入れテストの練習

Scenario: グーグル検索とその結果の表示

Given 実行環境は 'firefox'

Then 'https://google.com' へアクセス

Then 検索フォームに 'happy hacking!' と入力する

Then フォームをsubmitする

Then 検索結果に遷移したことを確認する

Then 検索結果をスクリーンショットに保存する

どんなテストをしているのかひと目でわかると思います。そのため「このテストをこうして欲しい」といったコミュニケーションも簡単にとることができます。Gherkin をしっかりサポートできていそうなのは Ruby,Java,JavaScript,.NET でしたが、個人的な好みもあって RSpec + turnip を使っています。

ついでなので selenium-webdriver を使っている理由も簡単に説明しておきます。最初は driver に capybara を使っていたのですが、capybara が単に selenium-webdriver をラップしていたので、直接触ってしまって良いだろうということで selenium-webdriver を採用しました。特に capybara に不満があったというわけでもないのですが、こういうスクレイピングのような作業は繊細なので、うまく動かなかった場合に疑う対象は一つでも少ないほうが良いと考えました。

2.iOS, Android のブラウザをターゲットにできる

昨今はメインターゲットがスマートフォンだという場合が多いと思います。GREEもその例に漏れずスマートフォンからの利用が多いプロダクトなので、iOS 及び Android のブラウザからテストが行えるという点はかなり重要視していました。そういった意味では calabash でも良かったのですが、native アプリのサポートしかなく、iOS Safari ではテストできなさそうだということで使用をやめました。代わりに Appium を採用し、iOS でのテスト環境は当初想定したとおりに作ることができました。そのまま Appium で Android の環境も構築しようと考えましたが、chromedriver がデバイスの root を取らないと使えないということで見送り、代わりに端末に android-webserver を立てる形で対応しました。

雑感

少し脱線してしまいますが、色々やらせて頂いて学ぶことも多かったので雑感を書き連ねてみようと思います。

効果ある？

パピルスから紙に変えた効果を定量的に説明するのは難しいです。そして、テストを充実させるというのはそういう類の話だと思います。

また、開発に銀の弾丸はないとはよく言われますが、それ以前の問題としてテストは充実させるのは必要条件だと思います。

テスト書くと工数倍になる？

確かに書かなければならない行数は増えますが、TDD,BDD で開発する場合仕様やバグの確認を行いながら開発できるので、手戻りがなくかえって開発を早く行えるケースも珍しくないと思います。また、自身の書きたいコードを明文化しながら開発できるという点も大きなメリットで、これも開発効率を向上させる追い風になります。

逆に後追いでテストを書いていく場合、開発時の不安を思い出せないので大変です。

受け入れテストって難しい？

受け入れテストに使えるツールは本当に色々開発されているので、選ぶのが大変でした。またツールに一癖ある場合が多く、少し慣れが必要です。例えば対象ページのHTMLがうまく取得できず悩んだが、実際はページのレンダリングが終わらない内に要素を取得しにいっていたので、再試行する必要があった、などといった話です。こういう場合特にエラーを発したりしないのでこちらで原因にあたりをつけなければなりません。

方法論に関していえば、いわゆる普通のソフトウェアエンジニアの中でメジャーになっているものはあまりない印象ですが、かなり研究されており興味深かったです。一応調べた中ではハードウェアメーカーのテスト技法が面白かったです。HAYST法、CFD法、状態遷移図などを使えばより効率的にテストケースを洗い出せたかもしれません。またISTQB,JSTQB といったものもあるので、学んでみると面白そうです。

まとめ

抱えていた問題と対処策

コードが読めない
- 単体テストをドキュメントにする
  - (テスト対象関数名)_ Return(返り値) _When(条件) にした
受け入れテストに時間がかかる
- 端末操作を自動化する
  - RSpec + turnip + selenium-webdriver + Appium を使った

なんだかまとまりのない記事になってしまいましたが、ひとつの事例として皆様の参考になれば幸いです。